Discussion:
[m-dev.] proposal: extension to pragma obsolete
Julien Fischer
2014-09-09 07:18:16 UTC
Permalink
Hi,

I think that it would be useful if obsolete pragmas optionally allowed a second
argument that contained a user-defined message that could extend the warning
message that they generate. For example, instead of just:

foo.m: 561: Warning: call to obsolete predicate `bar.baz'/2.

We could instead generate:

foo.m: 561: Warning: call to obsolete predicate `bar.baz'/2.
foo.m: 561: Use predicate `bar.baz_to_int'/2 instead.

The extended variant of the pragma would look something like:

:- pragma obsolete(bar.baz/2, "Use predicate `bar.bat_to_int'/2 instead.").

The main question I have is what should the second argument actually be. There
are three proposals below: (1) use a string literal, (2) use a list of string
literals or (3) make (some of) the functionality of the error_util library
available so we can mark up the warning messages.

(1) :- pragma obsolete(bar.baz/2, "Use predicate `bar.baz_to_int'/2 instead.").
(2) :- pragma obsolete(bar.baz/2, ["Use predicate",
"`bar.baz_to_int'/2 instead"]).
(3) :- pragma obsolete(bar.baz/2, [words("Use predicate"), sym_name_and_arity("bar.baz_to_int", 2),
words("instead.")]).

(3) obviously provides more control over the eventual output, but is also more
complicated for users. (For (1) and (2) the strings would just be turned into
error_util words/1 components.)

Comments?

Cheers,
Julien.
Paul Bone
2014-09-09 07:28:26 UTC
Permalink
Post by Julien Fischer
Hi,
I think that it would be useful if obsolete pragmas optionally allowed a second
argument that contained a user-defined message that could extend the warning
foo.m: 561: Warning: call to obsolete predicate `bar.baz'/2.
foo.m: 561: Warning: call to obsolete predicate `bar.baz'/2.
foo.m: 561: Use predicate `bar.baz_to_int'/2 instead.
:- pragma obsolete(bar.baz/2, "Use predicate `bar.bat_to_int'/2 instead.").
The main question I have is what should the second argument actually be. There
are three proposals below: (1) use a string literal, (2) use a list of string
literals or (3) make (some of) the functionality of the error_util library
available so we can mark up the warning messages.
(1) :- pragma obsolete(bar.baz/2, "Use predicate `bar.baz_to_int'/2 instead.").
(2) :- pragma obsolete(bar.baz/2, ["Use predicate",
"`bar.baz_to_int'/2 instead"]).
(3) :- pragma obsolete(bar.baz/2, [words("Use predicate"), sym_name_and_arity("bar.baz_to_int", 2),
words("instead.")]).
(3) obviously provides more control over the eventual output, but is also more
complicated for users. (For (1) and (2) the strings would just be turned into
error_util words/1 components.)
Comments?
We could support multiple options so that you have the flexability of
option 3. But so that users can trivially just put a string there and move
on with their lives. For example. first try parsing the argument as a
string, and if that fails then try parsing it as a list of error_util items.

I"m not farmiliar with how the front end of the compiler works so parsing a
token into one of two types may be easier said than done.
--
Paul Bone
Peter Wang
2014-09-09 07:52:20 UTC
Permalink
Post by Julien Fischer
The main question I have is what should the second argument actually be. There
are three proposals below: (1) use a string literal, (2) use a list of string
literals or (3) make (some of) the functionality of the error_util library
available so we can mark up the warning messages.
(1) :- pragma obsolete(bar.baz/2, "Use predicate `bar.baz_to_int'/2 instead.").
(2) :- pragma obsolete(bar.baz/2, ["Use predicate",
"`bar.baz_to_int'/2 instead"]).
(3) :- pragma obsolete(bar.baz/2, [words("Use predicate"), sym_name_and_arity("bar.baz_to_int", 2),
words("instead.")]).
I think it's unnecessary, as a comment on the predicate declaration is
fine. Otherwise my vote is for (1) literal string.

