[m-rev.] for review: fix compiler abort after unresolved overloading

[Julien Fischer]

On Sun, 21 Aug 2022, Zoltan Somogyi wrote:

> This bug fix is relatively straighforward. The part most worth
> looking at are the error message and the additions to list.m.
> If anyone has better names for the new predicates there, speak up.
> I will add a blurb the NEWS about them once we have settled
> on a name.


> Don't abort compilation for unresolved overloading.

...

> diff --git a/compiler/pred_table.m b/compiler/pred_table.m

...

> @@ -1171,9 +1174,71 @@ resolve_pred_overloading(ModuleInfo, CallerMarkers, TVarSet, ExistQTVars,
>
>  %---------------------------------------------------------------------------%
> 
> -find_matching_pred_id(ModuleInfo, [PredId | PredIds], TVarSet, ExistQTVars,
> +find_matching_pred_id(ModuleInfo, PredIds, TVarSet, ExistQTVars,
>          ArgTypes, ExternalTypeParams, MaybeConstraintSearch, Context,
> -        ThePredId, PredName) :-
> +        ThePredId, ThePredSymName, Specs) :-
> +    find_matching_pred_ids(ModuleInfo, TVarSet, ExistQTVars,
> +        ArgTypes, ExternalTypeParams, MaybeConstraintSearch, Context,
> +        PredIds, MatchingPredIdsInfos),
> +    (
> +        MatchingPredIdsInfos = [],
> +        fail
> +    ;
> +        MatchingPredIdsInfos =
> +            [ThePredId - ThePredInfo | TailMatchingPredIdsInfos],
> +        % We have found a matching predicate.
> +        pred_info_get_sym_name(ThePredInfo, ThePredSymName),
> +        % Was there was more than one matching predicate/function?
> +        (
> +            TailMatchingPredIdsInfos = [],
> +            Specs = []
> +        ;
> +            TailMatchingPredIdsInfos = [_ | _],
> +            GetPredDesc =
> +                ( func(_ - PI) = Piece :-
> +                    pred_info_get_pf_sym_name_arity(PI, PFSNA),
> +                    Piece = qual_pf_sym_name_orig_arity(PFSNA)
> +                ),
> +            PredDescPieces = list.map(GetPredDesc, MatchingPredIdsInfos),
> +            add_sep_separators([suffix(","), nl],
> +                [suffix(","), words("and"), nl], PredDescPieces,
> +                PredDescSepPieces),
> +            Pieces = [words("Error: unresolved predicate overloading."), nl,
> +                words("The matches are"), nl_indent_delta(1)] ++
> +                PredDescSepPieces ++ [suffix("."), nl_indent_delta(-1),
> +                words("You need to use an explicit module qualifier"),
> +                words("to select the one you intend to refer to."), nl,
> +                words("Proceeding on the assumption that"),
> +                words("the intended match is the first."),
> +                words("If this assumption is incorrect, other error messages"),
> +                words("may be reported for this predicate or function"),
> +                words("solely because of this wrong assumption."), nl],
> +            % In the frequent case (for us, at least) where the ambiguity
> +            % is caused by moving a predicate from one module of the standard

     standard _library_

> +            % to another but leaving a forwarding predicate behind,
> +            % the choices will be equivalent, and there will be *no* avalanche
> +            % errors caused by our assumption. However, it is better to warn
> +            % about avalanche errors that won't happen than to not warn
> +            % about avalanche errors that *do* happen.
> +            Spec = simplest_spec($pred, severity_error, phase_type_check,
> +                Context, Pieces),
> +            Specs = [Spec]
> +        )
> +    ).

...

> diff --git a/library/list.m b/library/list.m
> index 5942c6c30..69a029de1 100644
> --- a/library/list.m
> +++ b/library/list.m
> @@ -585,6 +585,42 @@
>      %
>  :- pred det_split_last(list(T)::in, list(T)::out, T::out) is det.
> 
> +    % add_separator(Sep, List, ListWithSep):
> +    %
> +    % Insert Sep between each pair of elements in List, and return
> +    % the result as ListWithSep.
> +    %
> +    % For example, add_separator("and", ["jan", "feb", "mar"], ListWithSep)
> +    % will bind ListWithSep to ["jan", "and", "feb", "and", "mar"].
> +    %
> +:- pred add_separator(T::in, list(T)::in, list(T)::out) is det.
> +
> +    % add_separators(Seps, List, ListWithSeps):
> +    %
> +    % Insert Seps between each pair of elements in List, and return
> +    % the result as ListWithSeps.
> +    %
> +    % For example, add_separators(["and", "then"], ["jan", "feb", "mar"],
> +    % ListWithSeps) will bind ListWithSeps to
> +    % ["jan", "and", "then", "feb", "and", "then", "mar"].
> +    %
> +:- pred add_separators(list(T)::in, list(T)::in, list(T)::out) is det.
> +
> +    % add_sep_separators(NonLastSeps, LastSeps, List, ListWithSeps):
> +    %
> +    % Insert NonLastSeps between each pair of elements in List except
> +    % the last pair, insert LastSeps between the last pair of elements,
> +    % and return the result as ListWithSeps. The middle "sep" in the name
> +    % is for the fact that are separate separators with non-last and last
> +    % pairs of elements.
> +    %
> +    % For example, add_separators(["and", "then"], ["and", "finally"],
> +    % ["jan", "feb", "mar"], ListWithSeps) will bind ListWithSeps to
> +    % ["jan", "and", "then", "feb", "and", "finaly", "mar"].
> +    %
> +:- pred add_sep_separators(list(T)::in, list(T)::in, list(T)::in,
> +    list(T)::out) is det.

Some alterantive suggestions for the names of these new predicates:

     interleave
     interleave_list
     interleave_list_last

or:

     add_between
     add_between_list
     add_between_list_last

My own preference would be the the interleave* naming.

The diff looks fine otherwise.

Julien.