[m-rev.] for review: document require_tail_recursion pragmas

[Julien Fischer]

On Wed, 24 Jun 2026 at 02:57, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
...

> I came up with a design that identifies non-tail-recursive calls
> in the presence of trailing for both backends. Its implementation
> is attached, for your review.

> Make non-tail-recursion reports work with trailing.

...


> diff --git a/compiler/hlds_markers.m b/compiler/hlds_markers.m
> index b1736e9e5..322eaa82a 100644
> --- a/compiler/hlds_markers.m
> +++ b/compiler/hlds_markers.m
> @@ -464,13 +464,20 @@
>              % inserting one or more of these lifted-out deconstructions
>              % before the original switch.
>
> -    ;       feature_lambda_from_try.
> +    ;       feature_lambda_from_try
>              % This lambda goal wraps the main part of a try goal,
>              % the part that does the main job but may throw an exception.
>              % This feaure is used to inform the code that warns about

Existing issue s/feaure/feature/


>              % infinite recursion that the lambda goal *will* be executed
>              % in context in which it is constructed.
>
> +    ;       feature_non_tailrec_reported.
> +            % This goal is a (self or mutual) recursive call that is not
> +            % TAIL recursive, and this fact has already had a report
> +            % generated for it. Do not generate another report.
> +            % (The precise reason for this is explained on the comment

*in* the comment

> +            % on the ntrcr_program in mark_tail_calls.m.)

...

> diff --git a/compiler/mark_tail_calls.m b/compiler/mark_tail_calls.m
> index 00a8f732b..7550ced1d 100644
> --- a/compiler/mark_tail_calls.m
> +++ b/compiler/mark_tail_calls.m

...


> @@ -138,9 +141,39 @@
>  :- pred maybe_override_warn_params_for_proc(proc_info::in,
>      warn_non_tail_rec_params::in, warn_non_tail_rec_params::out) is det.
>
> +%---------------------------------------------------------------------------%
> +
>  :- type nontail_rec_call_reason
> -    --->    ntrcr_program
> -            % The call is not a tail call in the program.
> +    --->    ntrcr_program(set_tree234(later_op))
> +            % The call is not a tail call in the program, because
> +            % it is followed by some operations.
> +            %
> +            % If the set is nonempty, which it always should be when
> +            % we generate a report from inside this module, the elements
> +            % of the set describe the kinds of operations that will happen
> +            % after the call.
> +            %
> +            % If the set is empty, which it always will be when
> +            % we generate a report from outside this module, then we have
> +            % no such information. (Our outside callers are part of the
> +            % MLDS code generator, and its tasks do not include keeping track
> +            % of such things).
> +            %
> +            % The ability of the MLDS code generator to generate diagnostics
> +            % about non-tail recursion is mostly a historical accident.
> +            % We should take away that ability as soon as we are sure that
> +            % all the non-tail recursive calls that the code generator
> +            % would report are reported by this module as well.
> +            %
> +            % However, until that is done, we do NOT way both (a) the code of

s/way/want/

> +            % do_mark_tail_rec_calls_in_proc and (b) the MLDS code generator
> +            % to generate reports for the same non-tail recursive call.
> +            % This is because we add a short explanation for any later_op
> +            % in the set that is not visible (or at least not easily visible)
> +            % to users. The code that writes out error_specs removes any
> +            % duplicates, but it cannot remove *almost* duplicates
> +            % that differ only in the presence vs absence of these
> +            % explanations. We therefore attach
>
>      ;       ntrcr_mlds_in_scc_not_in_tscc
>              % The call is a tail call in the program, but the MLDS code

...


