From 0f66dd16f35eac64149919f0f4048b422345c5eb Mon Sep 17 00:00:00 2001 From: Tim King Date: Mon, 26 Oct 2015 12:21:42 -0700 Subject: [PATCH] This commit fixes a bug related to a public header depending on a compiler flag. This resulted in user code seeing a different size for the SmtEngine class than what was compiled in the library. Proofs are enabled by default again. See http://cvc4.cs.nyu.edu/bugs/show_bug.cgi?id=688 for more information. --- configure.ac | 12 ++++++------ src/proof/proof.h | 25 +++++++++++++++++++++++++ src/smt/options_handlers.h | 5 +++-- src/smt/smt_engine.cpp | 15 +++++++-------- src/smt/smt_engine.h | 3 +-- src/smt/smt_engine_check_proof.cpp | 7 ++++--- src/smt/smt_engine_scope.h | 17 +++++++++-------- src/theory/arith/constraint.cpp | 8 ++++---- src/theory/arith/constraint.h | 24 +++++++++++++----------- 9 files changed, 72 insertions(+), 44 deletions(-) diff --git a/configure.ac b/configure.ac index 7fa6eafc5..fe3d7b8d9 100644 --- a/configure.ac +++ b/configure.ac @@ -445,7 +445,7 @@ case "$with_build" in if test -z "${enable_statistics+set}" ; then enable_statistics=yes ; fi if test -z "${enable_replay+set}" ; then enable_replay=no ; fi if test -z "${enable_assertions+set}" ; then enable_assertions=no ; fi - if test -z "${enable_proof+set}" ; then enable_proof=no ; fi + if test -z "${enable_proof+set}" ; then enable_proof=yes ; fi if test -z "${enable_tracing+set}" ; then enable_tracing=no ; fi if test -z "${enable_dumping+set}" ; then enable_dumping=yes ; fi if test -z "${enable_muzzle+set}" ; then enable_muzzle=no ; fi @@ -461,7 +461,7 @@ case "$with_build" in if test -z "${enable_statistics+set}" ; then enable_statistics=yes ; fi if test -z "${enable_replay+set}" ; then enable_replay=yes ; fi if test -z "${enable_assertions+set}" ; then enable_assertions=yes ; fi - if test -z "${enable_proof+set}" ; then enable_proof=no ; fi + if test -z "${enable_proof+set}" ; then enable_proof=yes ; fi if test -z "${enable_tracing+set}" ; then enable_tracing=yes ; fi if test -z "${enable_dumping+set}" ; then enable_dumping=yes ; fi if test -z "${enable_muzzle+set}" ; then enable_muzzle=no ; fi @@ -478,7 +478,7 @@ case "$with_build" in if test -z "${enable_statistics+set}" ; then enable_statistics=yes ; fi if test -z "${enable_replay+set}" ; then enable_replay=yes ; fi if test -z "${enable_assertions+set}" ; then enable_assertions=yes ; fi - if test -z "${enable_proof+set}" ; then enable_proof=no ; fi + if test -z "${enable_proof+set}" ; then enable_proof=yes ; fi if test -z "${enable_tracing+set}" ; then enable_tracing=yes ; fi if test -z "${enable_dumping+set}" ; then enable_dumping=yes ; fi if test -z "${enable_muzzle+set}" ; then enable_muzzle=no ; fi @@ -541,10 +541,10 @@ fi AC_MSG_CHECKING([whether to support proofs in libcvc4]) AC_ARG_ENABLE([proof], - [AS_HELP_STRING([--enable-proof], - [support proof generation])]) + [AS_HELP_STRING([--disable-proof], + [do not support proof generation])]) if test -z "${enable_proof+set}"; then - enable_proof=no + enable_proof=yes fi AC_MSG_RESULT([$enable_proof]) diff --git a/src/proof/proof.h b/src/proof/proof.h index 440279dbc..25979dc46 100644 --- a/src/proof/proof.h +++ b/src/proof/proof.h @@ -21,6 +21,30 @@ #include "smt/options.h" + +/* Do NOT use #ifdef CVC4_PROOF to check if proofs are enabled. + * We cannot assume users will use -DCVC4_PROOFS if they have a proofs build. + * The preferred way of checking that proofs are enabled is to use: + * #if IS_PROOFS_BUILD + * ... + * #endif + * + * The macro IS_PROOFS_BUILD is defined in util/configuration_private.h + * + * This has the effect of forcing that location to have included this header + * *before* performing this test. This includes C preprocessing expansion. + * This forces the inclusion of "cvc4_private.h". This is intentional! + * + * See bug 688 for more details: + * http://cvc4.cs.nyu.edu/bugs/show_bug.cgi?id=688 + * + * If you want to check CVC4_PROOF, you should have a very good reason + * and should list the exceptions here: + * - Makefile.am + * - proof/proofs.h + * - util/configuration_private.h + */ + #ifdef CVC4_PROOF # define PROOF(x) if(options::proof() || options::unsatCores()) { x; } # define NULLPROOF(x) (options::proof() || options::unsatCores()) ? x : NULL @@ -31,4 +55,5 @@ # define PROOF_ON() false #endif /* CVC4_PROOF */ + #endif /* __CVC4__PROOF__PROOF_H */ diff --git a/src/smt/options_handlers.h b/src/smt/options_handlers.h index fc2b796d3..eeefd7af0 100644 --- a/src/smt/options_handlers.h +++ b/src/smt/options_handlers.h @@ -20,6 +20,7 @@ #define __CVC4__SMT__OPTIONS_HANDLERS_H #include "cvc4autoconfig.h" +#include "util/configuration_private.h" #include "util/dump.h" #include "util/resource_manager.h" #include "smt/modal_exception.h" @@ -307,13 +308,13 @@ inline void setProduceAssertions(std::string option, bool value, SmtEngine* smt) // ensure we are a proof-enabled build of CVC4 inline void proofEnabledBuild(std::string option, bool value, SmtEngine* smt) throw(OptionException) { -#ifndef CVC4_PROOF +#if !(IS_PROOFS_BUILD) if(value) { std::stringstream ss; ss << "option `" << option << "' requires a proofs-enabled build of CVC4; this binary was not built with proof support"; throw OptionException(ss.str()); } -#endif /* CVC4_PROOF */ +#endif /* IS_PROOFS_BUILD */ } // This macro is used for setting :regular-output-channel and :diagnostic-output-channel diff --git a/src/smt/smt_engine.cpp b/src/smt/smt_engine.cpp index c863909e5..5dfada655 100644 --- a/src/smt/smt_engine.cpp +++ b/src/smt/smt_engine.cpp @@ -57,6 +57,7 @@ #include "util/boolean_simplification.h" #include "util/node_visitor.h" #include "util/configuration.h" +#include "util/configuration_private.h" #include "util/exception.h" #include "util/nary_builder.h" #include "smt/command_list.h" @@ -701,9 +702,7 @@ SmtEngine::SmtEngine(ExprManager* em) throw() : d_modelGlobalCommands(), d_modelCommands(NULL), d_dumpCommands(), -#ifdef CVC4_PROOF d_defineCommands(), -#endif d_logic(), d_originalOptions(em->getOptions()), d_pendingPops(0), @@ -4377,7 +4376,7 @@ UnsatCore SmtEngine::getUnsatCore() throw(ModalException, UnsafeInterruptExcepti if(Dump.isOn("benchmark")) { Dump("benchmark") << GetUnsatCoreCommand(); } -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD if(!options::unsatCores()) { throw ModalException("Cannot get an unsat core when produce-unsat-cores option is off."); } @@ -4389,9 +4388,9 @@ UnsatCore SmtEngine::getUnsatCore() throw(ModalException, UnsafeInterruptExcepti d_proofManager->getProof(this);// just to trigger core creation return UnsatCore(this, d_proofManager->begin_unsat_core(), d_proofManager->end_unsat_core()); -#else /* CVC4_PROOF */ +#else /* IS_PROOFS_BUILD */ throw ModalException("This build of CVC4 doesn't have proof support (required for unsat cores)."); -#endif /* CVC4_PROOF */ +#endif /* IS_PROOFS_BUILD */ } Proof* SmtEngine::getProof() throw(ModalException, UnsafeInterruptException) { @@ -4401,7 +4400,7 @@ Proof* SmtEngine::getProof() throw(ModalException, UnsafeInterruptException) { if(Dump.isOn("benchmark")) { Dump("benchmark") << GetProofCommand(); } -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD if(!options::proof()) { throw ModalException("Cannot get a proof when produce-proofs option is off."); } @@ -4412,9 +4411,9 @@ Proof* SmtEngine::getProof() throw(ModalException, UnsafeInterruptException) { } return ProofManager::getProof(this); -#else /* CVC4_PROOF */ +#else /* IS_PROOFS_BUILD */ throw ModalException("This build of CVC4 doesn't have proof support."); -#endif /* CVC4_PROOF */ +#endif /* IS_PROOFS_BUILD */ } void SmtEngine::printInstantiations( std::ostream& out ) { diff --git a/src/smt/smt_engine.h b/src/smt/smt_engine.h index b4c293dff..be31d768b 100644 --- a/src/smt/smt_engine.h +++ b/src/smt/smt_engine.h @@ -196,9 +196,8 @@ class CVC4_PUBLIC SmtEngine { *A vector of command definitions to be imported in the new *SmtEngine when checking unsat-cores. */ -#ifdef CVC4_PROOF std::vector d_defineCommands; -#endif + /** * The logic we're in. */ diff --git a/src/smt/smt_engine_check_proof.cpp b/src/smt/smt_engine_check_proof.cpp index 4c35bd863..d04daef92 100644 --- a/src/smt/smt_engine_check_proof.cpp +++ b/src/smt/smt_engine_check_proof.cpp @@ -16,6 +16,7 @@ **/ #include "smt/smt_engine.h" +#include "util/configuration_private.h" #include "util/statistics_registry.h" #include "check.h" @@ -49,7 +50,7 @@ public: void SmtEngine::checkProof() { -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD Chat() << "generating proof..." << endl; @@ -88,10 +89,10 @@ void SmtEngine::checkProof() { check_file(pfFile, args()); close(fd); -#else /* CVC4_PROOF */ +#else /* IS_PROOFS_BUILD */ Unreachable("This version of CVC4 was built without proof support; cannot check proofs."); -#endif /* CVC4_PROOF */ +#endif /* IS_PROOFS_BUILD */ } diff --git a/src/smt/smt_engine_scope.h b/src/smt/smt_engine_scope.h index 701775c9c..bc978b728 100644 --- a/src/smt/smt_engine_scope.h +++ b/src/smt/smt_engine_scope.h @@ -17,14 +17,15 @@ #include "cvc4_private.h" +#pragma once + +#include "expr/node_manager.h" #include "smt/smt_engine.h" -#include "util/tls.h" +#include "smt/options.h" +#include "util/configuration_private.h" #include "util/cvc4_assert.h" -#include "expr/node_manager.h" #include "util/output.h" -#include "proof/proof.h" - -#pragma once +#include "util/tls.h" namespace CVC4 { @@ -43,14 +44,14 @@ inline bool smtEngineInScope() { } inline ProofManager* currentProofManager() { -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD Assert(options::proof() || options::unsatCores()); Assert(s_smtEngine_current != NULL); return s_smtEngine_current->d_proofManager; -#else /* CVC4_PROOF */ +#else /* IS_PROOFS_BUILD */ InternalError("proofs/unsat cores are not on, but ProofManager requested"); return NULL; -#endif /* CVC4_PROOF */ +#endif /* IS_PROOFS_BUILD */ } class SmtScope : public NodeManagerScope { diff --git a/src/theory/arith/constraint.cpp b/src/theory/arith/constraint.cpp index f1cac9044..4acf86d43 100644 --- a/src/theory/arith/constraint.cpp +++ b/src/theory/arith/constraint.cpp @@ -615,10 +615,10 @@ bool Constraint::wellFormedFarkasProof() const { ConstraintCP antecedent = d_database->d_antecedents[p]; if(antecedent == NullConstraint) { return false; } -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD if(!PROOF_ON()){ return cr.d_farkasCoefficients == RationalVectorCPSentinel; } Assert(PROOF_ON()); - + if(cr.d_farkasCoefficients == RationalVectorCPSentinel){ return false; } if(cr.d_farkasCoefficients->size() < 2){ return false; } @@ -717,9 +717,9 @@ bool Constraint::wellFormedFarkasProof() const { (lhs.isNull() || Constant::isMember(lhs) && Constant(lhs).isZero() ) && rhs.sgn() < 0; -#else +#else /* IS_PROOFS_BUILD */ return true; -#endif +#endif /* IS_PROOFS_BUILD */ } ConstraintP Constraint::makeNegation(ArithVar v, ConstraintType t, const DeltaRational& r){ diff --git a/src/theory/arith/constraint.h b/src/theory/arith/constraint.h index 0e0b35020..877546da8 100644 --- a/src/theory/arith/constraint.h +++ b/src/theory/arith/constraint.h @@ -88,6 +88,8 @@ #include "theory/arith/constraint_forward.h" #include "theory/arith/callbacks.h" +#include "util/configuration_private.h" + #include #include #include @@ -299,17 +301,17 @@ struct ConstraintRule { * There is no requirement that the proof is minimal. * We do however use all of the constraints by requiring non-zero coefficients. */ -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD RationalVectorCP d_farkasCoefficients; -#endif +#endif /* IS_PROOFS_BUILD */ ConstraintRule() : d_constraint(NullConstraint) , d_proofType(NoAP) , d_antecedentEnd(AntecedentIdSentinel) { -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD d_farkasCoefficients = RationalVectorCPSentinel; -#endif +#endif /* IS_PROOFS_BUILD */ } ConstraintRule(ConstraintP con, ArithProofType pt) @@ -317,18 +319,18 @@ struct ConstraintRule { , d_proofType(pt) , d_antecedentEnd(AntecedentIdSentinel) { -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD d_farkasCoefficients = RationalVectorCPSentinel; -#endif +#endif /* IS_PROOFS_BUILD */ } ConstraintRule(ConstraintP con, ArithProofType pt, AntecedentId antecedentEnd) : d_constraint(con) , d_proofType(pt) , d_antecedentEnd(antecedentEnd) { -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD d_farkasCoefficients = RationalVectorCPSentinel; -#endif +#endif /* IS_PROOFS_BUILD */ } ConstraintRule(ConstraintP con, ArithProofType pt, AntecedentId antecedentEnd, RationalVectorCP coeffs) @@ -337,11 +339,11 @@ struct ConstraintRule { , d_antecedentEnd(antecedentEnd) { Assert(PROOF_ON() || coeffs == RationalVectorCPSentinel); -#ifdef CVC4_PROOF +#if IS_PROOFS_BUILD d_farkasCoefficients = coeffs; -#endif +#endif /* IS_PROOFS_BUILD */ } - + void print(std::ostream& out) const; void debugPrint() const; }; /* class ConstraintRule */ -- 2.30.2