#469: [CAGE] revisit t/tools/ops2pm/05-renum_op_map_file.t
---------------------+------------------------------------------------------
Reporter: allison | Owner: jkeenan
Type: cage | Status: assigned
Priority: normal | Milestone: 1.1
Component: none | Version:
Severity: medium | Keywords:
Lang: | Patch:
Platform: |
---------------------+------------------------------------------------------
Comment(by jkeenan):
Replying to [comment:4 allison]:[[BR]]
After many hours staring at ''tools/dev/opsrenumber.pl'',
''lib/Parrot/OpsRenumber.pm'', ''t/tools/ops2pm/05-renum_op_map_file.t''
and related files, I have become convinced that it does not do what we
think it does -- or, at the very least, what ''I'' have long thought it
did.
If you add ops codes, whether to an existing file or to a new file, ''make
opsrenumber'' -- which is just a call to ''tools/dev/opsrenumber.pl'' --
does '''not''' automatically add those to ''src/ops/ops.num'' and
appropriately renumber the ops codes in that file. ''src/ops/ops.num'',
in fact, does not change at all.
If, however, you delete ops codes and run ''make opsrenumber'',
''src/ops/ops.num'' changes appropriately to reflect the absence of the
deleted ops.
In my attempt to get ''t/tools/ops2pm/05-renum_op_map_file.t'' DWIMming in
this ticket, the ''Stage 2'' test stanza reflects deletion of ops codes,
while ''Stage 3'' represents addition. The Stage 2 tests pass; the Stage
3 tests don't and are TODO-ed out.
For a long time, I thought the problem lay in my tests and my difficulty
in understanding the underlying code in files such as
''lib/Parrot/OpsFile.pm'' and ''lib/Parrot/Op.pm''. But last night, while
staring at `Parrot::OpsRenumber::renum_op_map_file()` -- the method
underlying all the ops renumbering code -- I came to realize that it
simply had no functionality to add ops codes to ''src/ops/ops.num''.
{{{
for ( @{ $self->{ops}->{OPS} } ) {
if ( defined $fixed{ $_->full_name } ) {
$n = $fixed{ $_->full_name };
}
elsif ( $seen{ $_->full_name } ) {
printf $OP "%-31s%4d\n", $_->full_name, ++$n;
}
}
}}}
The Parrot::OpsFile call which results in `...@{ $self->{ops}->{OPS} }`
''does'' pick up newly added ops codes. But the `if-elsif` code
thereafter accounts only for (a) the 8 ''fixed'' ops codes and (b) those
ops codes '''already found''' in ''src/ops/ops.num''. There's no code
there which says, ''Those new ops codes which you didn't find in ops.num?
Add them at the end of ops.num.''
Apart from noting this in ''t/tools/ops2pm/05-renum_op_map_file.t'', I
conducted a couple of experiments outside the Test::More context. I did a
fresh checkout, created a dummy ''src/ops/ver.ops'' file, edited the
Makefile to add that to `$OPS_FILES`, and called ''make opsrenumber''.
''src/ops/ops.num'' was unchanged; the last opcode was numbered `1248`.
I then killed ''ver.ops'' and temporarily deleted the real ops file
''var.ops'' as well. I then edited the ''Makefile'' to reflect the
deletion of ''var.ops'' from `$OPD_FILES`. In this case,
''src/ops/ops.num'' '''was''' changed: the last opcode was now numbered
`1193`.
I would infer from this that the only thing ''make opsrenumber'' is
accomplishing with respect to addition of opscodes is that, when someone
manually adds opscodes to the end of the file, running ''make
opsrenumber'' assigns new numbers.
My questions are: Is my analysis correct? If so, then my long-held
conception of what ''make opsrenumber'' was doing is wrong. But is that
then what we want ''make opsrenumber'' or ''tools/dev/opsrenumber.pl'' to
be doing? Wouldn't we ''really'' want it to automatically detect newly
added ops codes and add them with appropriate numbers to
''src/ops/ops.num''?
--
Ticket URL: <https://trac.parrot.org/parrot/ticket/469#comment:6>
Parrot <https://trac.parrot.org/parrot/>
Parrot Development
_______________________________________________
parrot-tickets mailing list
[email protected]
http://lists.parrot.org/mailman/listinfo/parrot-tickets
