[m-rev.] for review: implement Mantis 497

[Peter Wang]

On Sun, 05 Apr 2020 02:41:14 +1000 (AEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by Peter, since it is his feature request.
> The diff is with -b.
> 
> Note: this diff makes duplicate warnings in disable_warnings
> scopes into errors, since they can and should be fixed
> regardless of any bootstrapping issues. However, I could be
> persuaded to make them warnings.

That's fine.

> Let clauses with unknown warnings be processed.
> 
> This implements Mantis feature request #497.
> 

> diff --git a/compiler/du_type_layout.m b/compiler/du_type_layout.m
> index 9be93c79d..dd8c05708 100644
> --- a/compiler/du_type_layout.m
> +++ b/compiler/du_type_layout.m
> @@ -1914,7 +1914,7 @@ is_direct_arg_ctor(ComponentTypeMap, TypeCtorModule, TypeStatus,
>      ConsArgs = [ConsArg],
>      expect(unify(ConsArity, 1), $pred, "ConsArity != 1"),
>      ConsArg = ctor_arg(_MaybeFieldName, ArgType, _ArgContext),
> -    type_to_ctor_and_args(ArgType, ArgTypeCtor, ArgTypeCtorArgTypes),
> +    type_to_ctor_and_args(ArgType, ArgTypeCtor, _ArgTypeCtorArgTypes),
>  
>      ConsConsId = sym_name_arity(ConsSymName, ConsArity),
>      ( if
> @@ -1943,7 +1943,7 @@ is_direct_arg_ctor(ComponentTypeMap, TypeCtorModule, TypeStatus,
>              % XXX TYPE_REPN Should be able to delete this test, since it
>              % duplicates one that was done when adding this entry to
>              % ComponentTypeMap.
> -            ArgTypeCtorArgTypes = [],
> +            % ArgTypeCtorArgTypes = [],
>              % XXX We could let this be a subset of the type params, but that
>              % would require the runtime system to be able to handle variables
>              % in the argument type, during unification and comparison

This change is unrelated.

> diff --git a/compiler/parse_goal.m b/compiler/parse_goal.m
> index 8151ac389..2b1322cbe 100644
> --- a/compiler/parse_goal.m
> +++ b/compiler/parse_goal.m
> @@ -311,30 +312,47 @@ parse_non_call_goal(Functor, Args, Context, ContextPieces, MaybeGoal,
>                          NonDuplicateWarnings = [HeadWarning | TailWarnings],
>                          Goal = disable_warnings_expr(Context,
>                              HeadWarning, TailWarnings, SubGoal),
> -                        MaybeGoal = ok1(Goal)
> +                        MaybeGoal = ok2(Goal, WarningSpecs)
>                      ;
>                          NonDuplicateWarnings = [],
> +                        (
> +                            WarningsWarningSpecs = [],
>                              Pieces = cord.list(ContextPieces) ++
>                                  [lower_case_next_if_not_first, words("Error:"),
>                                  words("a"), fixed(Functor), words("scope"),
>                                  words("must list at least one warning."), nl],
>                              Spec = simplest_spec($pred, severity_error,
> -                            phase_term_to_parse_tree, WarningsContext, Pieces),
> -                        MaybeGoal = error1([Spec])
> +                                phase_term_to_parse_tree, WarningsContext,
> +                                Pieces),
> +                            MaybeGoal = error2([Spec | WarningSpecs])
> +                        ;
> +                            WarningsWarningSpecs = [_ | _],
> +                            % We get here if WarningsTerm is a well formed list
> +                            % but contains only elements thaat are *not*

that

> +                            % warning names. Generating the error message
> +                            % immediately above would be misleading, since
> +                            % the user seemingly *did* try to put a warning
> +                            % in the warning list, he/she just failed at it.
> +                            % But we don't have any valid warnings for
> +                            % a disable_warnings scope either. In this case,
> +                            % we just forgo constructing a scope that would
> +                            % have no effect.
> +                            MaybeGoal = ok2(SubGoal, WarningSpecs)
> +                        )

The rest looks fine, thanks.

Can you add a NEWS entry as well?

Peter