Peter
Julien Fischer
2014-09-09 07:56:22 UTC
Permalink
Post by Peter Wang
Post by Julien Fischer
The main question I have is what should the second argument actually be. There
are three proposals below: (1) use a string literal, (2) use a list of string
literals or (3) make (some of) the functionality of the error_util library
available so we can mark up the warning messages.
(1) :- pragma obsolete(bar.baz/2, "Use predicate `bar.baz_to_int'/2 instead.").
(2) :- pragma obsolete(bar.baz/2, ["Use predicate",
"`bar.baz_to_int'/2 instead"]).
(3) :- pragma obsolete(bar.baz/2, [words("Use predicate"), sym_name_and_arity("bar.baz_to_int", 2),
words("instead.")]).
I think it's unnecessary, as a comment on the predicate declaration is
fine.
Experience would suggest that's the one place people never look ... ;-)

Cheers,
Julien.
Peter Wang
2014-09-09 08:19:06 UTC
Permalink
Post by Julien Fischer
Post by Peter Wang
Post by Julien Fischer
The main question I have is what should the second argument actually be. There
are three proposals below: (1) use a string literal, (2) use a list of string
literals or (3) make (some of) the functionality of the error_util library
available so we can mark up the warning messages.
(1) :- pragma obsolete(bar.baz/2, "Use predicate `bar.baz_to_int'/2 instead.").
(2) :- pragma obsolete(bar.baz/2, ["Use predicate",
"`bar.baz_to_int'/2 instead"]).
(3) :- pragma obsolete(bar.baz/2, [words("Use predicate"), sym_name_and_arity("bar.baz_to_int", 2),
words("instead.")]).
I think it's unnecessary, as a comment on the predicate declaration is
fine.
Experience would suggest that's the one place people never look ... ;-)
I don't see why not? If the user doesn't already know the preferred
replacement for an obsolete predicate then the documentation is the
obvious place to look.

Peter
Paul Bone
2014-09-09 08:15:50 UTC
Permalink
Post by Peter Wang
Post by Julien Fischer
The main question I have is what should the second argument actually be. There
are three proposals below: (1) use a string literal, (2) use a list of string
literals or (3) make (some of) the functionality of the error_util library
available so we can mark up the warning messages.
(1) :- pragma obsolete(bar.baz/2, "Use predicate `bar.baz_to_int'/2 instead.").
(2) :- pragma obsolete(bar.baz/2, ["Use predicate",
"`bar.baz_to_int'/2 instead"]).
(3) :- pragma obsolete(bar.baz/2, [words("Use predicate"), sym_name_and_arity("bar.baz_to_int", 2),
words("instead.")]).
I think it's unnecessary, as a comment on the predicate declaration is
fine. Otherwise my vote is for (1) literal string.
Oh, I should have said. I agree with Peter 1 is best as it's the easiest to
use. I also would find this useful.
--
Paul Bone
Julien Fischer
2014-09-09 08:20:57 UTC
Permalink
Post by Paul Bone
Post by Peter Wang
Post by Julien Fischer
The main question I have is what should the second argument actually be. There
are three proposals below: (1) use a string literal, (2) use a list of string
literals or (3) make (some of) the functionality of the error_util library
available so we can mark up the warning messages.
(1) :- pragma obsolete(bar.baz/2, "Use predicate `bar.baz_to_int'/2 instead.").
(2) :- pragma obsolete(bar.baz/2, ["Use predicate",
"`bar.baz_to_int'/2 instead"]).
(3) :- pragma obsolete(bar.baz/2, [words("Use predicate"), sym_name_and_arity("bar.baz_to_int", 2),
words("instead.")]).
I think it's unnecessary, as a comment on the predicate declaration is
fine. Otherwise my vote is for (1) literal string.
Oh, I should have said. I agree with Peter 1 is best as it's the easiest to
use.
My only issue with (1) is that while it's fine for short messages, it's
going to get a bit messy for longer ones.

Cheers,
Julien.
Mark Brown
2014-09-09 11:20:48 UTC
Permalink
Post by Julien Fischer
Post by Paul Bone
On Tue, 9 Sep 2014 17:18:16 +1000 (EST), Julien Fischer
Post by Julien Fischer
The main question I have is what should the second argument actually be.
There
are three proposals below: (1) use a string literal, (2) use a list of string
literals or (3) make (some of) the functionality of the error_util library
available so we can mark up the warning messages.
(1) :- pragma obsolete(bar.baz/2, "Use predicate `bar.baz_to_int'/2 instead.").
(2) :- pragma obsolete(bar.baz/2, ["Use predicate",
"`bar.baz_to_int'/2 instead"]).
(3) :- pragma obsolete(bar.baz/2, [words("Use predicate"),
sym_name_and_arity("bar.baz_to_int", 2),
words("instead.")]).
I think it's unnecessary, as a comment on the predicate declaration is
fine. Otherwise my vote is for (1) literal string.
Oh, I should have said. I agree with Peter 1 is best as it's the easiest to
use.
My only issue with (1) is that while it's fine for short messages, it's
going to get a bit messy for longer ones.
I think it's a lot to ask that users write consistent error messages.

