[m-rev.] for review: Enable non-tail-call warnings for?mutually-recursive code

[Paul Bone]

On Wed, Mar 29, 2017 at 03:00:13PM +1100, Zoltan Somogyi wrote:
> 
> 
> On Wed, 29 Mar 2017 14:41:44 +1100, Paul Bone <paul at bone.id.au> wrote:
> > > >  :- type warn_non_tail_rec_calls_opt
> > > >      --->    do_not_warn_non_tail_rec_calls_opt
> > > > -    ;       warn_non_tail_rec_calls_opt.
> > > > +    ;       warn_non_tail_rec_calls_self_opt
> > > > +    ;       warn_non_tail_rec_calls_self_and_mut_opt.
> > > 
> > > I think having two separate flags, one for self and one for mutual
> > > recursion, would be cleaner.
> > 
> > Normally yes, however I don't intend to support creating warnings for mutual
> > recursion but not self recursion.  The issue is raised again below and I'll
> > discuss it there.
> 
> What I am saying is that it is cleaner *internally* to have two separate flags.
> I am *not* saying *anything* about what users see *externally*. Just because
> the internals support a setting of "warn about mutual but not self recursive calls"
> does not mean that this capability has to be visible to users.

I was conflating two things.  Whether we expose it to users.  And whether we
represent what may be an "illegal state" internally.  I suppose three
things, whether that state/combination should be illegal.

I will attempt to make that combination legal, allow it to be represented
internally, but not expose it to users, document it or test it.

> > > > @@ -579,6 +615,8 @@ mark_tail_rec_calls_in_goal(Goal0, Goal, AtTail0, AtTail, !Info) :-
> > > >      (
> > > >          ( GoalExpr0 = call_foreign_proc(_, _, _, _, _, _, _)
> > > >          ; GoalExpr0 = generic_call(_, _, _, _, _)
> > > > +        % Note: we don't give tailcall warnings for negated goals, maybe we
> > > > +        % should?
> > > >          ; GoalExpr0 = negation(_)
> > > >          ),
> > > 
> > > I don't think so. Any code in a negated scope *has* to be followed by the execution
> > > of target language code that switches failure to success and vice versa, so any calls
> > > in such scopes *inherently* cannot be tail recursive.
> > 
> > Yes.  I guess the question is, do people who use Mercury, but don't
> > implement it, know this?  Many will once they give it some thought, but some
> > won't.  Likewise I once surprised Peter Schachte with this:
> > 
> >     p(X::out, Y::out) :-
> >         ...,
> >         p(Y, X).
> > 
> > He assumed it was tail recursive, when it isn't.  This case is more subtle,
> > and one that _is_ tail recursive in Prolog, but perhaps people could make
> > a similar error with negation.
> 
> That sounds like it argues for another option to control what calls the compiler
> should consider to be "obviously not tail calls", which it shouldn't warn about.
> We already a "have we seen a recursive call in our backward traversal so far"
> flag specifically for this purpose; maybe we should make its operation,
> and the treatment of negated scopes and calls where the output args don't match up,
> subject to this option. However, that is definitely a separate change.

I think that the compiler does currently warn for cases where the argument
order doesn't match.

Allowing developers to fine tuning these may help, particularly for
experienced Mercury people who may wish for ways to make the warnings less
numerous.  The YesLogic team have expressed some confusion about what I
considered obvious and not obvious, so it's worth considering
(separately).

Regarding obvious and not-obvious tail calls, I wrote a blog article about
this recently 

http://paul.bone.id.au/2017/03/16/tail-recursion/

and will be giving the same material as a presentation to the
Melbourne Functional Users Group on Thursday April 6th

https://www.meetup.com/Melbourne-Functional-User-Group-MFUG/events/238359521/



-- 
Paul Bone
http://paul.bone.id.au