<div dir="ltr"><div dir="ltr"> </div><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Fri, 17 Jul 2026 at 10:32, Zoltan Somogyi <<a href="mailto:zoltan.somogyi@runbox.com">zoltan.somogyi@runbox.com</a>> wrote:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> The diff is huge and boring. </blockquote><div> </div><div>Yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The only parts I am seeking feedback on are the parts marked by "XXX DIAG_SPEC" and a comment. The parts marked by "XXX DIAG_SPEC" without a new comment are reminders to myself about minor changes to do after this diff is committed.</blockquote><div> </div><div>> Use {err,warn,info}_spec as subtypes of diag_spec ... ... > compiler/superhomogeneous_lambda.m: > Return one (or more) of {err,warn,info}_specs instead of > diag_specs. Perform the same replacements in data structures. > (In most modules, diag_specs are replaced by *only* err_specs.) > > In modules where the diag_specs have all been replaced > by the same subtype, keep using Specs as (part of) the names > of the variables containing them. In modules that use > more than one of the new subtypes, use ErrSpecs, WarnSpecs > and/or InfoSpecs. > > Consistenly put err_specs before warn_specs, and both > before info_specs. s/Consistenly/Consistently/ ... > diff --git a/compiler/add_pragma_util.m b/compiler/add_pragma_util.m > index 2d1a1f336..c61b80635 100644 > --- a/compiler/add_pragma_util.m > +++ b/compiler/add_pragma_util.m ... > @@ -295,10 +292,13 @@ look_up_pragma_pf_sym_arity(ModuleInfo, IsFullyQualified, FailHandling, > Specs = [] > ; > InformIgnored = yes, > - Severity = > - severity_informational(inform_ignored_pragma_errors), > - Spec = report_ambiguous_pred_or_func(Severity, PragmaName, > - Context, PredOrFunc, SymName, UserArity), > + % XXX DIAG_SPEC > + % This Spec used to have this severity: > + % Severity = > + % severity_informational(inform_ignored_pragma_errors), > + % Should it stay that way? > + Spec = report_ambiguous_pred_or_func(PragmaName, Context, > + PredOrFunc, SymName, UserArity), > Specs = [Spec] Strictly speaking, unrecognised pragmas should be ignored. Practically, since there is only one Mercury implementation, it's probably better to treat them as errors. ... > diff --git a/compiler/check_module_interface.m b/compiler/check_module_interface.m > index 500110a36..b765b69e2 100644 > --- a/compiler/check_module_interface.m > +++ b/compiler/check_module_interface.m > @@ -30,7 +30,7 @@ > % report a warning. > % > :- pred check_module_interface_for_no_exports(globals::in, > - parse_tree_module_src::in, list(diag_spec)::out) is det. > + parse_tree_module_src::in, list(warn_spec)::out) is det. > > %---------------------------------------------------------------------------% > %---------------------------------------------------------------------------% > @@ -63,7 +63,7 @@ check_module_interface_for_no_exports(Globals, ParseTreeModuleSrc, Specs) :- > _IntFIMs, _ImpFIMs, _IntSelfFIMLangs, _ImpSelfFIMLangs, > > TypeCtorCheckedMap, InstCtorCheckedMap, ModeCtorCheckedMap, > - TypeSpecs, InstModeSpecs, > + TypeErrSpecs, TypeWarnSpecs, InstModeErrSpecs, InstModeWarnSpecs, > > IntTypeClasses, IntInstances, IntPredDecls, IntModeDecls, > IntDeclPragmas, IntDeclMarkers, IntPromises, _IntBadClauses, > @@ -133,8 +133,15 @@ check_module_interface_for_no_exports(Globals, ParseTreeModuleSrc, Specs) :- > % a "no exports" warning or not. This means that the warning > % is not really needed *now*; it can be generated later, > % once the complained-about invalid definitions are fixed. > - TypeSpecs = [], > - InstModeSpecs = [], > + TypeErrSpecs = [], > + InstModeErrSpecs = [], > + > + % XXX DIAG_SPECS > + % These two conditions preserve existing behavior, from a time > + % where the parse tree stored co-mingled diag_specs. but > + % I (zs) don't think it should be preserved. I don't think it's worth preserving the old behaviour here. ... > diff --git a/compiler/grab_modules.m b/compiler/grab_modules.m > index b65725ec7..442c9c946 100644 > --- a/compiler/grab_modules.m > +++ b/compiler/grab_modules.m ... > @@ -725,10 +724,11 @@ grab_trans_opt_files(ProgressStream, Globals, TransOptModuleNames, > > read_trans_opt_files(ProgressStream, Globals, TransOptModuleNames, > cord.init, ParseTreeTransOptsCord, > - [], NonBlockingSpecs, [], BlockingSpecs, !IO), > + [], BlockErrSpecs, [], WarnSpecs, !IO), > list.foldl(aug_compilation_unit_add_trans_opt, > cord.list(ParseTreeTransOptsCord), !AugCompUnit), > - module_baggage_add_nonfatal_specs(NonBlockingSpecs, !Baggage), > + % XXX DIAG_SPECS We used to call module_baggage_add_nonfatal_specs. > + module_baggage_add_warnings(WarnSpecs, !Baggage), Looking at the nonfatal errors we have rme_unexpected_term_in_int_or_opt, which seems relevant to reading .trans_opt files. > % XXX why ignore any existing errors? > !Baggage ^ mb_errors := init_read_module_errors, > ... > diff --git a/compiler/make_hlds_passes.m b/compiler/make_hlds_passes.m > index bee01e019..67bc39850 100644 > --- a/compiler/make_hlds_passes.m > +++ b/compiler/make_hlds_passes.m ... > @@ -698,7 +707,9 @@ maybe_warn_include_and_non_include(Globals, ParseTreeModuleSrc, Specs) :- > words("Moving those entities to other modules can reduce"), > words("the number of modules needing to be recompiled"), > words("after a change."), nl], > - Spec = spec($pred, severity_error, phase_pt2h, Context, Pieces), > + % XXX DIAG_SPECS The severity used to be error. > + Severity = severity_warning(warn_include_and_non_include), > + Spec = spec($pred, Severity, phase_pt2h, Context, Pieces), > Specs = [Spec] > else > Specs = [] The old behaviour was surely a mistake. The message says that this is a warning, as does the name of this predicate. > diff --git a/compiler/maybe_error.m b/compiler/maybe_error.m > index 540e72d35..f62eb3f76 100644 > --- a/compiler/maybe_error.m > +++ b/compiler/maybe_error.m ... > @@ -106,36 +111,79 @@ > > %---------------------% > > -:- func get_any_errors1(maybe1(T1)) = list(diag_spec). > -:- func get_any_errors2(maybe2(T1, T2)) = list(diag_spec). > -:- func get_any_errors3(maybe3(T1, T2, T3)) = list(diag_spec). > -:- func get_any_errors4(maybe4(T1, T2, T3, T4)) = list(diag_spec). > -:- func get_any_errors5(maybe5(T1, T2, T3, T4, T5)) = list(diag_spec). > -:- func get_any_errors6(maybe6(T1, T2, T3, T4, T5, T6)) = list(diag_spec). > + % This type, which is used mostly by code that parses goals, > + % allows us to record both > + % - the presence of at least one error, and > + % - the presence of zero or more warnings. > + % The reason why we use tuples is that any readable function symbol > + % we could use would make the lines that construct errors too long. > +:- type err_warn_error == {one_or_more(err_spec), list(warn_spec)}. > > -:- func get_any_errors1el(maybe1el(T1)) = list(diag_spec). > -:- func get_any_errors2el(maybe2el(T1, T2)) = list(diag_spec). > -:- func get_any_errors3el(maybe3el(T1, T2, T3)) = list(diag_spec). > -:- func get_any_errors4el(maybe4el(T1, T2, T3, T4)) = list(diag_spec). > -:- func get_any_errors5el(maybe5el(T1, T2, T3, T4, T5)) = list(diag_spec). > -:- func get_any_errors6el(maybe6el(T1, T2, T3, T4, T5, T6)) = list(diag_spec). > +% XXX DIAG_SPECS > +% The numbers in these names mirror the number of arguments of the okN > +% function symbol, but they do not match the type constructor's arity, > +% due to the warn_specs being implicit. Which is more important? I would say the latter. > +:- type parse_result2(T) == > + maybe2(T, list(warn_spec), err_warn_error). > +:- type parse_result3(T1, T2) == > + maybe3(T1, T2, list(warn_spec), err_warn_error). > +:- type parse_result4(T1, T2, T3) == > + maybe4(T1, T2, T3, list(warn_spec), err_warn_error). > ... > diff --git a/compiler/mercury_compile_args.m b/compiler/mercury_compile_args.m > index 8e53f54f1..a3c0bd2f6 100644 > --- a/compiler/mercury_compile_args.m > +++ b/compiler/mercury_compile_args.m ... > @@ -286,34 +288,37 @@ process_options_std(ProgressStream, DefaultOptionTable, CmdLineArgs, > DefaultOptionTable, ArgsOptionTable, cord.init, _UserData, !IO), > ( > MaybeError = yes(OptionError), > - Specs = report_option_error(OptionError), > - Result = opr_failure(Specs) > + OoMErrSpecs = report_option_error(OptionError), > + Result = opr_failure(OoMErrSpecs, []) > ; > MaybeError = no, > read_options_files_named_in_options_file_option(ProgressStream, > ArgsOptionTable, EnvOptFileVariables0, > - OptFileNonUndefSpecs, OptFileUndefSpecs, !IO), > + OptFileErrSpecs, OptFileUndefWarnSpecs0, !IO), > getopt.lookup_bool_option(ArgsOptionTable, > warn_undefined_options_variables, WarnUndef), > ( > WarnUndef = no, > - OptFileSpecs = OptFileNonUndefSpecs > + OptFileUndefWarnSpecs = [] > ; > WarnUndef = yes, > - OptFileSpecs = OptFileNonUndefSpecs ++ OptFileUndefSpecs > + OptFileUndefWarnSpecs = OptFileUndefWarnSpecs0 > ), > io.environment.get_environment_var_map(EnvVarMap, !IO), > + % XXX DIAG_SPEC we used to include OptFileUndefWarnSpecs in this Not sure about that one; are they written out somewhere else? ... > diff --git a/compiler/parse_dcg_goal.m b/compiler/parse_dcg_goal.m > index 70e3d7ed4..87e769306 100644 > --- a/compiler/parse_dcg_goal.m > +++ b/compiler/parse_dcg_goal.m ... > @@ -505,7 +509,7 @@ parse_dcg_goal_some_all(GoalKind, ArgTerms, Context, ContextPieces, > % to warrant a small amount of code target language code duplication. > % > :- pred parse_dcg_goal_conj(dcg_goal_kind, list(term), > - prog_context, cord(format_piece), maybe2(goal, list(warning_spec)), > + prog_context, cord(format_piece), parse_result2(goal), > prog_varset, prog_varset, counter, counter, prog_var, prog_var). > :- mode parse_dcg_goal_conj(in(dcg_goal_kind_conj), in, in, in, out, > in, out, in, out, in, out) is det. > @@ -518,10 +522,13 @@ parse_dcg_goal_conj(GoalKind, ArgTerms, Context, ContextPieces, > string_dcg_goal_kind(Functor, GoalKind), > ( if ArgTerms = [TermA, TermB] then > parse_dcg_goal_conjunction(Functor, TermA, TermB, Context, > - ContextPieces, cord.init, ConjunctsCord, [], Warnings, [], Errors, > - !VarSet, !Counter, !DCGVar), > + ContextPieces, cord.init, ConjunctsCord, > + [], ErrSpecs, [], WarnSpecs, !VarSet, !Counter, !DCGVar), > ( > - Errors = [], > + ErrSpecs = [], > + % XXX DIAG_SPEC In the past, we used to get here only if > + % parse_dcg_goal_conjunction returned no diag_specs of > + % *any* severity. > Conjuncts = cord.list(ConjunctsCord), The old behaviour seems wrong. parse_goal is called by parse_dcg_goal_conjuction and that can produce warnings. ... > :- pred parse_dcg_goal_semicolon(list(term)::in, > prog_context::in, cord(format_piece)::in, > - maybe2(goal, list(warning_spec))::out, > + parse_result2(goal)::out, > prog_varset::in, prog_varset::out, counter::in, counter::out, > prog_var::in, prog_var::out) is det. > :- pragma inline(pred(parse_dcg_goal_semicolon/10)). > @@ -611,10 +618,13 @@ parse_dcg_goal_semicolon(ArgTerms, Context, ContextPieces, > InitDCGVar = !.DCGVar, > parse_dcg_goal_disjunction(InitDCGVar, ContextPieces, > SubGoalTermA, SubGoalTermB, no, MaybeFirstDiffDCGVar, > - cord.init, DisjunctsDCGVarsCord, [], Warnings, [], ErrorSpecs, > - !VarSet, !Counter), > + cord.init, DisjunctsDCGVarsCord, > + [], ErrSpecs, [], WarnSpecs, !VarSet, !Counter), > ( > - ErrorSpecs = [], > + ErrSpecs = [], > + % XXX DIAG_SPEC In the past, we used to get here only if > + % parse_dcg_goal_disjunction returned no diag_specs of > + % *any* severity. Ditto for this one. ... > diff --git a/compiler/parse_goal.m b/compiler/parse_goal.m > index c40c607a7..5f84f9ddf 100644 > --- a/compiler/parse_goal.m > +++ b/compiler/parse_goal.m ... > @@ -768,10 +772,13 @@ parse_goal_conj(GoalKind, ArgTerms, Context, ContextPieces, > string_goal_kind(Functor, GoalKind), > ( if ArgTerms = [SubGoalTermA, SubGoalTermB] then > parse_goal_conjunction(Functor, SubGoalTermA, SubGoalTermB, > - ContextPieces, cord.init, ConjunctsCord, [], WarningSpecs, > - [], ErrorSpecs, !VarSet), > + ContextPieces, cord.init, ConjunctsCord, > + [], ErrSpecs, [], WarnSpecs, !VarSet), > ( > - ErrorSpecs = [], > + ErrSpecs = [], > + % XXX DIAG_SPEC In the past, we used to get here only if > + % parse_goal_conjunction returned no diag_specs of > + % *any* severity. > Conjuncts = cord.list(ConjunctsCord), As in the above DCG cases, the old behaviour seems wrong. ... > @@ -852,28 +859,36 @@ parse_goal_semicolon(ArgTerms, Context, ContextPieces, MaybeGoal, !VarSet) :- > parse_goal(ElseGoalTerm, ContextPieces, MaybeElseGoal, !VarSet), > ( if > MaybeCondGoal = > - ok4(Vars, StateVars, CondGoal, CondWarningSpecs), > - MaybeThenGoal = ok2(ThenGoal, ThenWarningSpecs), > - MaybeElseGoal = ok2(ElseGoal, ElseWarningSpecs) > + ok4(Vars, StateVars, CondGoal, CondWarnSpecs), > + MaybeThenGoal = ok2(ThenGoal, ThenWarnSpecs), > + MaybeElseGoal = ok2(ElseGoal, ElseWarnSpecs) > then > Goal = if_then_else_expr(Context, Vars, StateVars, > CondGoal, ThenGoal, ElseGoal), > - WarningSpecs = CondWarningSpecs ++ > - ThenWarningSpecs ++ ElseWarningSpecs, > - MaybeGoal = ok2(Goal, WarningSpecs) > + WarnSpecs = CondWarnSpecs ++ > + ThenWarnSpecs ++ ElseWarnSpecs, > + MaybeGoal = ok2(Goal, WarnSpecs) > else > - Specs = get_any_errors_warnings4(MaybeCondGoal) ++ > - get_any_errors_warnings2(MaybeThenGoal) ++ > - get_any_errors_warnings2(MaybeElseGoal), > - det_list_to_one_or_more(Specs, OoMSpecs), > - MaybeGoal = error2(OoMSpecs) > + get_all_errors_warnings4(MaybeCondGoal, > + CondErrSpecs, CondWarnSpecs), > + get_all_errors_warnings2(MaybeThenGoal, > + ThenErrSpecs, ThenWarnSpecs), > + get_all_errors_warnings2(MaybeElseGoal, > + ElseErrSpecs, ElseWarnSpecs), > + ErrSpecs = CondErrSpecs ++ ThenErrSpecs ++ ElseErrSpecs, > + WarnSpecs = CondWarnSpecs ++ ThenWarnSpecs ++ ElseWarnSpecs, > + det_list_to_one_or_more(ErrSpecs, OoMErrSpecs), > + MaybeGoal = error2({OoMErrSpecs, WarnSpecs}) > ) > else > parse_goal_disjunction(SubGoalTermA, SubGoalTermB, ContextPieces, > - cord.init, DisjunctsCord, [], WarningSpecs, [], ErrorSpecs, > + cord.init, DisjunctsCord, [], ErrSpecs, [], WarnSpecs, > !VarSet), > ( > - ErrorSpecs = [], > + ErrSpecs = [], > + % XXX DIAG_SPEC In the past, we used to get here only if > + % parse_goal_disjunction returned no diag_specs of > + % *any* severity. > Disjuncts = cord.list(DisjunctsCord), Ditto here. ... > diff --git a/compiler/parse_item.m b/compiler/parse_item.m > index 18e2ca3da..dc74b1d23 100644 > --- a/compiler/parse_item.m > +++ b/compiler/parse_item.m ... > @@ -1583,13 +1594,15 @@ read_term_to_iom_result(ModuleName, FileName, ReadTermResult, ReadIOMResult, > ReadTermResult = term(VarSet, Term), > counter.allocate(SeqNum, !SeqNumCounter), > parse_item_or_marker(ModuleName, VarSet, Term, item_seq_num(SeqNum), > - MaybeItemOrMarker), > + MaybeItemOrMarker, [], WarnSpecs), > ( > MaybeItemOrMarker = ok1(ItemOrMarker), > + % XXX DIAG_SPEC This ignores WarnSpecs. > ReadIOMResult = read_iom_ok(VarSet, Term, ItemOrMarker) > ; > - MaybeItemOrMarker = error1(Specs), > - ReadIOMResult = read_iom_parse_item_errors(VarSet, Term, Specs) > + MaybeItemOrMarker = error1(OoMErrSpecs), > + ReadIOMResult = read_iom_parse_item_errors(VarSet, Term, > + OoMErrSpecs, WarnSpecs) > ) > ). > ... > diff --git a/compiler/read_modules.m b/compiler/read_modules.m > index 24482f774..dc2c19124 100644 > --- a/compiler/read_modules.m > +++ b/compiler/read_modules.m > @@ -1487,7 +1487,10 @@ record_and_report_missing_timestamp(Globals, FileName, IOError, > words("Smart recompilation will not work."), nl], > Severity = severity_warning(warn_smart_recompilation), > Spec = no_ctxt_spec($pred, Severity, phase_read_files, Pieces), > - add_nonfatal_error(rme_cannot_find_modify_time, Spec, !Errors) > + % XXX DIAG_SPEC This used to be: > + % add_nonfatal_error(rme_cannot_find_modify_time, Spec, !Errors) > + % but this is a warning, not an error. > + add_warning(Spec, !Errors) > ; > Warn = no > ). I don't know about that one. I would leave it marked with an XXX and whoever undertakes the work need to complete smart recompilation can deal with it. </div><div> </div><div>Julien.</div><div> </div></div></div>