On Mon Feb 26 02:32:10 2007, kjs wrote:
> On Fri Feb 23 14:46:17 2007, [EMAIL PROTECTED] 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
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
Index: src/ops/core.ops
===================================================================
--- src/ops/core.ops (revision 28731)
+++ src/ops/core.ops (working copy)
@@ -974,18 +974,11 @@
Turns on warnings categories. Categories already turned on will stay on.
Initial setting is currently all warnings off. Include F<warnings.pasm> to
-access the categories. They are:
+access the categories. Refer to that file for the current list of warnings
+available.
=over 4
-=item .PARROT_WARNINGS_UNDEF_FLAG
-
-=item .PARROT_WARNINGS_IO_FLAG
-
-=item .PARROT_WARNINGS_PLATFORM_FLAG
-
-=item .PARROT_WARNINGS_ALL_FLAG
-
=back
To turn on multiple categories, OR the category numbers together.
Index: src/ops/object.ops
===================================================================
--- src/ops/object.ops (revision 28731)
+++ src/ops/object.ops (working copy)
@@ -366,7 +366,7 @@
=cut
-inline op getclass(out PMC, in STR) :object_classes :flow {
+inline op getclass(out PMC, in STR) :object_classes :flow :deprecated {
PMC * const _class = Parrot_class_lookup(interp, $2);
opcode_t * const next = expr NEXT();
@@ -376,8 +376,6 @@
$1 = _class;
goto ADDRESS(next);
-/* real_exception(interp, NULL, UNIMPLEMENTED,
- "The 'getclass' opcode has been deprecated, use 'get_class'. See RT
#47972.");*/
}
inline op getclass(out PMC, in PMC) :object_classes :flow {
Index: lib/Parrot/Op.pm
===================================================================
--- lib/Parrot/Op.pm (revision 28731)
+++ lib/Parrot/Op.pm (working copy)
@@ -168,8 +168,6 @@
$name .= "_" . join( "_", @arg_types ) if @arg_types;
- $name = "deprecated_$name" if ( $self->body =~ /DEPRECATED/ );
-
return $name;
}
Index: lib/Parrot/OpsFile.pm
===================================================================
--- lib/Parrot/OpsFile.pm (revision 28731)
+++ lib/Parrot/OpsFile.pm (working copy)
@@ -89,6 +89,10 @@
behaviors. For example, the lack of the C<flow> flag will cause the
op to be implicitly terminated with C<goto NEXT()>. (See next section).
+The :deprecated flag will generate a diagnostic to standard error at
+runtime when a deprecated opcode is invoked. (If and only if
+PARROT_WARNINGS_DEPRECATED_FLAG has been set with the warningson)
+
=back
=head2 Op Body (Macro Substitutions)
@@ -478,6 +482,12 @@
my $next = 0;
my $restart = 0;
+ if (exists($$flags{deprecated})) {
+ $body = <<"END_CODE" . $body;
+void * unused = PARROT_WARNINGS_test(interp,PARROT_WARNINGS_DEPRECATED_FLAG) &&
+ fprintf(stderr,"Warning: instruction '$short_name' is deprecated\\n");
+END_CODE
+}
unless (exists($$flags{flow})) {
$body .= "\ngoto NEXT();";
}
Index: include/parrot/warnings.h
===================================================================
--- include/parrot/warnings.h (revision 28731)
+++ include/parrot/warnings.h (working copy)
@@ -11,12 +11,13 @@
/* Warning flags */
/* &gen_from_enum(warnings.pasm) */
typedef enum {
- PARROT_WARNINGS_ALL_FLAG = 0xFF,
- PARROT_WARNINGS_NONE_FLAG = 0x00,
- PARROT_WARNINGS_UNDEF_FLAG = 0x01,
- PARROT_WARNINGS_IO_FLAG = 0x02,
- PARROT_WARNINGS_PLATFORM_FLAG = 0x04,
- PARROT_WARNINGS_DYNEXT_FLAG = 0x08
+ PARROT_WARNINGS_ALL_FLAG = 0xFF,
+ PARROT_WARNINGS_NONE_FLAG = 0x00,
+ PARROT_WARNINGS_UNDEF_FLAG = 0x01,
+ PARROT_WARNINGS_IO_FLAG = 0x02,
+ PARROT_WARNINGS_PLATFORM_FLAG = 0x04,
+ PARROT_WARNINGS_DYNEXT_FLAG = 0x08,
+ PARROT_WARNINGS_DEPRECATED_FLAG = 0x10
} Warnings_classes;
/* &end_gen */
