Do not apply fmfBound to standard quantifiers when only stringsExp is enabled (#6816)
authorAndrew Reynolds <andrew.j.reynolds@gmail.com>
Wed, 30 Jun 2021 16:50:33 +0000 (11:50 -0500)
committerGitHub <noreply@github.com>
Wed, 30 Jun 2021 16:50:33 +0000 (16:50 +0000)
There are compelling use cases that combine strings/sequences and quantifiers. Prior to this PR, strings enabled "bounded integer" quantifier instantiation so that internally generated quantifiers for strings reductions are handled in a complete manner. However, this enabled bounded integer quantifier instantiation globally. This degrades performance for "unsat" on quantified formulas in general.

After this PR, we do not enable bounded integer quantifier globally. Instead, we ensure that bounded integer quantification is applied to at least the internally generated quantified formulas; all other quantified formulas are handled as usual.

Notice this required ensuring that all quantified formulas generated by strings are marked properly. It also required adding --fmf-bound to a few regressions that explicitly require it.

13 files changed:
src/smt/set_defaults.cpp
src/theory/quantifiers/fmf/bounded_integers.cpp
src/theory/quantifiers/quantifiers_modules.cpp
src/theory/strings/regexp_elim.cpp
src/theory/strings/regexp_operation.cpp
src/theory/strings/theory_strings_preprocess.cpp
src/theory/strings/theory_strings_preprocess.h
src/theory/strings/theory_strings_utils.cpp
src/theory/strings/theory_strings_utils.h
test/regress/CMakeLists.txt
test/regress/regress1/fmf/bug651.smt2
test/regress/regress1/quantifiers/issue3537.smt2
test/regress/regress1/strings/seq-quant-infinite-branch.smt2 [new file with mode: 0644]

index 7ae07d196e108058349f377236369a144e617997..ee3701d513a490c3d2d1d153f9c8a081e6aed7f1 100644 (file)
@@ -292,14 +292,11 @@ void setDefaults(LogicInfo& logic, bool isInternalSubsolver)
       Trace("smt") << "turning on quantifier logic, for strings-exp"
                    << std::endl;
     }
-    // We require bounded quantifier handling.
-    if (!opts.quantifiers.fmfBoundWasSetByUser)
-    {
-      opts.quantifiers.fmfBound = true;
-      Trace("smt") << "turning on fmf-bound, for strings-exp" << std::endl;
-    }
     // Note we allow E-matching by default to support combinations of sequences
-    // and quantifiers.
+    // and quantifiers. We also do not enable fmfBound here, which would
+    // enable bounded integer instantiation for *all* quantifiers. Instead,
+    // the bounded integers module will always process internally generated
+    // quantifiers (those marked with InternalQuantAttribute).
   }
   // whether we must disable proofs
   bool disableProofs = false;
