[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