[m-rev.] for review: --allow-non-contiguity-for

[Julien Fischer]

On Sat, 31 May 2025 at 21:27, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:

> Add the --allow-non-contiguity-for option.
>
> compiler/options.m:
> doc/user_guide.texi:
> NEWS.md:
>     Add an announce the option.
>
> compiler/style_checks.m:
>     Parse and check the new options.
>
>     Change how we implement the warn-non-contiguous-{clauses,foreign-procs}
>     options. Instead of imemediately generating a diagnostic for every

immediately

...

> diff --git a/compiler/options.m b/compiler/options.m
> index 4852c788d..5ab8ce06c 100644
> --- a/compiler/options.m
> +++ b/compiler/options.m
> @@ -3545,6 +3549,8 @@ long_table("can-fail-function-obsolete-2024-08-10",
>                                      compiler_sufficiently_recent).
>  long_table("unused-statevar-warn-2025-05-16",
>                                      compiler_sufficiently_recent).
> +long_table("allow-non-contig-for-2025-06-01",
> +                                    compiler_sufficiently_recent).
>  long_table("experiment",            experiment).
>  long_table("experiment1",           experiment1).
>  long_table("experiment2",           experiment2).
> @@ -4664,6 +4670,15 @@ options_help_warning = Section :-
>              "Generate a warning if the clauses and foreign_procs of a",
>              "predicate or function are not contiguous."]),
>
> +        help("allow-non-contiguity-for <name1,name2,...>", [
> +            "Allow the clauses of the named predicates and/or functions",
> +            "to be intermingled with each other, but not with the clauses",
> +            "or any other predicates or functions. This option may be",
> +            "specified more than once, with each option value specifying",
> +            "a distinct set of predicates and/or function names",
> +            "that may be intermingled. Each name must uniquely specify",
> +            "a predicate or a function."]),

This should mention that the option also affects foreign_procs.

...

> diff --git a/compiler/style_checks.m b/compiler/style_checks.m
> index 4493c274a..6cb7d3f54 100644
> --- a/compiler/style_checks.m
> +++ b/compiler/style_checks.m

...

> @@ -187,6 +202,142 @@ do_we_want_style_warnings(Globals, DoWeWantStyleWarnings) :-

...

> +:- pred parse_non_contig_name(predicate_table::in, module_name::in, uint::in,
> +    string::in, set(pred_id)::in, set(pred_id)::out,
> +    list(error_spec)::in, list(error_spec)::out) is det.
> +
> +parse_non_contig_name(PredTable, ModuleName, OptNum, Name, !PredIds, !Specs) :-
> +    SymName = qualified(ModuleName, Name),
> +    predicate_table_lookup_sym(PredTable, is_fully_qualified, SymName,
> +        PredIds),
> +    (
> +        PredIds = [],
> +        Pieces =
> +            [words("Error in the"), unth_fixed(OptNum),
> +            quote("--allow-non-contiguity-for"), words("option:"),
> +            words("the name")] ++ color_as_subject([quote(Name)]) ++
> +            [words("is")] ++ color_as_incorrect([words("uknown.")]) ++ [nl],

s/uknown/unknown/

> +        Spec = no_ctxt_spec($pred, severity_error, phase_options, Pieces),
> +        !:Specs = [Spec | !.Specs]
> +    ;
> +        PredIds = [PredId],
> +        set.insert(PredId, !PredIds)
> +    ;

...

> @@ -194,12 +345,23 @@ generate_any_style_warnings(ModuleInfo, WarningsWeWant, !:Specs) :-
>          PredDeclDefnOrder),
>      module_info_get_valid_pred_id_set(ModuleInfo, ValidPredIds),
>      StyleInfo0 = style_info(NonContigDecls, NonContigDefns, PredDeclDefnOrder,
> -        ValidPredIds, [], [], [], []),
> +        ValidPredIds, [], [], [], map.init),
>      module_info_get_pred_id_table(ModuleInfo, PredIdTable),
>      map.foldl(gather_style_info, PredIdTable, StyleInfo0, StyleInfo),
>      StyleInfo = style_info(_, _, _, _, ExportedPreds, NonExportedPreds,
> -        ModeDeclItemNumberSpecs, ClauseGapSpecs),
> -    !:Specs = ModeDeclItemNumberSpecs ++ ClauseGapSpecs,
> +        ModeDeclItemNumberSpecs, ClauseGapMap0),
> +    !:Specs = ModeDeclItemNumberSpecs,
> +    (
> +        NonContigDefns = do_not_warn_non_contiguous_pred_defns
> +    ;
> +        NonContigDefns = warn_non_contiguous_pred_defns(_,
> +            AllowedNonContiguity),
> +        % ZZZ

What's the ZZZ for?

> +        list.foldl2(report_non_contiguous_clauses_beyond_group(ModuleInfo),
> +            AllowedNonContiguity, ClauseGapMap0, ClauseGapMap, !Specs),
> +        map.foldl(report_non_contiguous_clauses(ModuleInfo, []),
> +            ClauseGapMap, !Specs)
> +    ),
>      (
>          PredDeclDefnOrder = do_not_warn_pred_decl_vs_defn_order
>      ;

> @@ -476,35 +638,146 @@ maybe_gather_clause_gap_info(PredInfo, ItemNumbers, !StyleInfo) :-
>              % *if and only if* that malformed clause would cause a gap
>              % in the item number sequence.
>          then
> -            Spec = report_non_contiguous_clauses(PredInfo,
> -                FirstRegion, SecondRegion, LaterRegions),
> -            ClauseGapSpecs0 = !.StyleInfo ^ style_clause_gap_specs,
> -            ClauseGapSpecs = [Spec | ClauseGapSpecs0],
> -            !StyleInfo ^ style_clause_gap_specs := ClauseGapSpecs
> +            GapMap0 = !.StyleInfo ^ style_clause_gaps,
> +            map.det_insert(PredId, RegionsWithGaps, GapMap0, GapMap),
> +            !StyleInfo ^ style_clause_gaps := GapMap
> +            % ZZZ

Likewise.

> +            % Spec = report_non_contiguous_clauses(PredInfo, RegionsWithGaps),
> +            % ClauseGapSpecs0 = !.StyleInfo ^ style_clause_gap_specs,
> +            % ClauseGapSpecs = [Spec | ClauseGapSpecs0],
> +            % !StyleInfo ^ style_clause_gap_specs := ClauseGapSpecs
>          else
>              true

That looks fine otherwise.

Julien.