Make theory preprocess rewrite equalities a preprocessing pass (#5711)

Some theories rewrite equalities during ppRewrite. An example is arithmetic with the option arith-rewrite-eq, which rewrites (= x y) to (and (>= x y) (<= x y)) during theory preprocessing.

This PR makes it so that ppRewrite is only called on equalities in preprocessing, during a new preprocessing pass "TheoryRewriteEq". On the other hand, ppRewrite is never called on new equalities generated in lemmas from TheoryEngine.

In detail, the motivation for this change:

(1) Rewriting equalities during ppRewrite is dangerous since it may break invariants wrt theory combination. In particular, equalities in splitting lemmas originating from theory combination must not be theory-preprocessed, or else we may be non-terminating or solution unsound. This can happen if a theory requests a split on (= x y) but is not notified of this atom when another theory rewrites (= x y) during ppRewrite.

(2) After this PR, we can simplify our policy for all lemmas generated, in particular, we can say that all lemmas must be theory preprocessed before their literals are asserted to TheoryEngine. This is now possible as the invariant cannot be broken (theoryRewriteEq is relegated to the preprocessor, which is only applied once). This will make LemmaProperty::PREPROCESS obsolete, which in turn will simplify several lemma caches for nonlinear and quantifiers. It will also significantly simplify proof production for the theory preprocessor (which maintains two stacks of utilities for preprocessed vs non-preprocessed lemmas).

(3) Simplifications to the above policy will make it significantly easier to implement theory-preprocessing apply when literals are asserted. It is currently not possible to implement this in a coherent way without tracking which literals were a part of lemmas marked as "do not theory-preprocess".

3 files changed, 171 insertions, 0 deletions

diff --git a/src/preprocessing/passes/theory_rewrite_eq.cpp b/src/preprocessing/passes/theory_rewrite_eq.cpp
new file mode 100644
index 000000000..68006b67f
--- /dev/null
+++ b/src/preprocessing/passes/theory_rewrite_eq.cpp
@@ -0,0 +1,113 @@
+/*********************                                                        */
+/*! \file theory_rewrite_eq.cpp
+ ** \verbatim
+ ** Top contributors (to current version):
+ **   Andrew Reynolds
+ ** This file is part of the CVC4 project.
+ ** Copyright (c) 2009-2020 by the authors listed in the file AUTHORS
+ ** in the top-level source directory and their institutional affiliations.
+ ** All rights reserved.  See the file COPYING in the top-level source
+ ** directory for licensing information.\endverbatim
+ **
+ ** \brief The TheoryRewriteEq preprocessing pass
+ **/
+
+#include "preprocessing/passes/theory_rewrite_eq.h"
+
+#include "theory/theory_engine.h"
+
+using namespace CVC4::theory;
+
+namespace CVC4 {
+namespace preprocessing {
+namespace passes {
+
+TheoryRewriteEq::TheoryRewriteEq(PreprocessingPassContext* preprocContext)
+    : PreprocessingPass(preprocContext, "theory-rewrite-eq"){};
+
+PreprocessingPassResult TheoryRewriteEq::applyInternal(
+    AssertionPipeline* assertions)
+{
+  // apply ppRewrite to all equalities in assertions
+  for (std::size_t i = 0, size = assertions->size(); i < size; ++i)
+  {
+    Node assertion = (*assertions)[i];
+    TrustNode trn = rewriteAssertion(assertion);
+    if (!trn.isNull())
+    {
+      // replace based on the trust node
+      assertions->replaceTrusted(i, trn);
+    }
+  }
+  return PreprocessingPassResult::NO_CONFLICT;
+}
+
+theory::TrustNode TheoryRewriteEq::rewriteAssertion(TNode n)
+{
+  NodeManager* nm = NodeManager::currentNM();
+  TheoryEngine* te = d_preprocContext->getTheoryEngine();
+  std::unordered_map<TNode, Node, TNodeHashFunction> visited;
+  std::unordered_map<TNode, Node, TNodeHashFunction>::iterator it;
+  std::vector<TNode> visit;
+  TNode cur;
+  visit.push_back(n);
+  do
+  {
+    cur = visit.back();
+    visit.pop_back();
+    it = visited.find(cur);
+
+    if (it == visited.end())
+    {
+      if (cur.getKind() == kind::EQUAL)
+      {
+        // For example, (= x y) ---> (and (>= x y) (<= x y))
+        theory::TrustNode trn = te->ppRewriteEquality(cur);
+        // can make proof producing by using proof generator from trn
+        visited[cur] = trn.isNull() ? Node(cur) : trn.getNode();
+      }
+      else
+      {
+        visited[cur] = Node::null();
+        visit.push_back(cur);
+        visit.insert(visit.end(), cur.begin(), cur.end());
+      }
+    }
+    else if (it->second.isNull())
+    {
+      Node ret = cur;
+      bool childChanged = false;
+      std::vector<Node> children;
+      if (cur.getMetaKind() == kind::metakind::PARAMETERIZED)
+      {
+        children.push_back(cur.getOperator());
+      }
+      for (const Node& cn : cur)
+      {
+        it = visited.find(cn);
+        Assert(it != visited.end());
+        Assert(!it->second.isNull());
+        childChanged = childChanged || cn != it->second;
+        children.push_back(it->second);
+      }
+      if (childChanged)
+      {
+        ret = nm->mkNode(cur.getKind(), children);
+      }
+      visited[cur] = ret;
+    }
+  } while (!visit.empty());
+  Assert(visited.find(n) != visited.end());
+  Assert(!visited.find(n)->second.isNull());
+  Node ret = visited[n];
+  if (ret == n)
+  {
+    return TrustNode::null();
+  }
+  // can make proof producing by providing a term conversion generator here
+  return TrustNode::mkTrustRewrite(n, ret, nullptr);
+}
+
+}  // namespace passes
+}  // namespace preprocessing
+}  // namespace CVC4
diff --git a/src/preprocessing/passes/theory_rewrite_eq.h b/src/preprocessing/passes/theory_rewrite_eq.h
new file mode 100644
index 000000000..bc4027310
--- /dev/null
+++ b/src/preprocessing/passes/theory_rewrite_eq.h
@@ -0,0 +1,56 @@
+/*********************                                                        */
+/*! \file theory_rewrite_eq.h
+ ** \verbatim
+ ** Top contributors (to current version):
+ **   Andrew Reynolds
+ ** This file is part of the CVC4 project.
+ ** Copyright (c) 2009-2020 by the authors listed in the file AUTHORS
+ ** in the top-level source directory and their institutional affiliations.
+ ** All rights reserved.  See the file COPYING in the top-level source
+ ** directory for licensing information.\endverbatim
+ **
+ ** \brief The TheoryRewriteEq preprocessing pass
+ **/
+
+#include "cvc4_private.h"
+
+#ifndef CVC4__PREPROCESSING__PASSES__THEORY_REWRITE_EQ_H
+#define CVC4__PREPROCESSING__PASSES__THEORY_REWRITE_EQ_H
+
+#include "preprocessing/preprocessing_pass.h"
+#include "preprocessing/preprocessing_pass_context.h"
+#include "theory/trust_node.h"
+
+namespace CVC4 {
+namespace preprocessing {
+namespace passes {
+
+/**
+ * Implements the preprocessing pass for called ppRewrite on all equalities
+ * in the input. This is required to be a preprocessing pass since it is not
+ * recommended that ppRewrite is called on equalities generated in lemmas (e.g.
+ * it may interfere with equality splitting in theory combination).
+ */
+class TheoryRewriteEq : public PreprocessingPass
+{
+ public:
+  TheoryRewriteEq(PreprocessingPassContext* preprocContext);
+
+ protected:
+  PreprocessingPassResult applyInternal(
+      AssertionPipeline* assertionsToPreprocess) override;
+  /**
+   * Rewrite the assertion based on rewriting equalities based on theory
+   * specific rewriting.
+   * An example is removing arithmetic equalities via:
+   *   (= x y) ---> (and (>= x y) (<= x y))
+   * Returns the trust node corresponding to the rewrite.
+   */
+  theory::TrustNode rewriteAssertion(TNode assertion);
+};
+
+}  // namespace passes
+}  // namespace preprocessing
+}  // namespace CVC4
+
+#endif /* CVC4__PREPROCESSING__PASSES__THEORY_REWRITE_EQ_H */
diff --git a/src/preprocessing/preprocessing_pass_registry.cpp b/src/preprocessing/preprocessing_pass_registry.cpp
index 46c1fff68..b21d4e30c 100644
--- a/src/preprocessing/preprocessing_pass_registry.cpp
+++ b/src/preprocessing/preprocessing_pass_registry.cpp
@@ -53,6 +53,7 @@
 #include "preprocessing/passes/sygus_inference.h"
 #include "preprocessing/passes/synth_rew_rules.h"
 #include "preprocessing/passes/theory_preprocess.h"
+#include "preprocessing/passes/theory_rewrite_eq.h"
 #include "preprocessing/passes/unconstrained_simplifier.h"
 #include "preprocessing/preprocessing_pass.h"
 
@@ -149,6 +150,7 @@ PreprocessingPassRegistry::PreprocessingPassRegistry()
   registerPassInfo("bool-to-bv", callCtor<BoolToBV>);
   registerPassInfo("ho-elim", callCtor<HoElim>);
   registerPassInfo("fun-def-fmf", callCtor<FunDefFmf>);
+  registerPassInfo("theory-rewrite-eq", callCtor<TheoryRewriteEq>);
 }
 
 }  // namespace preprocessing