hi,
it might be a good idea to add a flag to parrot that checks at runtime
whether any deprecated ops are used.
the flag should be turned off by default; some ops like find_global are
used all over the place, so if it would be
turned on, that would give too many warnings.
Having this flag will help people to stop using those ops (the flag can
be added when doing make test), so we
can remove the ops soon.
The flag should check at runtime, not compile time, because the check
should also be possible when running pbc files.
regards,
kjs
I did some more thinking about this. The point is, we don't want the
engine to check all ops being executed whether they are deprecated. This
would make the runcore slower, so that doesn't really motivate to use
the flag (--check-deprecated-ops or whatever).
Another thing: it is undesirable to have some list of deprecated ops
somewhere in the core that should be updated whenever ops are
deprecated, or after a deprecation cycle, ops are removed (the list to
be checked should be cleaned up).
A much better approach, IMHO, would be to add some annotation to the
.ops file.
Now, an instruction is specified as:
op foo(out PMC) {
/* do something */
goto NEXT();
}
If "foo" would be deprecated, just add some tag, for instance like so:
deprecated op foo(out PMC) {
}
The ops2c.pl script should then check for the tag "deprecated", and if
present, add this line to the op body: (or something a like)
if(FLAGS(DEPRECATED_OPS)) { /* or whatever a flag check looks like */
fprintf(stderr, "Warning: instruction 'foo' is deprecated\n");
}
To summarize the advantages of this approach:
1. no messing around with runtime core code
2. the solution is isolated to the point where it should be: the source
of the instruction itself: easy to deprecated ops; easy to remove ops
(no need to change other files)
3. self-documenting .ops files. (is .ops documentation extracted from
the .ops files? -- if so this would be an additional feature, the docs
then can add: "DEPRECATED")
4. Not too hard to implement (although I don't really understand
ops2c.pl, it shouldn't be too hard I guess)
5. No overhead for non-deprecated instructions.
Comments are most certainly welcome,
kjs
Attached, find a patch that allows us to specify a ":deprecated" flag (post op, ala :flow). It
also adds a new parrot warning (configurable with warningson) level called
PARROT_WARNING_DEPRECATED_FLAG. The patch only applies this flag to getclass.
Comments welcome. (Hopefully sooner than it took me to comment on kjs's last send on this
ticket. =-). The one thing I'm not sure about is that I'm just using fprintf as in kjs's original
sample. Could probably stand to actually use parrot IO. I'm also not happy that it doesn't
show the location of the opcode, but I can live with that for now.
You can see the behavior with:
%cat foo.pir
.include 'include/warnings.pasm'
.sub main
$P1 = getclass 'Integer'
say 'ok'
warningson .PARROT_WARNINGS_DEPRECATED_FLAG
$P1 = getclass 'Float'
.end
%./parrot foo.pir
ok
Warning: instruction 'getclass' is deprecated
--
Will "Coke" Coleda
> Attached, find a patch that allows us to specify a ":deprecated" flag (post
> op, ala :flow). It also adds a new parrot warning (configurable with
> warningson) level called PARROT_WARNING_DEPRECATED_FLAG. The patch only
> applies this flag to getclass.
>
> Comments welcome. (Hopefully sooner than it took me to comment on kjs's
> last send on this ticket. =-). The one thing I'm not sure about is that I'm
> just using fprintf as in kjs's original sample. Could probably stand to
> actually use parrot IO. I'm also not happy that it doesn't show the
> location of the opcode, but I can live with that for now.
+1 from me as-is. ParrotIO stuff will likely have to change in the
medium-term anyway, so we'll probably have to modify this code (if we don't
remove the ops altogether) at some point anyway.
-- c
Here's a more extensive version of the patch, marking other ops as
deprecated, and updating DEPRECATED.pod.
However, running this, the new deprecation warnings are triggered during
'make test'. Which is nifty, but not something we want turned on by
default yet, I think.
--
Will "Coke" Coleda
void * unused
use
INTVAL unused
to avoid compiler warnings. =-)
--
Will "Coke" Coleda
This was because when using t/harness, Test::Harness::runtests was adding the '-w' option..
which is usually harmless with both perl and parrot, but since we're adding a level of
warnings we don't want enabled by default for parrot, I've disabled this in r28843.
Rest of the patch applied in r28844.
Closing ticket.
--
Will "Coke" Coleda