Looking through a recent version of the library, there seems to be two
main cases for pragma obsolete: where a predicate or function is
replaced by something with identical semantics, and where it is
superseded by something with better semantics. Perhaps the pragmas
could be written with optional attributes covering these cases?
Name/arity references could then use the same syntax as in the first
argument. For example:

:- pragma obsolete(hash_table.new/3, [replaced_by(hash_table.init/3)]).
:- pragma obsolete(string.substring/3, [superseded_by(string.between/3)]).

These may well be more informative than the existing library comments
(I had to look closely to see that substring was *not* the same as
between).

Cheers,
Mark.
Julien Fischer
2014-09-09 15:42:25 UTC
Permalink
Hi Mark,
Post by Mark Brown
Looking through a recent version of the library, there seems to be two
main cases for pragma obsolete: where a predicate or function is
replaced by something with identical semantics, and where it is
superseded by something with better semantics.
Actually, there's one other case that sometimes occurs, although not in
the current version of the library, and that's when we deprecate entire
modules (bintree and bintree_set are recent-ish examples). In that case
you probably don't want to say anything about the predicates involved,
but instead, for example, that uses of type 'foo' should be replaced by
uses of type 'bar'.

Cheers,
Julien.
Mark Brown
2014-09-10 13:42:44 UTC
Permalink
Post by Julien Fischer
Hi Mark,
Post by Mark Brown
Looking through a recent version of the library, there seems to be two
main cases for pragma obsolete: where a predicate or function is
replaced by something with identical semantics, and where it is
superseded by something with better semantics.
Actually, there's one other case that sometimes occurs, although not in
the current version of the library, and that's when we deprecate entire
modules (bintree and bintree_set are recent-ish examples). In that case
you probably don't want to say anything about the predicates involved,
but instead, for example, that uses of type 'foo' should be replaced by
uses of type 'bar'.
Sure. These are just symbolic names, and all the proposals so far
could also apply to types or modules. There's still just two cases,
though: either the names mean the same thing or they don't.

Cheers,
Mark.
Zoltan Somogyi
2014-09-09 13:59:20 UTC
Permalink
Post by Mark Brown
Post by Julien Fischer
My only issue with (1) is that while it's fine for short messages, it's
going to get a bit messy for longer ones.
I think it's a lot to ask that users write consistent error messages.
If the second argument of the obsolete pragma is optional, as I think
it should be, then we are not asking them to; we are merely giving them
the ability to do so, if they wish.
Post by Mark Brown
Looking through a recent version of the library, there seems to be two
main cases for pragma obsolete: where a predicate or function is
replaced by something with identical semantics, and where it is
superseded by something with better semantics. Perhaps the pragmas
could be written with optional attributes covering these cases?
Name/arity references could then use the same syntax as in the first
:- pragma obsolete(hash_table.new/3, [replaced_by(hash_table.init/3)]).
:- pragma obsolete(string.substring/3, [superseded_by(string.between/3)]).
These may well be more informative than the existing library comments
(I had to look closely to see that substring was *not* the same as
between).
That is actually an argument for the greater flexibility of Julien's original
proposal: a human-written purpose-specific message could tell you EXACTLY
what the difference between "between" and "substring" was.

