[m-rev.] for review: clean up the mode_error type.

[Julien Fischer]

On Wed, 24 Feb 2021, Zoltan Somogyi wrote:

> Clean up the representation of mode errors.
> 
> compiler/mode_errors.m:
>     Give the function symbols representing the various kinds of mode errors
>     more meaningful names. (Some fix misleading implications, such as when
>     a function symbol name includes "pred", but the error applies to functions
>     as well.) Do the same for other function symbols in this module.
>
>     Where arguments are lists that should never be empty, change their type
>     to one_or_more to encode this invariant.
>
>     In one case (var_multimode_pred_error), the same data items were packaged
>     in two different ways in two different places. Remove this unnecessary
>     difference.
>
>     In some cases, put arguments in a more logical order.
>
>     Improve the comments on most of these function symbols, fixing errors,
>     fixing omissions, documenting arguments. Add XXXs where warranted.
>
>     Put the types used in the various kinds of mode errors below the
>     mode_error type itself. Put them into meaningful groups.
>
>     The current order of the mode_error function symbols is totally
>     haphazard. Propose a new order for them, but do not implement it yet,
>     in order to make this diff easier to review.
>
>     Make the representation of the error that says "calling an implied mode
>     for this predicate or function is not implemented" way *why* it is

s/way/say/

>     not implemented. Include this info in the error message we generate.
>     This error message is not exercised by any existing test case, which is
>     why there are no changes to test cases in this diff.
>
>     Improve the explanation for why you cannot unify two functions or
>     two predicates. Since this explanation occurs only in verbose output,
>     no test case is affected by this change either.
>
>     Improve variable names.

...

> diff --git a/compiler/mode_errors.m b/compiler/mode_errors.m
> index 88f7bdcfd..fe2476f70 100644
> --- a/compiler/mode_errors.m
> +++ b/compiler/mode_errors.m

...

> @@ -123,54 +115,145 @@
>                  pred_id, proc_id, list(mode_error_info))
>              % Call to a predicate with initial argument insts for which mode
>              % inference gave a mode error in the callee.
> +            % XXX I (zs) see nothing in either the code that generates
> +            % this kind of error, or in the message we generate for it,
> +            % that restricts it to occur only in the presence of mode
> +            % inference.
> 
> -    ;       mode_error_bind_var(var_lock_reason, prog_var, mer_inst, mer_inst)
> +    ;       mode_error_bind_locked_var(var_lock_reason, prog_var,
> +                mer_inst, mer_inst)
>              % Attempt to bind a non-local variable inside a negated context,
>              % or attempt to re-bind a variable in a parallel conjunct.
> +            % The two insts are the insts of the variable before and after
> +            % *something*, but the code that creates this error is not clear
> +            % on what that "something" is :-( Presumably, the identity of that
> +            % "something" may depend on the reason for the lock.
> 
> -    ;       mode_error_non_local_lambda_var(prog_var, mer_inst)
> +    ;       mode_error_non_ground_non_local_lambda_var(prog_var, mer_inst)
>              % Attempt to pass a live non-ground var as a non-local variable
>              % to a lambda goal.
>
>      ;       mode_error_unify_var_var(prog_var, prog_var, mer_inst, mer_inst)
> -            % Attempt to unify two free variables.
> +            % Attempt to unify two variables whose initial insts *cannot*
> +            % be unified.
> +            % XXX But see also the comment on the code that constructs these
> +            % kinds of errors.
> 
> -    ;       mode_error_unify_var_functor(prog_var, cons_id,
> -                list(prog_var), mer_inst, list(mer_inst))
> -            % Attempt to unify a free var with a functor containing
> -            % free arguments.
> +    ;       mode_error_unify_var_functor(prog_var, cons_id, list(prog_var),
> +                mer_inst, list(mer_inst))
> +            % Attempt to unify a var with a structured term, where their
> +            % initial insts *cannot* be unified.
> +            % XXX But see also the comment in modecheck_unify_functor.
>
>      ;       mode_error_unify_var_lambda(prog_var, mer_inst, mer_inst)
> -            % Some sort of error in attempt to unify a variable with lambda
> -            % expression.
> +            % Attempt to unify a var with a lambda expression, where their
> +            % initial insts *cannot* be unified.
> 
> -    ;       mode_error_unify_var_multimode_pred(prog_var, pred_id,
> -                var_multimode_pred_error)
> -            % Some sort of error in attempt to take address of a multi-moded
> -            % predicate.
> +    ;       mode_error_unify_var_multimode_pf(prog_var,
> +                pred_id_var_multimode_error)
> +            % Attempt to construct a closure using a multi-moded predicate or
> +            % function.
> 
> -    ;       mode_error_conj(list(delayed_goal), schedule_culprit)
> +    ;       mode_error_unschedulable_conjuncts(one_or_more(delayed_goal),
> +                schedule_culprit)
>              % A conjunction contains one or more unscheduleable goals;
>              % schedule_culprit gives the reason why they couldn't be scheduled.
> 
> -    ;       mode_error_final_inst(int, prog_var, mer_inst, mer_inst,
> +    ;       mode_error_unexpected_final_inst(int, prog_var, mer_inst, mer_inst,
>                  final_inst_error)
> -            % One of the head variables did not have the expected final inst
> -            % on exit from the proc.
> +            % The head variables in the given argument position did not have

s/variables/variable/

> +            % the expected final inst on exit from the proc.
> +            % The first inst gives its actual final inst,
> +            % the second inst its expected final inst.
> +            % The last argument specifies the nature of the discrepancy
> +            % between these two insts.
>
>      ;       purity_error_should_be_in_promise_purity_scope(
>                  negated_context_desc, prog_var)
>              % The condition of an if-then-else or the body of a negation
> -            % contained an inst any non-local, but was not inside a
> -            % promise_purity scope.
> +            % contains a nonlocal variable with inst `any', but is not
> +            % inside a promise_purity scope.
> +
> +    ;       purity_error_lambda_should_be_any(one_or_more(prog_var)).
> +            % A ground lambda term contains the given nonlocal variables
> +            % that have inst `any', but is not marked impure.
> +
> +% XXX Proposed new order for the above function symbols,
> +% going from simple to complex and putting related issues together:
> +
> +% errors in var/var unifications

...

> +% purity errors
> +%   purity_error_should_be_in_promise_purity_scope
> +%   purity_error_lambda_should_be_any

That proposed ordering looks fine.

Julien.