[m-dev.] question about infinite recursion warnings

[Zoltan Somogyi]

Just before implementing Mantis 477 for Julien, I had a situation
in which I could have used an infinite recursion warning but
did not get one. It was a stock standard structural induction
on a list, and as usual I copied-and-pasted the clause head
to be the tail recursive call, but forgot to delete the list head,
yielding code like this, which froze my machine for several
minutes (before I could kill it) by running it out of memory:

p([X | Xs], !S) :-
    ...,
    p([X | Xs], !S).

This did not trigger the usual infinite recursion warning because
some of the code inside the ... actually updated !S, so the
second input arg was NOT unchanged between the clause head
and the recursive call.

I implemented a change that loosened the test for an infinite
recursion warning. With this change, the compiler warns that
calls like this, in which every input arg is *either* the same
in the clause head and the call, *or* it is an instance of the same
state variable in both places, will *probably* lead to infinite
recursion.

This diff generates this loosened form of the usual infinite recursion
warning for my test case. However, it does not bootcheck,
because it also generates warnings for some predicates
in the standard library, which --halt-at-warn turns into errors.
The predicates involved are gather_flag_chars in string.parse_util,
and four different versions of simple_merge_renaming_loop
in varset. After I got the warning for gather_flag_chars, in which
all input args are state vars, I modified the heuristic to require
at least one input arg not to be a state var, but the predicates
in varset.m do have such input args.

My question is: should I enable the new warning, either with
or without this extra proviso, anyway? We could avoid the problem
with --halt-at-warn by disabling infinite recursion warnings
either module-wide (in Mercury.options), or just for the calls
involved using the disable_warnings scope. The latter would
require bootstrapping the change to the scope.

Basically, my question boils down to: which is more annoying?
Not getting an infinite recursion warning when you should,
in situations resembling the one above, or getting one when
you shouldn't?

Zoltan.