[m-rev.] for review: workaround an abort in the polymorphism pass
Julien Fischer
jfischer at opturion.com
Thu Mar 17 00:17:33 AEDT 2016
For review by anyone.
The workaround in this diff addresses the symptom rather than the cause
of the problem and the request for a review is get feedback on what the
best approach to the fixing the latter may be. I was thinking of one of
the following:
1. Update the method definitions in the class table when we remove a proc_id from
the pred_info. (Obviously if there are other references in the proc_id in the
HLDS they should be updated too.)
2. Have post_typecheck.m record which pred_proc_ids are invalid (in a similar
fashion to what we do with pred_ids) and have polymorphism only process the
method definitions that correspond to valid pred_proc_ids.
Julien.
------------------------------
Workaround an abort in the polymorphism pass.
post_typecheck.m deletes all but one of the any indistinguishable modes that a
predicate has. For the predicates that the compiler introduces for type class
methods, this means that we end up with dangling references to proc_ids in the
HLDS class table. This causing an abort in the polymorphism pass.
compiler/polymorphism.m:
Workaround the above problem by only expanding method bodies if the
proc_id is present in the proc_table. (This should be safe since
post_typecheck.m is currently the only part of the compiler that
deletes proc_ids.)
compiler/post_typecheck.m:
Add an XXX comment explaining why deleting all but the first
of a set of indistinguishable modes is a bit dodgy.
tests/invalid/Mmakefile:
Enable the typeclass_dup_method_mode test since we now pass it.
tests/invalid/Mercury.options:
Run the typeclass_dup_method_mode test with verbose error messages
enabled.
tests/invalid/typeclass_dup_method_mode.err_exp:
Update the expected output.
diff --git a/compiler/polymorphism.m b/compiler/polymorphism.m
index a1c00f1..f6d4995 100644
--- a/compiler/polymorphism.m
+++ b/compiler/polymorphism.m
@@ -4105,83 +4105,91 @@ expand_class_method_body(hlds_class_proc(PredId, ProcId), !ProcNum,
module_info_get_preds(!.ModuleInfo, PredTable0),
map.lookup(PredTable0, PredId, PredInfo0),
pred_info_get_proc_table(PredInfo0, ProcTable0),
- map.lookup(ProcTable0, ProcId, ProcInfo0),
- % Find which of the constraints on the pred is the one introduced
- % because it is a class method.
- pred_info_get_class_context(PredInfo0, ClassContext),
- ( if ClassContext = constraints([Head | _], _) then
- InstanceConstraint = Head
- else
- unexpected($module, $pred, "class method is not constrained")
- ),
+ % XXX Looking up the proc_info for ProcId can fail here because
+ % post_typecheck.m deletes proc_ids corresponding to indistinguishable
+ % modes from the proc_table but does *not* delete any references to those
+ % proc_ids from the class table.
+ %
+ ( if map.search(ProcTable0, ProcId, ProcInfo0) then
- proc_info_get_rtti_varmaps(ProcInfo0, RttiVarMaps),
- rtti_lookup_typeclass_info_var(RttiVarMaps, InstanceConstraint,
- TypeClassInfoVar),
+ % Find which of the constraints on the pred is the one introduced
+ % because it is a class method.
+ pred_info_get_class_context(PredInfo0, ClassContext),
+ ( if ClassContext = constraints([Head | _], _) then
+ InstanceConstraint = Head
+ else
+ unexpected($module, $pred, "class method is not constrained")
+ ),
- proc_info_get_headvars(ProcInfo0, HeadVars0),
- proc_info_get_argmodes(ProcInfo0, Modes0),
- proc_info_get_declared_determinism(ProcInfo0, MaybeDetism0),
- (
- MaybeDetism0 = yes(Detism)
- ;
- MaybeDetism0 = no,
- % Omitting the determinism for a method is not allowed. But make_hlds
- % will have already detected and reported the error. So here we can
- % just pick some value at random; hopefully something that won't cause
- % flow-on errors. We also mark the predicate as invalid, also to avoid
- % flow-on errors.
- Detism = detism_non,
- module_info_make_pred_id_invalid(PredId, !ModuleInfo)
- ),
+ proc_info_get_rtti_varmaps(ProcInfo0, RttiVarMaps),
+ rtti_lookup_typeclass_info_var(RttiVarMaps, InstanceConstraint,
+ TypeClassInfoVar),
- % Work out which argument corresponds to the constraint which is introduced
- % because this is a class method, then delete it from the list of args to
- % the class_method_call. That variable becomes the "dictionary" variable
- % for the class_method_call. (cf. the closure for a higher order call).
- ( if
- list.index1_of_first_occurrence(HeadVars0, TypeClassInfoVar, N),
- delete_nth(HeadVars0, N, HeadVarsPrime),
- delete_nth(Modes0, N, ModesPrime)
- then
- HeadVars = HeadVarsPrime,
- Modes = ModesPrime
- else
- unexpected($module, $pred, "typeclass_info var not found")
- ),
+ proc_info_get_headvars(ProcInfo0, HeadVars0),
+ proc_info_get_argmodes(ProcInfo0, Modes0),
+ proc_info_get_declared_determinism(ProcInfo0, MaybeDetism0),
+ (
+ MaybeDetism0 = yes(Detism)
+ ;
+ MaybeDetism0 = no,
+ % Omitting the determinism for a method is not allowed. But make_hlds
+ % will have already detected and reported the error. So here we can
+ % just pick some value at random; hopefully something that won't cause
+ % flow-on errors. We also mark the predicate as invalid, also to avoid
+ % flow-on errors.
+ Detism = detism_non,
+ module_info_make_pred_id_invalid(PredId, !ModuleInfo)
+ ),
- InstanceConstraint = constraint(ClassName, InstanceArgs),
- list.length(InstanceArgs, InstanceArity),
- pred_info_get_call_id(PredInfo0, CallId),
- BodyGoalExpr = generic_call(
- class_method(TypeClassInfoVar, !.ProcNum,
- class_id(ClassName, InstanceArity), CallId),
- HeadVars, Modes, arg_reg_types_unset, Detism),
-
- % Make the goal info for the call.
- set_of_var.list_to_set(HeadVars0, NonLocals),
- instmap_delta_from_mode_list(HeadVars0, Modes0, !.ModuleInfo,
- InstmapDelta),
- pred_info_get_purity(PredInfo0, Purity),
- pred_info_get_context(PredInfo0, Context),
- goal_info_init(NonLocals, InstmapDelta, Detism, Purity, Context, GoalInfo),
- BodyGoal = hlds_goal(BodyGoalExpr, GoalInfo),
-
- proc_info_set_goal(BodyGoal, ProcInfo0, ProcInfo),
- map.det_update(ProcId, ProcInfo, ProcTable0, ProcTable),
- pred_info_set_proc_table(ProcTable, PredInfo0, PredInfo1),
- % XXX STATUS
- ( if pred_info_is_imported(PredInfo1) then
- pred_info_set_status(pred_status(status_opt_imported),
- PredInfo1, PredInfo)
- else
- PredInfo = PredInfo1
- ),
+ % Work out which argument corresponds to the constraint which is introduced
+ % because this is a class method, then delete it from the list of args to
+ % the class_method_call. That variable becomes the "dictionary" variable
+ % for the class_method_call. (cf. the closure for a higher order call).
+ ( if
+ list.index1_of_first_occurrence(HeadVars0, TypeClassInfoVar, N),
+ delete_nth(HeadVars0, N, HeadVarsPrime),
+ delete_nth(Modes0, N, ModesPrime)
+ then
+ HeadVars = HeadVarsPrime,
+ Modes = ModesPrime
+ else
+ unexpected($module, $pred, "typeclass_info var not found")
+ ),
- map.det_update(PredId, PredInfo, PredTable0, PredTable),
- module_info_set_preds(PredTable, !ModuleInfo),
+ InstanceConstraint = constraint(ClassName, InstanceArgs),
+ list.length(InstanceArgs, InstanceArity),
+ pred_info_get_call_id(PredInfo0, CallId),
+ BodyGoalExpr = generic_call(
+ class_method(TypeClassInfoVar, !.ProcNum,
+ class_id(ClassName, InstanceArity), CallId),
+ HeadVars, Modes, arg_reg_types_unset, Detism),
+
+ % Make the goal info for the call.
+ set_of_var.list_to_set(HeadVars0, NonLocals),
+ instmap_delta_from_mode_list(HeadVars0, Modes0, !.ModuleInfo,
+ InstmapDelta),
+ pred_info_get_purity(PredInfo0, Purity),
+ pred_info_get_context(PredInfo0, Context),
+ goal_info_init(NonLocals, InstmapDelta, Detism, Purity, Context, GoalInfo),
+ BodyGoal = hlds_goal(BodyGoalExpr, GoalInfo),
+
+ proc_info_set_goal(BodyGoal, ProcInfo0, ProcInfo),
+ map.det_update(ProcId, ProcInfo, ProcTable0, ProcTable),
+ pred_info_set_proc_table(ProcTable, PredInfo0, PredInfo1),
+ % XXX STATUS
+ ( if pred_info_is_imported(PredInfo1) then
+ pred_info_set_status(pred_status(status_opt_imported),
+ PredInfo1, PredInfo)
+ else
+ PredInfo = PredInfo1
+ ),
+ map.det_update(PredId, PredInfo, PredTable0, PredTable),
+ module_info_set_preds(PredTable, !ModuleInfo)
+ else
+ true
+ ),
!:ProcNum = !.ProcNum + 1.
:- pred delete_nth(list(T)::in, int::in, list(T)::out) is semidet.
diff --git a/compiler/post_typecheck.m b/compiler/post_typecheck.m
index 46b4b8d..4b88d31 100644
--- a/compiler/post_typecheck.m
+++ b/compiler/post_typecheck.m
@@ -711,6 +711,11 @@ check_for_indistinguishable_mode(ModuleInfo, PredId, ProcId1,
else
true
),
+ % XXX doing this leaves dangling references the deleted proc_id in the
+ % method definitions in the class table if the predicate being
+ % processed is one of those introduced for type class methods.
+ % See also: the comment above expand_class_method_body/5 in
+ % polymorphism.m.
pred_info_remove_procid(ProcId1, !PredInfo),
Removed = yes
else
diff --git a/tests/invalid/Mercury.options b/tests/invalid/Mercury.options
index ca153d4..46261cb 100644
--- a/tests/invalid/Mercury.options
+++ b/tests/invalid/Mercury.options
@@ -125,6 +125,7 @@ MCFLAGS-require_tailrec_invalid = --allow-stubs --no-warn-stubs
# with the features in the test require_feature_set pragma.
MCFLAGS-test_feature_set = --grade hl.gc --verbose-error-messages
MCFLAGS-tricky_assert1 = --verbose-error-messages
+MCFLAGS-typeclass_dup_method_mode = --verbose-error-messages
MCFLAGS-typeclass_constraint_extra_var = --verbose-error-messages
MCFLAGS-typeclass_missing_det_3 = --verbose-error-messages
MCFLAGS-typeclass_test_11 = --verbose-error-messages
diff --git a/tests/invalid/Mmakefile b/tests/invalid/Mmakefile
index 8a1291a..e06c783 100644
--- a/tests/invalid/Mmakefile
+++ b/tests/invalid/Mmakefile
@@ -255,6 +255,7 @@ SINGLEMODULE= \
typeclass_bad_method_mode \
typeclass_bogus_method \
typeclass_constraint_extra_var \
+ typeclass_dup_method_mode \
typeclass_missing_det \
typeclass_missing_det_2 \
typeclass_missing_det_3 \
@@ -346,7 +347,6 @@ NON_PROFDEEP_MODULES = \
# when doing dynamic linking)
# parent.undeclared_child (just not yet implemented)
# bad_detism (error check not yet implemented)
-# typeclass_dup_method_mode (error check NYI - just calls error)
# XXX we do not currently pass the following tests:
# nonexistent_import (it is unclear whether the new output is OK or not)
diff --git a/tests/invalid/typeclass_dup_method_mode.err_exp b/tests/invalid/typeclass_dup_method_mode.err_exp
index 8d96f4f..0755716 100644
--- a/tests/invalid/typeclass_dup_method_mode.err_exp
+++ b/tests/invalid/typeclass_dup_method_mode.err_exp
@@ -1,4 +1,6 @@
-typeclass_dup_method_mode.m:005: In mode declarations for type class predicate
-typeclass_dup_method_mode.m:005: method `typeclass_dup_method_mode.p/1':
-typeclass_dup_method_mode.m:005: error: duplicate mode declaration.
-typeclass_dup_method_mode.m:006: Here is the conflicting mode declaration.
+typeclass_dup_method_mode.m:010: In mode declarations for type class predicate
+typeclass_dup_method_mode.m:010: method `typeclass_dup_method_mode.p'/1:
+typeclass_dup_method_mode.m:010: error: duplicate mode declaration.
+typeclass_dup_method_mode.m:010: Modes `p(in) is semidet' and `p(in) is det'
+typeclass_dup_method_mode.m:010: are indistinguishable.
+typeclass_dup_method_mode.m:011: Here is the conflicting mode declaration.
More information about the reviews
mailing list