> @@ -547,16 +582,78 @@ find_output_args(ModuleInfo, Types, Modes, Vars, OutputVars) :-
>
>  %---------------------------------------------------------------------------%
>
> -    % Is the current position within the procedure a tail position?
> -    % If it is, what are the output arguments?
> -    %
> -:- type at_tail
> -    --->    at_tail(list(prog_var))
> -    ;       not_at_tail(later_rec_call).
> +:- type at_tail_info
> +    --->    at_tail_info(
> +                % Is the current position within the procedure a tail position?
> +                % The answer is "yes" if-and-only-if this set is empty.
> +                set_tree234(later_op),
>
> -:- type later_rec_call
> -    --->    have_seen_later_rec_call
> -    ;       have_not_seen_later_rec_call.
> +                % If the current position within the procedure a tail position,

*is* a tail position

> +                % then what are the output arguments? These will start out
> +                % as the vector of the procedure's output arguments, but
> +                % if our backwards traversal sees an assignment unification
> +                % to one of these variables, then we replace it in this list
> +                % with the id of the variable it is assigned from.
> +                % (This is because any recursive call must define the
> +                % assigned-from variable, since the mode system ensures
> +                % that the assigned-to variable cannot have as generators
> +                % *both* the recursive call and the assignment unification.)
> +                %
> +                % If the first argument is a nonempty set, then this field
> +                % is meaningless. The reason why we do not have a separate
> +                % function symbol in this type that eliminates this field
> +                % in that event is that we have tried that, and it complicates
> +                % all our code for managing branching goals.
> +                list(prog_var)
> +            ).
> +
> +:- type later_op
> +    --->    later_unify
> +    ;       later_unify_assign
> +    ;       later_nonrec_call
> +    ;       later_rec_call
> +
> +    ;       later_par_join
> +            % Each conjunct in a parallel conjunction is followed by
> +            % the join of the parallel threads.
> +
> +    ;       later_disjunction
> +    ;       later_disjunction_fail
> +    ;       later_switch
> +    ;       later_ite
> +    ;       later_negation
> +            % These mean that a branched goal (a disjunction, a switch,
> +            % or an if-then-else) follows the goal whole initial at_tail_info

s/whole/whose/

> +            % contains one of these later_ops.
> +            %
> +            % Just in case we can use these detaiils later, we distinguish

s/detaiils/details/

> +            % empty disjunctions (later_disjunction_fail) from nonempty ones
> +            % (later_disjunction), and negations (which are effectively
> +            % specials case of if-then-elses) from their general form.

s/specials case/special cases/

> +
> +    ;       later_next_disjunct
> +            % This applies to goals that represent non-last disjuncts.
> +            % Such goals are followed by the code of the next disjunct.
> +
> +    ;       later_cond_end
> +            % This applies to goal that represent an if-then-else's condition.
> +            % Such goals are followed by the code that switches between
> +            % the then part and the else part.
> +
> +    ;       later_negation_end
> +            % This applies to goals inside negations. Such goals are
> +            % followed by the code that flips success to failure or vice versa.

This one does not appear to be used (i.e. in a call to add_later_op/3).

> +
> +    ;       later_commit
> +            % This applies to goals inside scopes that perform commits.
> +            % Such goals are followed by the code that performs that commit.
> +
> +    ;       later_trail_prune.
> +            % This applies to goals that represent disjuncts
> +            % - in a model_det or model_semi disjunction
> +            % - in trailing grades.
> +            % Such goals are followed by an operation that prunes away
> +            % the trail ticket that was created to represent the disjunction.
>
>  :- type call_is_self_or_mutual_rec


...


> @@ -677,7 +771,7 @@ mark_tail_rec_calls_in_goal(Goal0, Goal, AtTail0, AtTail, !Info) :-
>              % the conjunction into a loop control scope, and therefore any
>              % parallel conjunctions we find at *this* point cannot support
>              % tail calls.
> -            not_at_tail(AtTail0, AtTail1)
> +            add_later_op(later_par_join, AtTail0, AtTail1)
>          ),
>          list.reverse(Goals0, RevGoals0),
>          mark_tail_rec_calls_in_rev_conj(RevGoals0, RevGoals, AtTail1, AtTail,
> @@ -685,40 +779,65 @@ mark_tail_rec_calls_in_goal(Goal0, Goal, AtTail0, AtTail, !Info) :-
>          list.reverse(RevGoals, Goals),
>          GoalExpr = conj(ConjType, Goals),
>          Goal = hlds_goal(GoalExpr, GoalInfo0)
> +    ;
> +        % No goal inside a negation can be a *tail* call. However, a negation
> +        % can contain recursive calls about whose non-tail-call nature
> +        % we COULD generate a report.
> +        GoalExpr0 = negation(SubGoal0),
> +        mark_tail_rec_calls_in_goal(SubGoal0, SubGoal,
> +            AtTail0, AtTail1, !Info),

Do you not need to add later_negation_end to AtTail0 here?

The rest looks ok.

Julien.