Peter: just because it is logical to look there, does not mean that people
actually will :-( If I had a dollar for every time I have watched as a student
in a lab class (for 252, 297 etc) looked at an error message from the
compiler that told him/her exactly what was wrong with their code,
but instead asked me, I would be a rich man. Granted this could
be taken as an argument that trying to enhance the error message
from obsolete pragmas would be futile, but I think we may be justified
in asking a *bit* more from Mercury programmers than from second
year students.

My vote is that we should support both (1) and (3). In answer to Paul's
question: it is quite easy for the parser to handle things which can be
either strings or lists.

As for what subset of format_components would be useful here:
I think we can start with words, fixed, quote, symname_and_arity,
suffix and nl. We can translate them to the error_util equivalents
when parsed, and if we ever find a need for format component
we don't yet support in obsolete declarations, it should be simple
to add one more arm to the string switch on the component name.

Internally, we could also store string second args of obsolete pragmas
using a words() wrapper, making the internal representation
uniform.

Zoltan.
Mark Brown
2014-09-10 13:31:47 UTC
Permalink
On Tue, Sep 9, 2014 at 11:59 PM, Zoltan Somogyi
Post by Zoltan Somogyi
Post by Mark Brown
Post by Julien Fischer
My only issue with (1) is that while it's fine for short messages, it's
going to get a bit messy for longer ones.
I think it's a lot to ask that users write consistent error messages.
If the second argument of the obsolete pragma is optional, as I think
it should be, then we are not asking them to; we are merely giving them
the ability to do so, if they wish.
The emphasis is on consistent. In particular, two library authors
might not mean the same thing when they say "Please use XXX instead";
it could be read as saying that the names are equivalent, or not.
Post by Zoltan Somogyi
Post by Mark Brown
Looking through a recent version of the library, there seems to be two
main cases for pragma obsolete: where a predicate or function is
replaced by something with identical semantics, and where it is
superseded by something with better semantics. Perhaps the pragmas
could be written with optional attributes covering these cases?
Name/arity references could then use the same syntax as in the first
:- pragma obsolete(hash_table.new/3, [replaced_by(hash_table.init/3)]).
:- pragma obsolete(string.substring/3, [superseded_by(string.between/3)]).
These may well be more informative than the existing library comments
(I had to look closely to see that substring was *not* the same as
between).
That is actually an argument for the greater flexibility of Julien's original
proposal: a human-written purpose-specific message could tell you EXACTLY
what the difference between "between" and "substring" was.
I see no reason to think people will leave better messages in pragmas
than they do in comments.

My proposal:
(a) is easy to use
(b) is easy to implement and maintain (compared to (3), at least)
(c) answers the two most important questions for somebody porting
obsolete code, namely
- What are the replacement predicates/functions/types, if any?
- Is a replacement just a name change, or is there a semantic
difference as well?
If the difference is semantic, the user will know that they need to
consult the documentation (or their lab instructor :-().

If the more general case is really wanted, 'comment' and
'verbose_comment' attributes could be added. For example:

:- pragma obsolete(string.substring/3, [
superseded_by(string.between/3),
comment("The new string operations were introduced to help
provide better Unicode support."),
verbose_comment("If Start + Count = End then substring(String,
Start, Count) = between(String, Start, End).")
]).

Cheers,
Mark.
Zoltan Somogyi
2014-09-11 02:35:58 UTC
Permalink
Post by Mark Brown
Post by Zoltan Somogyi
If the second argument of the obsolete pragma is optional, as I think
it should be, then we are not asking them to; we are merely giving them
the ability to do so, if they wish.
The emphasis is on consistent. In particular, two library authors
might not mean the same thing when they say "Please use XXX instead";
it could be read as saying that the names are equivalent, or not.
Now that I see what you mean, I agree. I think what we want is
the best of what you and Julien are proposing: (a) making it compulsory
to say whether the obsolete predicate/function/module has a replacement,
and if yes, whether it has the same semantics, but also (b) making it
possible to expand on that using a message that the writer of
the pragma can control.
Post by Mark Brown
I see no reason to think people will leave better messages in pragmas
than they do in comments.
I agree, but we can hope :-)
Post by Mark Brown
(a) is easy to use
(b) is easy to implement and maintain (compared to (3), at least)
(c) answers the two most important questions for somebody porting
obsolete code, namely
- What are the replacement predicates/functions/types, if any?
- Is a replacement just a name change, or is there a semantic
difference as well?
If the difference is semantic, the user will know that they need to
consult the documentation (or their lab instructor :-().
If the more general case is really wanted, 'comment' and
:- pragma obsolete(string.substring/3, [
superseded_by(string.between/3),
comment("The new string operations were introduced to help
provide better Unicode support."),
verbose_comment("If Start + Count = End then substring(String,
Start, Count) = between(String, Start, End).")
]).
I like this, but I would prefer one change. As you argue, I think
there should be a second argument that expresses one of three things:

(a) there is no replacement for the obsolete entity
(b) there is a direct, semantically identical replacement for the obsolete entity
(c) there is a direct, but semantically different replacement

(b) and (c) would be accompanied by the name of the replacement.

This argument would be optional overall (for backward compatibility),
but would be required if the user wanted to add the third argument,
which would be a general message (string or list of format components),
as per Julien's original proposal. When the pragma is triggered,
this message would be printed AFTER a message that the compiler
would derive from the second argument.

I think this design approach handles all the concerns we have identified
so far.

Zoltan.
Mark Brown
2014-09-11 09:54:13 UTC
Permalink
On Thu, Sep 11, 2014 at 12:35 PM, Zoltan Somogyi
Post by Zoltan Somogyi
Post by Mark Brown
If the more general case is really wanted, 'comment' and
:- pragma obsolete(string.substring/3, [
superseded_by(string.between/3),
comment("The new string operations were introduced to help
provide better Unicode support."),
verbose_comment("If Start + Count = End then substring(String,
Start, Count) = between(String, Start, End).")
]).
I like this, but I would prefer one change. As you argue, I think
(a) there is no replacement for the obsolete entity
(b) there is a direct, semantically identical replacement for the obsolete entity
(c) there is a direct, but semantically different replacement
(b) and (c) would be accompanied by the name of the replacement.
Note also that the user may wish to list more than one potential
replacement, for (c) at least.

Cheers,
Mark.
Paul Bone
2014-09-15 01:47:23 UTC
Permalink
Post by Mark Brown
On Thu, Sep 11, 2014 at 12:35 PM, Zoltan Somogyi
Post by Zoltan Somogyi
Post by Mark Brown
If the more general case is really wanted, 'comment' and
:- pragma obsolete(string.substring/3, [
superseded_by(string.between/3),
comment("The new string operations were introduced to help
provide better Unicode support."),
verbose_comment("If Start + Count = End then substring(String,
Start, Count) = between(String, Start, End).")
]).
I like this, but I would prefer one change. As you argue, I think
(a) there is no replacement for the obsolete entity
(b) there is a direct, semantically identical replacement for the obsolete entity
(c) there is a direct, but semantically different replacement
(b) and (c) would be accompanied by the name of the replacement.
Note also that the user may wish to list more than one potential
replacement, for (c) at least.
Another alternative - I'm not sure if it's a good one - is to makes these
"is it a replacment" items format components. This allows the developer to
easily specify more than one replacement, but it makes it easier for the
developer to provide incomplete, confliciting or misleading information.

I think I prefer Zoltan's proposial with the addition that (c) be allowed to
take a list of replacements.
--
Paul Bone
Julien Fischer
2014-09-15 05:24:53 UTC
Permalink
Post by Mark Brown
On Thu, Sep 11, 2014 at 12:35 PM, Zoltan Somogyi
Post by Zoltan Somogyi
Post by Mark Brown
If the more general case is really wanted, 'comment' and
:- pragma obsolete(string.substring/3, [
superseded_by(string.between/3),
comment("The new string operations were introduced to help
provide better Unicode support."),
verbose_comment("If Start + Count = End then substring(String,
Start, Count) = between(String, Start, End).")
]).
I like this, but I would prefer one change. As you argue, I think
(a) there is no replacement for the obsolete entity
(b) there is a direct, semantically identical replacement for the obsolete entity
(c) there is a direct, but semantically different replacement
(b) and (c) would be accompanied by the name of the replacement.
Note also that the user may wish to list more than one potential
replacement, for (c) at least.
That is precisely the situation that is now present in the standard
library's char module (after commit 696829e6c1cafe86ece8091e36f54f60bcb63076).

Cheers,
Julien.

Mark Brown
2014-09-13 14:51:50 UTC
Permalink
On Thu, Sep 11, 2014 at 12:35 PM, Zoltan Somogyi
Post by Zoltan Somogyi
This argument would be optional overall (for backward compatibility),
but would be required if the user wanted to add the third argument,
which would be a general message (string or list of format components),
as per Julien's original proposal. When the pragma is triggered,
this message would be printed AFTER a message that the compiler
would derive from the second argument.
For any general message argument, I would vote for (1) rather than
(3). I haven't seen any example yet that would warrant the use of
format components, that isn't covered by the specific cases.

Cheers,
Mark.
Loading...