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

Zoltan Somogyi zoltan.somogyi at runbox.com
Sun Jun 1 17:14:35 AEST 2025



On Sun, 1 Jun 2025 14:59:17 +1000, Julien Fischer <jfischer at opturion.com> wrote:
> > @@ -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?

A reminder to add a comment, which I have now done.
 
> > @@ -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.

That was a reminder to delete the following commented out code 
if it turned out not to be needed. I deleted it.

> That looks fine otherwise.

Thank you. I followed all your other suggestions as well.

Zoltan.


More information about the reviews mailing list