[m-rev.] for post-commit review: Restore a fix to handle_inst_var_subs_2.
Peter Wang
novalazy at gmail.com
Wed Oct 8 16:50:18 AEDT 2014
A previous change improved the precision of inst matching by making
handle_inst_var_subs_2 recurse on the abstract unification of InstA and
SubInstB,
Recurse(InstA, abstractly_unify_inst(InstA, SubInstB))
The change was temporarily reverted because it is susceptible to
infinite recursion when abstractly_unify_inst returns an inst containing
larger and larger "unify_insts" in each successive call.
compiler/inst_match.m:
Restore the fix but avoid infinite recursion.
tests/EXPECT_FAIL_TESTS.all_grades:
Enable invalid/constrained_poly_insts2 again.
diff --git a/compiler/inst_match.m b/compiler/inst_match.m
index 5cb8247..d7e586f 100644
--- a/compiler/inst_match.m
+++ b/compiler/inst_match.m
@@ -495,35 +495,25 @@ handle_inst_var_subs(Recurse, Continue, InstA, InstB, Type, !Info) :-
handle_inst_var_subs_2(Recurse, Continue, InstA, InstB, Type, !Info) :-
( InstB = constrained_inst_vars(InstVarsB, SubInstB) ->
- % InstB is a constrained_inst_var with upper bound SubInstB.
- % We need to check that InstA matches_initial SubInstB and add the
- % appropriate inst_var substitution.
- Recurse(InstA, SubInstB, Type, !Info),
-
- % Call abstractly_unify_inst to calculate the uniqueness of the
- % inst represented by the constrained_inst_var.
- % We pass `Live = is_dead' because we want
- % abstractly_unify(unique, unique) = unique, not shared.
- Live = is_dead,
- ModuleInfo0 = !.Info ^ imi_module_info,
- abstractly_unify_inst(Live, InstA, SubInstB, fake_unify,
- Inst, _Det, ModuleInfo0, ModuleInfo),
- !Info ^ imi_module_info := ModuleInfo,
- update_inst_var_sub(InstVarsB, Inst, Type, !Info)
-
- % This accepts more code but some cases cause infinite recursion.
- /*
% Add the substitution InstVarsB => InstA `glb` SubInstB
% (see get_subst_inst in dmo's thesis, page 78).
+ %
+ % We pass `Live = is_dead' because we want
+ % abstractly_unify(unique, unique) = unique, not shared.
ModuleInfo0 = !.Info ^ imi_module_info,
abstractly_unify_inst(is_dead, InstA, SubInstB, fake_unify,
- Inst, _Det, ModuleInfo0, ModuleInfo),
+ UnifyInst, _Det, ModuleInfo0, ModuleInfo),
!Info ^ imi_module_info := ModuleInfo,
- update_inst_var_sub(InstVarsB, Inst, Type, !Info),
+ update_inst_var_sub(InstVarsB, UnifyInst, Type, !Info),
+
% Check that InstA matches InstB after applying the substitution
% to InstB.
- Recurse(InstA, Inst, Type, !Info)
- */
+ ( UnifyInst = constrained_inst_vars(InstVarsB, UnifySubInst) ->
+ % Avoid infinite regress.
+ Recurse(InstA, UnifySubInst, Type, !Info)
+ ;
+ Recurse(InstA, UnifyInst, Type, !Info)
+ )
; InstA = constrained_inst_vars(_InstVarsA, SubInstA) ->
Recurse(SubInstA, InstB, Type, !Info)
;
diff --git a/tests/EXPECT_FAIL_TESTS.all_grades b/tests/EXPECT_FAIL_TESTS.all_grades
index ebcf18d..febbde7 100644
--- a/tests/EXPECT_FAIL_TESTS.all_grades
+++ b/tests/EXPECT_FAIL_TESTS.all_grades
@@ -4,4 +4,3 @@ valid/bug85
valid/constraint_prop_bug
valid/csharp_hello
valid/ho_and_type_spec_bug2
-invalid/constrained_poly_insts2
More information about the reviews
mailing list