[m-rev.] for review: implement Mantis 497
novalazy at gmail.com
Sun Apr 5 11:44:27 AEST 2020
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.
> 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*
> + % 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?
More information about the reviews