Ensure that expand definitions is called on all non-variable expressi… (#1070)
authorAndrew Reynolds <andrew.j.reynolds@gmail.com>
Sun, 10 Sep 2017 06:12:34 +0000 (08:12 +0200)
committerGitHub <noreply@github.com>
Sun, 10 Sep 2017 06:12:34 +0000 (08:12 +0200)
* Ensure that expand definitions is called on all non-variable expressions. In particular, it is necessary that the sets theory is notified when a set universe term occurs in the input to ensure options are set correctly. The commit moves this check from within check() to expandDefinitions(), and also adds the check for join image which relies on universe set. This fixes a bug reported by Arjun. Add and update regressions.

* Add comments concerning expandDefinitions

* Expand comment, move to .h

16 files changed:
src/smt/smt_engine.cpp
src/theory/sets/theory_sets.cpp
src/theory/sets/theory_sets.h
src/theory/sets/theory_sets_private.cpp
src/theory/sets/theory_sets_private.h
src/theory/theory.h
test/regress/regress0/fmf/quant_real_univ.cvc
test/regress/regress0/rels/joinImg_0.cvc
test/regress/regress0/rels/joinImg_0_1.cvc
test/regress/regress0/rels/joinImg_1.cvc
test/regress/regress0/rels/joinImg_1_1.cvc
test/regress/regress0/rels/joinImg_2.cvc
test/regress/regress0/rels/joinImg_2_1.cvc
test/regress/regress0/sets/Makefile.am
test/regress/regress0/sets/arjun-set-univ.cvc [new file with mode: 0644]
test/regress/regress0/sets/univ-set-uf-elim.smt2 [new file with mode: 0644]

index f360ae2fdf0764181ff18378a27d6d493bff5373..b2f7d6ccce9159570c22ce408dbdfb6e602f1a1f 100644 (file)
@@ -2277,8 +2277,8 @@ Node SmtEnginePrivate::expandDefinitions(TNode n, unordered_map<Node, Node, Node
     if(!childrenPushed) {
       Kind k = n.getKind();
 
-      // Apart from apply, we can short circuit leaves
-      if(k != kind::APPLY && n.getNumChildren() == 0) {
+      // we can short circuit (variable) leaves
+      if(n.isVar()) {
         SmtEngine::DefinedFunctionMap::const_iterator i = d_smt.d_definedFunctions->find(n);
         if(i != d_smt.d_definedFunctions->end()) {
           // replacement must be closed
@@ -2372,15 +2372,6 @@ Node SmtEnginePrivate::expandDefinitions(TNode n, unordered_map<Node, Node, Node
         node = t->expandDefinition(req, n);
       }
 
-      // there should be children here, otherwise we short-circuited a result-push/continue, above
-      if (node.getNumChildren() == 0) {
-        Debug("expand") << "Unexpectedly no children..." << node << endl;
-      }
-      // This invariant holds at the moment but it is concievable that a new theory
-      // might introduce a kind which can have children before definition expansion but doesn't
-      // afterwards.  If this happens, remove this assertion.
-      Assert(node.getNumChildren() > 0);
-
       // the partial functions can fall through, in which case we still
       // consider their children
       worklist.push(make_triple(Node(n), node, true));            // Original and rewritten result
@@ -2394,22 +2385,24 @@ Node SmtEnginePrivate::expandDefinitions(TNode n, unordered_map<Node, Node, Node
       // Reconstruct the node from it's (now rewritten) children on the stack
 
       Debug("expand") << "cons : " << node << endl;
-      //cout << "cons : " << node << endl;
-      NodeBuilder<> nb(node.getKind());
-      if(node.getMetaKind() == kind::metakind::PARAMETERIZED) {
-        Debug("expand") << "op   : " << node.getOperator() << endl;
-        //cout << "op   : " << node.getOperator() << endl;
-        nb << node.getOperator();
-      }
-      for(size_t i = 0; i < node.getNumChildren(); ++i) {
-        Assert(!result.empty());
-        Node expanded = result.top();
-        result.pop();
-        //cout << "exchld : " << expanded << endl;
-        Debug("expand") << "exchld : " << expanded << endl;
-        nb << expanded;
+      if(node.getNumChildren()>0) {
+        //cout << "cons : " << node << endl;
+        NodeBuilder<> nb(node.getKind());
+        if(node.getMetaKind() == kind::metakind::PARAMETERIZED) {
+          Debug("expand") << "op   : " << node.getOperator() << endl;
+          //cout << "op   : " << node.getOperator() << endl;
+          nb << node.getOperator();
+        }
+        for(size_t i = 0; i < node.getNumChildren(); ++i) {
+          Assert(!result.empty());
+          Node expanded = result.top();
+          result.pop();
+          //cout << "exchld : " << expanded << endl;
+          Debug("expand") << "exchld : " << expanded << endl;
+          nb << expanded;
+        }
+        node = nb;
       }
-      node = nb;
       cache[n] = n == node ? Node::null() : node;           // Only cache once all subterms are expanded
       result.push(node);
     }
index 4992f654fa7b4cf22ec9d8fd4fa1878bdbf15eae..263d619342f41e1aaa2a5886290c9d5c7ae28556 100644 (file)
@@ -74,6 +74,10 @@ void TheorySets::preRegisterTerm(TNode node) {
   d_internal->preRegisterTerm(node);
 }
 
+Node TheorySets::expandDefinition(LogicRequest &logicRequest, Node n) {
+  return d_internal->expandDefinition(logicRequest, n);
+}
+
 Theory::PPAssertStatus TheorySets::ppAssert(TNode in, SubstitutionMap& outSubstitutions) {
   return d_internal->ppAssert( in, outSubstitutions );
 }
index bbc6ae0c4c8170a5ac6b013c919d8dc5d5075143..3ecd404bbb0062dcec32bc668afc99476453407b 100644 (file)
@@ -66,6 +66,8 @@ public:
 
   void preRegisterTerm(TNode node);
 
+  Node expandDefinition(LogicRequest &logicRequest, Node n);
+
   PPAssertStatus ppAssert(TNode in, SubstitutionMap& outSubstitutions);
 
   void presolve();
index a2a56e1371b0cce2055ed7ccf6351fc2e7efeacb..3013711eba68522998760db4290baf4e2eb50b74 100644 (file)
@@ -604,13 +604,8 @@ void TheorySetsPrivate::fullEffortCheck(){
           }else if( n.getKind()==kind::EMPTYSET ){
             d_eqc_emptyset[tnn] = eqc;
           }else if( n.getKind()==kind::UNIVERSE_SET ){
-            if( options::setsExt() ){
-              d_eqc_univset[tnn] = eqc;
-            }else{
-              std::stringstream ss;
-              ss << "Symbols complement and universe set are not supported in default mode, try --sets-ext." << std::endl;
-              throw LogicException(ss.str());
-            }
+            Assert( options::setsExt() );
+            d_eqc_univset[tnn] = eqc;
           }else{
             Node r1 = d_equalityEngine.getRepresentative( n[0] );
             Node r2 = d_equalityEngine.getRepresentative( n[1] );
@@ -2155,8 +2150,20 @@ void TheorySetsPrivate::preRegisterTerm(TNode node)
   }
 }
 
+Node TheorySetsPrivate::expandDefinition(LogicRequest &logicRequest, Node n) {
+  Debug("sets-proc") << "expandDefinition : " << n << std::endl;
+  if( n.getKind()==kind::UNIVERSE_SET || n.getKind()==kind::COMPLEMENT || n.getKind()==kind::JOIN_IMAGE ){
+    if( !options::setsExt() ){
+      std::stringstream ss;
+      ss << "Extended set operators are not supported in default mode, try --sets-ext.";
+      throw LogicException(ss.str());
+    }
+  }
+  return n;
+}
 
 Theory::PPAssertStatus TheorySetsPrivate::ppAssert(TNode in, SubstitutionMap& outSubstitutions) {
+  Debug("sets-proc") << "ppAssert : " << in << std::endl;
   Theory::PPAssertStatus status = Theory::PP_ASSERT_STATUS_UNSOLVED;
   
   //TODO: allow variable elimination for sets when setsExt = true
index 7bc17c9278496b6d6818066f9b8a24f0af5540e5..175e82bb8a8a9792a81d660cab9273b104c7af8f 100644 (file)
@@ -188,6 +188,38 @@ public:
 
   void preRegisterTerm(TNode node);
 
+  /** expandDefinition
+   * If the sets-ext option is not set and we have an extended operator, 
+   * we throw an exception. This function is a no-op otherwise.
+   *
+   * This is related to github issue #1076
+   * TheorySets uses expandDefinition as an entry point to see if the input
+   * contains extended operators.
+   *
+   * We need to be notified when a universe set occurs in our input,
+   * before preprocessing and simplification takes place. Otherwise the models
+   * may be inaccurate when setsExt is false, here is an example:
+   *
+   * x,y : (Set Int)
+   * x = (as univset (Set Int));
+   * 0 in y;
+   * check-sat;
+   *
+   * If setsExt is enabled, the model value of (as univset (Set Int)) is always accurate.
+   *
+   * If setsExt is not enabled, the following can happen for the above example:
+   * x = (as univset (Set Int)) is made into a model substitution during 
+   * simplification. This means (as univset (Set Int)) is not a term in any assertion, 
+   * and hence we do not throw an exception, nor do we infer that 0 is a member of 
+   * (as univset (Set Int)). We instead report a model where x = {}. The correct behavior 
+   * is to throw an exception that says universe set is not supported when setsExt disabled.
+   * Hence we check for the existence of universe set before simplification here.
+   *
+   * Another option to fix this is to make TheoryModel::getValue more general
+   * so that it makes theory-specific calls to evaluate interpreted symbols.
+   */
+  Node expandDefinition(LogicRequest &logicRequest, Node n);
+
   Theory::PPAssertStatus ppAssert(TNode in, SubstitutionMap& outSubstitutions);
   
   void presolve();
index 73102a6e20a0d284cc01c3b200c803d30e01a537..1a72944fff854b6cbdf54387d42a95d766f8d6d1 100644 (file)
@@ -424,6 +424,12 @@ public:
    * used, definitions should only be used when rewrites are not
    * possible, for example in handling under-specified operations
    * using partially defined functions.
+   *
+   * TODO (github issue #1076): 
+   * some theories like sets use expandDefinition as a "context
+   * independent preRegisterTerm".  This is required for cases where
+   * a theory wants to be notified about a term before preprocessing
+   * and simplification but doesn't necessarily want to rewrite it.
    */
   virtual Node expandDefinition(LogicRequest &logicRequest, Node node) {
     // by default, do nothing
index c3cefd7675b2d952b6aa267ca1ff21b9407a29a0..c3dbb2cd612e633bdec6dd281f880377865a0ed4 100644 (file)
@@ -1,5 +1,6 @@
 % EXPECT: sat\r
 OPTION "fmf-bound";\r
+OPTION "sets-ext";\r
 Atom : TYPE;\r
 REAL_UNIVERSE : SET OF [REAL];\r
 ATOM_UNIVERSE : SET OF [Atom];\r
index e36c6a647a791ca3f317b4615377d8db4159a432..297898a8167bfa1a58fdd703d7642924bf6353b0 100644 (file)
@@ -1,5 +1,6 @@
 % EXPECT: unsat
 OPTION "logic" "ALL_SUPPORTED";
+OPTION "sets-ext";
 IntPair: TYPE = [INT, INT];
 x : SET OF IntPair;
 y : SET OF IntPair;
index 602c4fe3f1080137680f397142ef4ad9123b5f90..4e69394bdb2af582bce12ef5c76882a498b5e8c7 100644 (file)
@@ -1,5 +1,6 @@
 % EXPECT: sat
 OPTION "logic" "ALL_SUPPORTED";
+OPTION "sets-ext";
 IntPair: TYPE = [INT, INT];
 x : SET OF IntPair;
 y : SET OF IntPair;
index 47e57c1fb15cc462422c1dc8810b4461c459505e..81f208fc43ae5c82bdab39fe3c81ecf1ee955cca 100644 (file)
@@ -1,5 +1,6 @@
 % EXPECT: unsat
 OPTION "logic" "ALL_SUPPORTED";
+OPTION "sets-ext";
 Atom: TYPE;
 x : SET OF [Atom, Atom];
 y : SET OF [Atom, Atom];
index a7927f7e25b47b4f07ccc82c2417629825dc4a58..003770a1b1d1c7455ebb882101781eb75011b478 100644 (file)
@@ -1,5 +1,6 @@
 % EXPECT: sat
 OPTION "logic" "ALL_SUPPORTED";
+OPTION "sets-ext";
 Atom: TYPE;
 x : SET OF [Atom, Atom];
 y : SET OF [Atom, Atom];
index bbf57629bbb5ff24e1264a2a5330feb46458f56f..a4acfe6c6127083ba22bef4615dc9d8bfb65e06e 100644 (file)
@@ -1,5 +1,6 @@
 % EXPECT: unsat
 OPTION "logic" "ALL_SUPPORTED";
+OPTION "sets-ext";
 Atom: TYPE;
 x : SET OF [Atom, Atom];
 y : SET OF [Atom, Atom];
index b38bfab067957503515ad9ad6ce5807aa237204e..03f88be37c35b5fa5b5f0dfe48243c00c579ad59 100644 (file)
@@ -1,5 +1,6 @@
 % EXPECT: sat
 OPTION "logic" "ALL_SUPPORTED";
+OPTION "sets-ext";
 Atom: TYPE;
 x : SET OF [Atom, Atom];
 y : SET OF [Atom, Atom];
@@ -21,4 +22,4 @@ ASSERT y = {(f, g), (b, c), (d, e), (c, e)};
 ASSERT x = {(f, g), (b, c), (d, e), (c, e)};
 ASSERT (NOT(a = b)) OR (NOT(a = f));
 
-CHECKSAT;
\ No newline at end of file
+CHECKSAT;
index c8e416a42e59c539ce447c17767a7d574792f520..de21707684e2ac423f15253e5f8c23a23444b102 100644 (file)
@@ -86,7 +86,9 @@ TESTS =       \
        sets-poly-nonint.smt2 \
        int-real-univ.smt2 \
        int-real-univ-unsat.smt2 \
-       sets-tuple-poly.cvc
+       sets-tuple-poly.cvc \
+       arjun-set-univ.cvc \
+       univ-set-uf-elim.smt2
 
 EXTRA_DIST = $(TESTS)
 
diff --git a/test/regress/regress0/sets/arjun-set-univ.cvc b/test/regress/regress0/sets/arjun-set-univ.cvc
new file mode 100644 (file)
index 0000000..3c35a62
--- /dev/null
@@ -0,0 +1,8 @@
+% EXPECT: Extended set operators are not supported in default mode, try --sets-ext.
+% EXIT: 1
+OPTION "produce-models" true;
+x,y,z : SET OF BOOLEAN;
+ASSERT x = {TRUE};
+ASSERT y = {FALSE};
+CHECKSAT z = ~ y;
+COUNTERMODEL;
diff --git a/test/regress/regress0/sets/univ-set-uf-elim.smt2 b/test/regress/regress0/sets/univ-set-uf-elim.smt2
new file mode 100644 (file)
index 0000000..a22f2a4
--- /dev/null
@@ -0,0 +1,16 @@
+; EXPECT: (error "Extended set operators are not supported in default mode, try --sets-ext.")
+; EXIT: 1
+(set-logic ALL)
+(declare-fun a () Int)
+(declare-fun f ((Set Bool)) Int)
+(declare-fun s () (Set Bool))
+
+(assert (member true s))
+(assert (member false s))
+(assert (= a (f (as univset (Set Bool)))))
+
+(assert (= (f (as emptyset (Set Bool))) 1))
+(assert (= (f (singleton true)) 2))
+(assert (= (f (singleton false)) 3))
+(assert (= (f (union (singleton true) (singleton false))) 4))
+(check-sat)