[m-rev.] for post-commit review: fix misleading context in error message for Sean's code

[Julien Fischer]

On Fri, 3 Nov 2023, Zoltan Somogyi wrote:

> For review by anyone. The only thing really worth
> reviewing is an XXX in parse_dcg_goal.m.

...

> Fix if-then-else contexts in DCG clauses.
> 
> compiler/parse_dcg_goal.m:
>     The DCG transformation adds unifications to the then-parts and else-parts
>     of if-then-elses. Use the contexts of the actual user code in these parts
>     for the compiler-generated conjunctions of which they are part.
> 
> tests/invalid/dcg_context.{m,err_exp}:
>     The test case from m-users which motivated this change. The error message
>     generated by the compiler for this code used the same context for
>     both the user-written then-part and the compiler-created else part
>     of an if-then (no else) goal in a DCG clause. The fix makes it clear,
>     in this instance and most others, that when mode analysis complains
>     about a mismatch between the two arms of the if-then-else, it is
>     talking about two separate pieces of code.
> 
> tests/invalid/Mmakefile:
>     Enable the new test case.
> diff --git a/compiler/parse_dcg_goal.m b/compiler/parse_dcg_goal.m
> index 96d9ee8f6..145a63e0e 100644
> --- a/compiler/parse_dcg_goal.m
> +++ b/compiler/parse_dcg_goal.m
> @@ -825,7 +825,7 @@ parse_dcg_goal_if(ArgTerms, Context, ContextPieces,
>              [CondGoalTerm, ThenGoalTerm], _)]
>      then
>          InVar = !.DCGVar,
> -        parse_dcg_if_then(CondGoalTerm, ThenGoalTerm, Context, ContextPieces,
> +        parse_dcg_if_then(CondGoalTerm, ThenGoalTerm, ContextPieces,
>              MaybeVarsCondGoal, MaybeThenGoal, !VarSet, !Counter, !DCGVar),
>          OutVar = !.DCGVar,
>          ( if
> @@ -834,12 +834,20 @@ parse_dcg_goal_if(ArgTerms, Context, ContextPieces,
>              MaybeThenGoal = ok2(ThenGoal, ThenWarningSpecs)
>          then
>              WarningSpecs = CondWarningSpecs ++ ThenWarningSpecs,
> +            % There is else part in the source code, so any context

There is *no* else part.

> +            % we use for it will be misleading. Using Context here is
> +            % also misleading, but any other context from the source code
> +            % will be at least as bad.
> +            % XXX We could use a context that is NOT in the source code.
> +            % For example, we could use the line number from Context,
> +            % but replace the filename with something like "MISSING_ELSE".

I think the only way you are reasonably going to do any better here is by
recording somewhere that the else branch arises from the DCG transformation and
mentioning that fact in the error message.  I suspect the result of your suggestion
in the XXX comment on the part of users will simply be: "Huh, what's that?".

The diff is fine otherwise.

Julien.