[m-rev.] for post commit review: different status types

Julien Fischer jfischer at opturion.com
Fri Sep 18 15:49:55 AEST 2015


Hi,

On Sat, 12 Sep 2015, Zoltan Somogyi wrote:

> A step towards implementing the proposal for fixing the
> status system.

...

> Use separate types for the status of different entity kinds.
> 
> We used the old import_status type to represent the status of six different
> kinds of entities:
> 
> - types
> - insts
> - modes
> - typeclasses
> - instances
> - predicates
> 
> even though some statuses that made sense for one kind of entity didn't for
> another another (e.g. predicates can be pseudo imported/exported, but the
> other five kinds of entities cannot).
> 
> Create the new types type_status, inst_status, ..., pred_status to represent
> the status of these entities in the HLDS. For now, these are just wrappers
> around the renamed old_import_status type, but I plan to replace them with
> status types that *are* specialized to the applicable kind of entity,
> along the lines of compiler/notes/status_proposal. This is a necessary
> first step towards that proposal.
>

...

>  %-----------------------------------------------------------------------------%
> 
> diff --git a/compiler/add_class.m b/compiler/add_class.m
> index 559c3c9..bbaae5a 100644
> --- a/compiler/add_class.m
> +++ b/compiler/add_class.m

...

> +report_any_overlapping_instance_declarations(_, _, _, _, [], !Specs).
> +report_any_overlapping_instance_declarations(ClassId,
> +        NewTypes, NewTVarSet, NewContext,
> +        [OtherInstanceDefn | OtherInstanceDefns], !Specs) :-
> +    OtherInstanceDefn = hlds_instance_defn(_, _, OtherContext,
> +        _, OtherTypes, _, OtherInstanceBody, _, OtherTVarSet, _),
> +    ( if
> +        OtherInstanceBody = instance_body_concrete(_), % XXX
> +        tvarset_merge_renaming(NewTVarSet, OtherTVarSet, _MergedTVarSet,
> +            Renaming),
> +        apply_variable_renaming_to_type_list(Renaming, OtherTypes,
> +            NewOtherTypes),
> +        type_list_subsumes(NewTypes, NewOtherTypes, _)
> +    then
> +        ClassId = class_id(ClassName, ClassArity),
> +        % XXX STATUS Multiply defined if type_list_subsumes in BOTH directions.
> +        NewPieces = [words("Error: multiply defined (or overlapping)"),
> +            words("instance declarations for class"),
> +            sym_name_and_arity(ClassName / ClassArity),
> +            suffix("."), nl],
> +        NewMsg = simple_msg(NewContext, [always(NewPieces)]),
> +        OtherPieces = [words("Previous instance declaration was here.")],
> +        OtherMsg = error_msg(yes(OtherContext), treat_as_first, 0,
> +            [always(OtherPieces)]),
> +        Spec = error_spec(severity_error, phase_parse_tree_to_hlds,
> +            [NewMsg, OtherMsg]),
> +        !:Specs = [Spec | !.Specs]
> +    else
> +        true
> +    ),
> +    % Maybe we shouldn't recursive if we generated an error above,

s/recursive/recurse/ or "make the recursive call".

> +    % but triply-defined instances are so rare that it doesn't much matter,
> +    % and in some even more rare cases, the extra info may be useful.
> +    report_any_overlapping_instance_declarations(ClassId,
> +        NewTypes, NewTVarSet, NewContext, OtherInstanceDefns, !Specs).

That looks fine otherwise.

Julien.



More information about the reviews mailing list