From: Morgan Deters <mdeters@gmail.com>
Date: Tue, 9 Oct 2012 11:58:08 +0000 (+0000)
Subject: fix beta reduction in both preRewrite() *and* postRewrite(), related to bug 417.... 
X-Git-Tag: cvc5-1.0.0~7722
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=bdaa3049467fd17d3fb95701f7946a4bf0f5206a;p=cvc5.git

fix beta reduction in both preRewrite() *and* postRewrite(), related to bug 417.  oops.  also fix spelling on "rewritting" test
---

diff --git a/src/theory/uf/theory_uf_rewriter.h b/src/theory/uf/theory_uf_rewriter.h
index 50211f1ad..15ac0f605 100644
--- a/src/theory/uf/theory_uf_rewriter.h
+++ b/src/theory/uf/theory_uf_rewriter.h
@@ -53,7 +53,23 @@ public:
       for(TNode::iterator formal = lambda[0].begin(), arg = node.begin(); formal != lambda[0].end(); ++formal, ++arg) {
         // typechecking should ensure that the APPLY_UF is well-typed, correct arity, etc.
         Assert(formal != node.end());
-        substitutions.addSubstitution(*formal, *arg);
+        // This rewrite step is important: if we have (f (f 5)) for
+        // some lambda term f, we want to beta-reduce the inside (f 5)
+        // application first.  Otherwise, we can end up in infinite
+        // recursion, because f's formal (say "x") gives the
+        // substitution "x |-> (f 5)".  Fine, the body of the lambda
+        // gets (f 5) in place for x.  But since the same lambda ("f")
+        // now occurs in the body, it's got the same bound var "x", so
+        // substitution continues and we replace that x by (f 5).  And
+        // then again.  :-(
+        //
+        // We need a better solution for distinguishing bound
+        // variables like this, but for now, handle it by going
+        // inside-out.  (Quantifiers shouldn't ever have this problem,
+        // so long as the bound vars in different quantifiers are kept
+        // different.)
+        Node n = Rewriter::rewrite(*arg);
+        substitutions.addSubstitution(*formal, n);
       }
       return RewriteResponse(REWRITE_DONE, substitutions.apply(lambda[1]));
     }
@@ -77,21 +93,7 @@ public:
       for(TNode::iterator formal = lambda[0].begin(), arg = node.begin(); formal != lambda[0].end(); ++formal, ++arg) {
         // typechecking should ensure that the APPLY_UF is well-typed, correct arity, etc.
         Assert(formal != node.end());
-        // This rewrite step is important: if we have (f (f 5)) for
-        // some lambda term f, we want to beta-reduce the inside (f 5)
-        // application first.  Otherwise, we can end up in infinite
-        // recursion, because f's formal (say "x") gives the
-        // substitution "x |-> (f 5)".  Fine, the body of the lambda
-        // gets (f 5) in place for x.  But since the same lambda ("f")
-        // now occurs in the body, it's got the same bound var "x", so
-        // substitution continues and we replace that x by (f 5).  And
-        // then again.  :-(
-        //
-        // We need a better solution for distinguishing bound
-        // variables like this, but for now, handle it by going
-        // inside-out.  (Quantifiers shouldn't ever have this problem,
-        // so long as the bound vars in different quantifiers are kept
-        // different.)
+        // This rewrite step is important: see note in postRewrite().
         Node n = Rewriter::rewrite(*arg);
         substitutions.addSubstitution(*formal, n);
       }
diff --git a/test/regress/regress0/rewriterules/Makefile.am b/test/regress/regress0/rewriterules/Makefile.am
index 3bcf8e62c..f0d45c283 100644
--- a/test/regress/regress0/rewriterules/Makefile.am
+++ b/test/regress/regress0/rewriterules/Makefile.am
@@ -21,7 +21,7 @@ MAKEFLAGS = -k
 TESTS =	\
 	length_trick.smt2 length_trick2.smt2 length_gen_020.smt2 \
 	datatypes.smt2 datatypes_sat.smt2 set_A_new_fast_tableau-base.smt2 \
-	set_A_new_fast_tableau-base_sat.smt2 relation.smt2 simulate_rewritting.smt2 \
+	set_A_new_fast_tableau-base_sat.smt2 relation.smt2 simulate_rewriting.smt2 \
 	reachability_back_to_the_future.smt2 native_arrays.smt2 reachability_bbttf_eT_arrays.smt2
 
 EXTRA_DIST = $(TESTS)
diff --git a/test/regress/regress0/rewriterules/simulate_rewriting.smt2 b/test/regress/regress0/rewriterules/simulate_rewriting.smt2
new file mode 100644
index 000000000..d1d88a549
--- /dev/null
+++ b/test/regress/regress0/rewriterules/simulate_rewriting.smt2
@@ -0,0 +1,24 @@
+;; A new fast tableau-base ... Domenico Cantone et Calogero G.Zarba
+(set-logic AUFLIA)
+(set-info :status sat)
+
+(declare-sort elt1 0)
+(declare-sort elt2 0)
+
+(declare-fun g (elt2) Bool)
+
+(declare-fun p (elt1 elt1) Bool)
+(declare-fun f (elt2) elt1)
+(declare-fun c1 () elt1)
+(declare-fun c2 () elt1)
+
+(assert (forall ((?e elt2)) (! (=> (g ?e) (= (f ?e) c2)) :rewrite-rule)))
+(assert (forall ((?e elt2)) (! (=> (g ?e) (= (f ?e) c1)) :rewrite-rule)))
+
+(declare-fun e () elt2)
+
+(assert (not (=> (g e) (=> (p c1 c2) (p (f e) (f e)))) ))
+
+(check-sat)
+
+(exit)
diff --git a/test/regress/regress0/rewriterules/simulate_rewritting.smt2 b/test/regress/regress0/rewriterules/simulate_rewritting.smt2
deleted file mode 100644
index d1d88a549..000000000
--- a/test/regress/regress0/rewriterules/simulate_rewritting.smt2
+++ /dev/null
@@ -1,24 +0,0 @@
-;; A new fast tableau-base ... Domenico Cantone et Calogero G.Zarba
-(set-logic AUFLIA)
-(set-info :status sat)
-
-(declare-sort elt1 0)
-(declare-sort elt2 0)
-
-(declare-fun g (elt2) Bool)
-
-(declare-fun p (elt1 elt1) Bool)
-(declare-fun f (elt2) elt1)
-(declare-fun c1 () elt1)
-(declare-fun c2 () elt1)
-
-(assert (forall ((?e elt2)) (! (=> (g ?e) (= (f ?e) c2)) :rewrite-rule)))
-(assert (forall ((?e elt2)) (! (=> (g ?e) (= (f ?e) c1)) :rewrite-rule)))
-
-(declare-fun e () elt2)
-
-(assert (not (=> (g e) (=> (p c1 c2) (p (f e) (f e)))) ))
-
-(check-sat)
-
-(exit)