[m-rev.] for review: more expressive warning for subtype order mismatch

[Peter Wang]

On Sat, 03 Jul 2021 10:41:36 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by Peter.
> 
> Zoltan.

> Provide more info in a warning.
> 
> compiler/add_type.m:
>     When a subtype lists data constructors in a different order than its
>     supertype, print a diff of the two orders, not just the list of
>     out-of-order constructors.
> 
>     Give some predicates more expressive names.
> 
> compiler/style_checks.m:
> compiler/error_util.m:
>     Move two predicates from style_checks to error_util.m
>     to allow them to be called from add_type.m.
> 
> tests/warnings/subtype_order.exp:
>     Expect the updated warning.

> -compute_subtype_ctors_out_of_order(Ctors, SuperCtors, CtorsOutOfOrder) :-
> +compute_subtype_ctors_out_of_order(Ctors, SuperCtors, OutOfOrderPieces) :-
>      (
> -        Ctors = [],
> -        CtorsOutOfOrder = []
> -    ;
> -        Ctors = [_],
> -        CtorsOutOfOrder = []
> +        ( Ctors = []
> +        ; Ctors = [_]
> +        ),
> +        OutOfOrderPieces = []
>      ;
>          Ctors = [_, _ | _],
> -        list.map(ctor_to_unqual_sym_name_arity, Ctors, CtorNames0),
> -        list.map(ctor_to_unqual_sym_name_arity, SuperCtors, SuperCtorNames),
> -        list.filter(list.contains(SuperCtorNames), CtorNames0, CtorNames),
> +        list.map(ctor_to_string, Ctors, CtorStrs0),
> +        list.map(ctor_to_string, SuperCtors, SuperCtorStrs0),
> +        list.filter(list.contains(SuperCtorStrs0), CtorStrs0, CtorStrs),
> +        list.filter(list.contains(CtorStrs0), SuperCtorStrs0, SuperCtorStrs),
>          EditParams = edit_params(1, 1, 1),
> -        find_shortest_edit_seq(EditParams, SuperCtorNames, CtorNames, EditSeq),
> -        list.filter_map(edit_to_ctor_out_of_order, EditSeq, CtorsOutOfOrder)
> +        find_shortest_edit_seq(EditParams, SuperCtorStrs, CtorStrs, EditSeq),
> +        find_diff_seq(SuperCtorStrs, EditSeq, DiffSeq),
> +        find_change_hunks(3, DiffSeq, CHunks),
> +        list.map(change_hunk_to_pieces, CHunks, OutOfOrderPiecesLists),
> +        list.condense(OutOfOrderPiecesLists, OutOfOrderPieces)
>      ).

CHunks

> @@ -2633,6 +2642,32 @@ get_later_words([Word | Words], OldLen, Avail, Line0, Line, RestWords) :-
>  
>  %---------------------------------------------------------------------------%
>  
> +change_hunk_to_pieces(CHunk, CHunkPieces) :-
> +    CHunk = change_hunk(StartA, LenA, StartB, LenB, Diffs),
> +    string.format("@@ -%d,%d +%d,%d @@",
> +        [i(StartA), i(LenA), i(StartB), i(LenB)], HeaderStr),
> +    HeaderPieces = [fixed(HeaderStr), nl],
> +    list.map(diff_seq_line_to_pieces, Diffs, DiffPieceLists),
> +    list.condense([HeaderPieces | DiffPieceLists], CHunkPieces).

CHunk

> diff --git a/tests/warnings/subtype_order.exp b/tests/warnings/subtype_order.exp
> index 68788f5c7..ca07ed357 100644
> --- a/tests/warnings/subtype_order.exp
> +++ b/tests/warnings/subtype_order.exp
> @@ -1,5 +1,14 @@
> -subtype_order.m:030: Warning: `citrus'/0 declares the following constructors in
> -subtype_order.m:030:   a different order to the supertype `fruit'/0:
> -subtype_order.m:030:     `pomelo'/0,
> -subtype_order.m:030:     `orange'/1,
> -subtype_order.m:030:     `lemon'/0
> +subtype_order.m:030: Warning: `citrus'/0 declares some constructors in a
> +subtype_order.m:030:   different order to its supertype `fruit'/0, as shown by
> +subtype_order.m:030:   this diff against those of the supertype's constructors
> +subtype_order.m:030:   which are present in the subtype:
> +subtype_order.m:030:   
> +subtype_order.m:030:   @@ -1,5 +1,5 @@
> +subtype_order.m:030:   -lemon/0
> +subtype_order.m:030:   +pomelo/0
> +subtype_order.m:030:    lime/0
> +subtype_order.m:030:   +tangelo/0
> +subtype_order.m:030:    orange/1
> +subtype_order.m:030:   -pomelo/0
> +subtype_order.m:030:   -tangelo/0
> +subtype_order.m:030:   +lemon/0

That's fine. The chunk header seems pointless in this example,
but I guess it serves as a separator if there are multiple chunks.

Peter