index 5ecc337785d8ef9625b700d74452ce0001400fd7..3fd478c31f55734b60ef4a84eb1981880c79cd30 100644 (file)
@@ -317,6 +317,19 @@ void BoundedIntegers::checkOwnership(Node f)
 {
   //this needs to be done at preregister since it affects e.g. QuantDSplit's preregister
   Trace("bound-int") << "check ownership quantifier " << f << std::endl;
+
+  // determine if we should look at the quantified formula at all
+  if (!options::fmfBound())
+  {
+    // only applying it to internal quantifiers
+    QuantAttributes& qattr = d_qreg.getQuantAttributes();
+    if (!qattr.isInternal(f))
+    {
+      Trace("bound-int") << "...not internal, skip" << std::endl;
+      return;
+    }
+  }
+
   NodeManager* nm = NodeManager::currentNM();
   SkolemManager* sm = nm->getSkolemManager();
 
index f6b8f30f452fc89cebed43043f93cb00b67f0754..27ec187a9b51a5ae13d1b3b1e7c1370eff0d586d 100644 (file)
@@ -16,6 +16,7 @@
 #include "theory/quantifiers/quantifiers_modules.h"
 
 #include "options/quantifiers_options.h"
+#include "options/strings_options.h"
 #include "theory/quantifiers/relevant_domain.h"
 #include "theory/quantifiers/term_registry.h"
 
@@ -74,14 +75,15 @@ void QuantifiersModules::initialize(QuantifiersState& qs,
     d_synth_e.reset(new SynthEngine(qs, qim, qr, tr));
     modules.push_back(d_synth_e.get());
   }
-  // finite model finding
-  if (options::fmfBound())
+  // bounded integer instantiation is used when the user requests it via
+  // fmfBound, or if strings are enabled.
+  if (options::fmfBound() || options::stringExp())
   {
     d_bint.reset(new BoundedIntegers(qs, qim, qr, tr));
     modules.push_back(d_bint.get());
   }
 
-  if (options::finiteModelFind() || options::fmfBound())
+  if (options::finiteModelFind() || options::fmfBound() || options::stringExp())
   {
     d_model_engine.reset(new ModelEngine(qs, qim, qr, tr, builder));
     modules.push_back(d_model_engine.get());
index 18a5b049ade5af820ae30a53e6115dc30bd3bc2b..dfe006a8645fee2a3bab1e2194f6b1051592c31d 100644 (file)
@@ -347,7 +347,7 @@ Node RegExpElimination::eliminateConcat(Node atom, bool isAgg)
         children2.push_back(res);
         Node body = nm->mkNode(AND, children2);
         Node bvl = nm->mkNode(BOUND_VAR_LIST, non_greedy_find_vars);
-        res = nm->mkNode(EXISTS, bvl, body);
+        res = utils::mkForallInternal(bvl, body.negate()).negate();
       }
       // must also give a minimum length requirement
       res = nm->mkNode(AND, res, nm->mkNode(GEQ, lenx, lenSum));
@@ -486,7 +486,7 @@ Node RegExpElimination::eliminateConcat(Node atom, bool isAgg)
       if (k.getKind() == BOUND_VARIABLE)
       {
         Node bvl = nm->mkNode(BOUND_VAR_LIST, k);
-        body = nm->mkNode(EXISTS, bvl, body);
+        body = utils::mkForallInternal(bvl, body.negate()).negate();
       }
       // e.g. x in re.++( R1, "AB", R2 ) --->
       //  exists k.
@@ -575,7 +575,7 @@ Node RegExpElimination::eliminateStar(Node atom, bool isAgg)
                                              : nm->mkNode(OR, char_constraints);
     Node body = nm->mkNode(OR, bound.negate(), conc);
     Node bvl = nm->mkNode(BOUND_VAR_LIST, index);
-    Node res = nm->mkNode(FORALL, bvl, body);
+    Node res = utils::mkForallInternal(bvl, body);
     // e.g.
     //   x in (re.* (re.union "A" "B" )) --->
     //   forall k. 0<=k<len(x) => (substr(x,k,1) in "A" OR substr(x,k,1) in "B")
@@ -605,7 +605,7 @@ Node RegExpElimination::eliminateStar(Node atom, bool isAgg)
                 .eqNode(s);
         Node body = nm->mkNode(OR, bound.negate(), conc);
         Node bvl = nm->mkNode(BOUND_VAR_LIST, index);
-        Node res = nm->mkNode(FORALL, bvl, body);
+        Node res = utils::mkForallInternal(bvl, body);
         res = nm->mkNode(
             AND, nm->mkNode(INTS_MODULUS_TOTAL, lenx, lens).eqNode(zero), res);
         // e.g.
index 106ce09d0f4da8c8a3917cf7bba5d7d0e6466422..268368e7fe4c220e6e2f0b6d7c34da494c2b827a 100644 (file)
@@ -928,7 +928,8 @@ Node RegExpOpr::reduceRegExpNeg(Node mem)
 
     conc = nm->mkNode(OR, s1r1, s2r2);
     conc = nm->mkNode(IMPLIES, g1, conc);
-    conc = nm->mkNode(FORALL, b1v, conc);
+    // must mark as an internal quantifier
+    conc = utils::mkForallInternal(b1v, conc);
     conc = nm->mkNode(AND, sne, conc);
   }
   else
