Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[perl #41606] [TODO] Add flag to do runtime check on deprecated ops

7 views
Skip to first unread message

Klaas-Jan Stol

unread,
Feb 23, 2007, 5:46:17 PM2/23/07
to bugs-bi...@rt.perl.org
# New Ticket Created by Klaas-Jan Stol
# Please include the string: [perl #41606]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=41606 >


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

Klaas-Jan Stol via RT

unread,
Feb 26, 2007, 5:32:10 AM2/26/07
to perl6-i...@perl.org
On Fri Feb 23 14:46:17 2007, parro...@gmail.com wrote:
> 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

Will Coleda via RT

unread,
Jun 26, 2008, 11:02:18 PM6/26/08
to perl6-i...@perl.org

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

deprecated.patch

Chromatic

unread,
Jun 27, 2008, 1:19:44 AM6/27/08
to parrot-...@perl.org, parrotbug...@parrotcode.org, perl6-i...@perl.org
On Thursday 26 June 2008 20:02:18 Will Coleda via RT wrote:

> 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

Will Coleda via RT

unread,
Jun 27, 2008, 11:02:00 AM6/27/08
to perl6-i...@perl.org

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

deprecated.patch

Will Coleda via RT

unread,
Jun 27, 2008, 11:27:34 AM6/27/08
to perl6-i...@perl.org
Minor update: instead of

void * unused

use

INTVAL unused

to avoid compiler warnings. =-)

--
Will "Coke" Coleda

Will Coleda via RT

unread,
Jun 30, 2008, 2:40:51 AM6/30/08
to perl6-i...@perl.org

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

0 new messages