[m-rev.] for review: implement --warn-suspicious-recursion

[Zoltan Somogyi]

On Wed, 1 May 2019 05:34:20 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
> > compiler/simplify_goal_call.m:
> >     If the --warn-suspicious-recursion option is set, and if the warning
> >     isn't disable, generate warnings for two different kinds of suspicious
> 
> s/disable/disabled/

Done.
 
> >     recursion. They are both related to, but separate from, the warning
> >     we have long generated for infinite recursion, which occurs when
> >     the input args of a recursive call are the same as the corresponding
> >     args in the clause head.
> >
> >     Both kinds suspicious recursion look at the input args of a recursive call
> >     that are NOT the same as the corresponding args in the clause head.
> >     Both require these args to have non-unique modes.  (If they have unique
> >     modes, then the depth of the recursion may be controlled by state outside
> >     the view of the Mercury compiler, which means that a warning would be
> >     likely to be misleading.)
> 
> Unless the unique arguments *only* occur in the recursive calls, in which case
> you could issue the warning.  That may be a case worth including here since
> it's not unheard of for users to thread the I/O (or other unique) state through
> a predicate and then not use it.

The code involved in this diff looks at a single call in isolation, so it cannot do
what you ask. Changing this would not be a good idea, because what you ask for
has a natural place elsewhere in the compiler: in unused_args.m.

The problem is that its criterion of what is "unused" is not quite right. Specifically,
it believes an input arg is used if it is used to instantiate an output arg. The typical
base case of a predicate that uses state variable notation copies the input arg to
the output arg, so the !. arg won't be considered unused.

In your scenario, *all* paths through the predicate body return a value for an
output argument (the new I/O state) that the caller already has (the old I/O state).
unused_args.m should not consider any variable whose value is copied to such
an output arg to be used, but it does.

BTW, uniqueness has nothing to do with this; the issue is the passing-around
of state without updating it.

Zoltan.