@@ -999,7 +1000,8 @@ Node RegExpOpr::reduceRegExpNegConcatFixed(Node mem, Node reLen, size_t index)
   if (!b1v.isNull())
   {
     conc = nm->mkNode(OR, guard.negate(), conc);
-    conc = nm->mkNode(FORALL, b1v, conc);
+    // must mark as an internal quantifier
+    conc = utils::mkForallInternal(b1v, conc);
   }
   return conc;
 }
index 5f21d6e658e30a39795e32ba5de0b0e578dbbdb3..550a9af8b472d1f003745d4ff5cd87b93c7fc109 100644 (file)
@@ -20,9 +20,9 @@
 #include "options/smt_options.h"
 #include "options/strings_options.h"
 #include "smt/logic_exception.h"
-#include "theory/quantifiers/quantifiers_attributes.h"
 #include "theory/strings/arith_entail.h"
 #include "theory/strings/sequences_rewriter.h"
+#include "theory/strings/theory_strings_utils.h"
 #include "theory/strings/word.h"
 #include "util/rational.h"
 #include "util/statistics_registry.h"
@@ -35,13 +35,6 @@ namespace cvc5 {
 namespace theory {
 namespace strings {
 
-/** Mapping to a dummy node for marking an attribute on internal quantified
- * formulas */
-struct QInternalVarAttributeId
-{
-};
-typedef expr::Attribute<QInternalVarAttributeId, Node> QInternalVarAttribute;
-
 StringsPreprocess::StringsPreprocess(SkolemCache* sc,
                                      HistogramStat<Kind>* statReductions)
     : d_sc(sc), d_statReductions(statReductions)
@@ -299,7 +292,7 @@ Node StringsPreprocess::reduce(Node t,
     // forall il.
     //   n <= i < ite(skk = -1, len(s), skk) ^ 0 < l <= len(s) - i =>
     //     ~in_re(substr(s, i, l), r)
-    Node firstMatch = mkForallInternal(bvl, body);
+    Node firstMatch = utils::mkForallInternal(bvl, body);
     Node bvll = nm->mkNode(BOUND_VAR_LIST, l);
     Node validLen =
         nm->mkNode(AND,
@@ -311,8 +304,10 @@ Node StringsPreprocess::reduce(Node t,
         nm->mkNode(STRING_IN_REGEXP, nm->mkNode(STRING_SUBSTR, s, skk, l), r));
     // skk != -1 =>
     //   exists l. (0 <= l < len(s) - skk) ^ in_re(substr(s, skk, l), r))
-    Node match = nm->mkNode(
-        OR, retNegOne, mkForallInternal(bvll, matchBody.negate()).negate());
+    Node match =
+        nm->mkNode(OR,
+                   retNegOne,
+                   utils::mkForallInternal(bvll, matchBody.negate()).negate());
 
     // assert:
     // IF:   n > len(s) OR 0 > n
@@ -382,7 +377,7 @@ Node StringsPreprocess::reduce(Node t,
     Node ux1lem = nm->mkNode(GEQ, n, ux1);
 
     lem = nm->mkNode(OR, g.negate(), nm->mkNode(AND, eq, cb, ux1lem));
-    lem = mkForallInternal(xbv, lem);
+    lem = utils::mkForallInternal(xbv, lem);
     conc.push_back(lem);
 
     Node nonneg = nm->mkNode(GEQ, n, zero);
@@ -470,7 +465,7 @@ Node StringsPreprocess::reduce(Node t,
     Node ux1lem = nm->mkNode(GEQ, stoit, ux1);
 
     lem = nm->mkNode(OR, g.negate(), nm->mkNode(AND, eq, cb, ux1lem));
-    lem = mkForallInternal(xbv, lem);
+    lem = utils::mkForallInternal(xbv, lem);
     conc2.push_back(lem);
 
     Node sneg = nm->mkNode(LT, stoit, zero);
@@ -651,7 +646,7 @@ Node StringsPreprocess::reduce(Node t,
         ufip1.eqNode(nm->mkNode(PLUS, ii, nm->mkNode(STRING_LENGTH, y))));
 
     Node body = nm->mkNode(OR, bound.negate(), nm->mkNode(AND, flem));
-    Node q = mkForallInternal(bvli, body);
+    Node q = utils::mkForallInternal(bvli, body);
     lem.push_back(q);
 
     // assert:
@@ -717,7 +712,7 @@ Node StringsPreprocess::reduce(Node t,
         nm->mkNode(STRING_IN_REGEXP, nm->mkNode(STRING_SUBSTR, k2, zero, l), y)
             .negate());
     // forall l. 0 <= l < len(k2) => ~in_re(substr(k2, 0, l), r)
-    Node shortestMatch = mkForallInternal(bvll, body);
+    Node shortestMatch = utils::mkForallInternal(bvll, body);
     // in_re(k2, y)
     Node match = nm->mkNode(STRING_IN_REGEXP, k2, y);
     // k = k1 ++ z ++ k3
@@ -811,7 +806,7 @@ Node StringsPreprocess::reduce(Node t,
             .negate());
     // forall l. 0 < l < Ul(i + 1) =>
     //   ~in_re(substr(x, Uf(i + 1) - Ul(i + 1), l), y')
-    flem.push_back(mkForallInternal(bvll, shortestMatchBody));
+    flem.push_back(utils::mkForallInternal(bvll, shortestMatchBody));
     Node pfxMatch =
         nm->mkNode(STRING_SUBSTR, x, ufi, nm->mkNode(MINUS, ii, ufi));
     // Us(i) = substr(x, Uf(i), ii - Uf(i)) ++ z ++ Us(i + 1)
@@ -820,7 +815,7 @@ Node StringsPreprocess::reduce(Node t,
             .eqNode(nm->mkNode(
                 STRING_CONCAT, pfxMatch, z, nm->mkNode(APPLY_UF, us, ip1))));
     Node body = nm->mkNode(OR, bound.negate(), nm->mkNode(AND, flem));
-    Node forall = mkForallInternal(bvli, body);
+    Node forall = utils::mkForallInternal(bvli, body);
     lemmas.push_back(forall);
 
     // IF indexof(x, y', 0) = -1
@@ -886,7 +881,7 @@ Node StringsPreprocess::reduce(Node t,
     Node bound =
         nm->mkNode(AND, nm->mkNode(LEQ, zero, i), nm->mkNode(LT, i, lenr));
     Node body = nm->mkNode(OR, bound.negate(), ri.eqNode(res));
-    Node rangeA = mkForallInternal(bvi, body);
+    Node rangeA = utils::mkForallInternal(bvi, body);
 
     // upper 65 ... 90
     // lower 97 ... 122
@@ -921,7 +916,7 @@ Node StringsPreprocess::reduce(Node t,
     Node bound =
         nm->mkNode(AND, nm->mkNode(LEQ, zero, i), nm->mkNode(LT, i, lenr));
     Node body = nm->mkNode(OR, bound.negate(), ssr.eqNode(ssx));
-    Node rangeA = mkForallInternal(bvi, body);
+    Node rangeA = utils::mkForallInternal(bvi, body);
     // assert:
     //   len(r) = len(x) ^
     //   forall i. 0 <= i < len(r) =>
@@ -952,7 +947,7 @@ Node StringsPreprocess::reduce(Node t,
             kind::EQUAL,
             NodeManager::currentNM()->mkNode(kind::STRING_SUBSTR, x, b1, lens),
             s));
-    retNode = NodeManager::currentNM()->mkNode( kind::EXISTS, b1v, body );
+    retNode = utils::mkForallInternal(b1v, body.negate()).negate();
   }
   else if (t.getKind() == kind::STRING_LEQ)
   {
@@ -982,8 +977,9 @@ Node StringsPreprocess::reduce(Node t,
     }
     conj.push_back(nm->mkNode(ITE, ite_ch));
 
-    Node conjn = nm->mkNode(
-        EXISTS, nm->mkNode(BOUND_VAR_LIST, k), nm->mkNode(AND, conj));
+    Node conjn = utils::mkForallInternal(nm->mkNode(BOUND_VAR_LIST, k),
+                                         nm->mkNode(AND, conj).negate())
+                     .negate();
     // Intuitively, the reduction says either x and y are equal, or they have
     // some (maximal) common prefix after which their characters at position k
     // are distinct, and the comparison of their code matches the return value
@@ -1095,31 +1091,6 @@ Node StringsPreprocess::processAssertion(Node n, std::vector<Node>& asserts)
   return ret;
 }
 
-Node StringsPreprocess::mkForallInternal(Node bvl, Node body)
-{
-  NodeManager* nm = NodeManager::currentNM();
-  QInternalVarAttribute qiva;
-  Node qvar;
-  if (bvl.hasAttribute(qiva))
-  {
-    qvar = bvl.getAttribute(qiva);
-  }
-  else
-  {
-    SkolemManager* sm = nm->getSkolemManager();
-    qvar = sm->mkDummySkolem("qinternal", nm->booleanType());
-    // this dummy variable marks that the quantified formula is internal
-    qvar.setAttribute(InternalQuantAttribute(), true);
-    // remember the dummy variable
-    bvl.setAttribute(qiva, qvar);
-  }
-  // make the internal attribute, and put it in a singleton list
-  Node ip = nm->mkNode(INST_ATTRIBUTE, qvar);
-  Node ipl = nm->mkNode(INST_PATTERN_LIST, ip);
-  // make the overall formula
-  return nm->mkNode(FORALL, bvl, body, ipl);
-}
-
 }  // namespace strings
 }  // namespace theory
 }  // namespace cvc5
index 7f0efe50fd2348976b2dcf46b8116a1d8d25f0e3..01e6fa85688278f72b95a35d2dca113d694328fd 100644 (file)
@@ -91,14 +91,6 @@ class StringsPreprocess {
    * visited stores a cache of previous results.
    */
   Node simplifyRec(Node t, std::vector<Node>& asserts);
-  /**
-   * Make internal quantified formula with bound variable list bvl and body.
-   * Internally, we get a node corresponding to marking a quantified formula as
-   * an "internal" one. This node is provided as the third argument of the
-   * FORALL returned by this method. This ensures that E-matching is not applied
-   * to the quantified formula.
-   */
-  static Node mkForallInternal(Node bvl, Node body);
 };
 
 }  // namespace strings
index cfe126ef3916c00f00cfdce786964a88e47aa380..27bf090ca7605b88887e340b637ae850e3b50b98 100644 (file)
 
 #include <sstream>
 
+#include "expr/attribute.h"
+#include "expr/skolem_manager.h"
 #include "options/strings_options.h"
+#include "theory/quantifiers/quantifiers_attributes.h"
 #include "theory/rewriter.h"
 #include "theory/strings/arith_entail.h"
 #include "theory/strings/strings_entail.h"
@@ -421,6 +424,40 @@ unsigned getLoopMinOccurrences(TNode node)
   return node.getOperator().getConst<RegExpLoop>().d_loopMinOcc;
 }
 
+/**
+ * Mapping to a dummy node for marking an attribute on internal quantified
+ * formulas. This ensures that reductions are deterministic.
+ */
+struct QInternalVarAttributeId
+{
+};
+typedef expr::Attribute<QInternalVarAttributeId, Node> QInternalVarAttribute;
+
+Node mkForallInternal(Node bvl, Node body)
+{
+  NodeManager* nm = NodeManager::currentNM();
+  QInternalVarAttribute qiva;
+  Node qvar;
+  if (bvl.hasAttribute(qiva))
+  {
+    qvar = bvl.getAttribute(qiva);
+  }
+  else
+  {
+    SkolemManager* sm = nm->getSkolemManager();
+    qvar = sm->mkDummySkolem("qinternal", nm->booleanType());
+    // this dummy variable marks that the quantified formula is internal
+    qvar.setAttribute(InternalQuantAttribute(), true);
+    // remember the dummy variable
+    bvl.setAttribute(qiva, qvar);
+  }
+  // make the internal attribute, and put it in a singleton list
+  Node ip = nm->mkNode(INST_ATTRIBUTE, qvar);
+  Node ipl = nm->mkNode(INST_PATTERN_LIST, ip);
+  // make the overall formula
+  return nm->mkNode(FORALL, bvl, body, ipl);
+}
+
 }  // namespace utils
 }  // namespace strings
 }  // namespace theory
