[m-rev.] for post commit review: different status types
jfischer at opturion.com
Fri Sep 18 15:49:55 AEST 2015
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).
> + 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.
More information about the reviews