index ec093031e59f63c9830af192b0a07c70f82e0f8c..58162ac1bc6b3e5a57a6846f63cf3610b914c38e 100644 (file)
@@ -216,6 +216,15 @@ unsigned getLoopMaxOccurrences(TNode node);
 /* Get the minimum occurrences of given regexp loop node. */
 unsigned getLoopMinOccurrences(TNode node);
 
+/**
+ * Make internal quantified formula with bound variable list bvl and body.
+ * Internally, we get a node corresponding to marking a quantified formula as
+ * an "internal" one. This node is provided as the third argument of the
+ * FORALL returned by this method. This ensures that E-matching is not applied
+ * to the quantified formula.
+ */
+Node mkForallInternal(Node bvl, Node body);
+
 }  // namespace utils
 }  // namespace strings
 }  // namespace theory
index da67705f57b6a2d2f189555419ebba95cee737a3..5312e70a38bfc2b2ec1557dce4df7cbd741a4e29 100644 (file)
@@ -2198,6 +2198,7 @@ set(regress_1_tests
   regress1/strings/rev-ex5.smt2
   regress1/strings/rew-020618.smt2
   regress1/strings/rew-check1.smt2
+  regress1/strings/seq-quant-infinite-branch.smt2
   regress1/strings/simple-re-consume.smt2
   regress1/strings/stoi-400million.smt2
   regress1/strings/stoi-solve.smt2
index d243e0ce717fa168e95896606352a59fc5fdb0cb..956a55cd32c33d76455f4aeba2a866478ea6848a 100644 (file)
@@ -1,4 +1,4 @@
-; COMMAND-LINE: --fmf-fun --no-check-models
+; COMMAND-LINE: --fmf-fun --no-check-models --fmf-bound
 ; EXPECT: sat
 (set-logic UFDTSLIA)
 (set-option :produce-models true)
index 7f0ab060f854569b8ba2988b091e9cd1bd3c7d28..442eb65ac16fcecb17dfa8f68f46eb862f1694cf 100644 (file)
@@ -1,4 +1,4 @@
-; COMMAND-LINE: --strings-exp --finite-model-find
+; COMMAND-LINE: --strings-exp --finite-model-find --fmf-bound
 ; EXPECT: sat
 (set-logic ALL)
 (declare-datatypes ((UNIT 0)) (((Unit))
diff --git a/test/regress/regress1/strings/seq-quant-infinite-branch.smt2 b/test/regress/regress1/strings/seq-quant-infinite-branch.smt2
new file mode 100644 (file)
index 0000000..9816301
--- /dev/null
@@ -0,0 +1,18 @@
+(set-logic ALL)
+(set-info :status unsat)
+(declare-const x8 Bool)
+(set-option :strings-exp true)
+(declare-fun s () (Seq Int))
+(declare-fun x () Int)
+(declare-fun i () Int)
+(declare-fun i@ () Int)
+(assert (and (forall ((u Int)) (or (> 0 u) (>= u i) (distinct x (seq.nth s u))))
+             (or (and x8 (not x8))
+                 (and (= i@ (+ i 1))
+                      (distinct x (seq.nth s i))
+                      (exists ((u Int)) (and (<= 0 u)
+                                             (< u i@)
+                                             (= x (seq.nth s u))))
+                 )
+        )))
+(check-sat)