Skip to content

Fix default BoolExpr accumulator construction #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zayenz wants to merge 3 commits into
base: main
Choose a base branch
from
+168 −7
Open

Fix default BoolExpr accumulator construction #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3c68c3
Fix default BoolExpr accumulator construction
Jul 7, 2026
6a6a010
Document BoolExpr accumulator fix
Jul 7, 2026
90631cd
minimodel: restrict empty BoolExpr accumulation
Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,17 @@ Rank: minor
Restore the legacy GIST_STATIC_LIBS compatibility macro for static
Windows clients. GECODE_STATIC_LIBS remains the preferred spelling.

[ENTRY]
Module: minimodel
What: bug
Rank: minor
Issue: 139
Thanks: yuri@FreeBSD
[DESCRIPTION]
Fix default-constructed Boolean expressions so they can be used as
empty accumulators for conjunction and disjunction without reading
uninitialized node state.

[ENTRY]
Module: float
What: bug
Expand Down
30 changes: 27 additions & 3 deletions gecode/minimodel.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1382,8 +1382,20 @@ namespace Gecode {
private:
/// Pointer to node for expression
Node* n;
/// Construct an expression, optionally treating an empty operand as an accumulator
BoolExpr(const BoolExpr& l, NodeType t, const BoolExpr& r,
bool accumulator);
friend GECODE_MINIMODEL_EXPORT
BoolExpr operator &&(const BoolExpr&, const BoolExpr&);
friend GECODE_MINIMODEL_EXPORT
BoolExpr operator ||(const BoolExpr&, const BoolExpr&);
public:
/// Default constructor
/** Default constructor
*
* Creates an empty accumulator for direct use with the public && and ||
* accumulation operators. In every other context, the expression is
* invalid; materialization throws MiniModel::TooFewArguments.
*/
GECODE_MINIMODEL_EXPORT
BoolExpr(void);
/// Copy constructor
Expand Down Expand Up @@ -1444,10 +1456,22 @@ namespace Gecode {
/// Negated Boolean expression
GECODE_MINIMODEL_EXPORT BoolExpr
operator !(const BoolExpr&);
/// Conjunction of Boolean expressions
/** Conjunction of Boolean expressions
*
* A default-constructed BoolExpr is treated as an empty accumulator
* only when it is a direct operand of this operator, in either operand
* position. This permits repeated accumulation; an empty expression in
* any other context remains invalid until materialization.
*/
GECODE_MINIMODEL_EXPORT BoolExpr
operator &&(const BoolExpr&, const BoolExpr&);
/// Disjunction of Boolean expressions
/** Disjunction of Boolean expressions
*
* A default-constructed BoolExpr is treated as an empty accumulator
* only when it is a direct operand of this operator, in either operand
* position. This permits repeated accumulation; an empty expression in
* any other context remains invalid until materialization.
*/
GECODE_MINIMODEL_EXPORT BoolExpr
operator ||(const BoolExpr&, const BoolExpr&);
/// Exclusive-or of Boolean expressions
Expand Down
26 changes: 22 additions & 4 deletions gecode/minimodel/bool-expr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ namespace Gecode {
*
*/
BoolExpr::Node::Node(void)
: use(1), l(nullptr), r(nullptr), m(nullptr) {}
: use(1), same(0), t(NT_AND), l(nullptr), r(nullptr), m(nullptr) {}

BoolExpr::Node::~Node(void) {
delete m;
Expand Down Expand Up @@ -135,7 +135,23 @@ namespace Gecode {
}

BoolExpr::BoolExpr(const BoolExpr& l, NodeType t, const BoolExpr& r)
: n(new Node) {
: BoolExpr(l,t,r,false) {}

BoolExpr::BoolExpr(const BoolExpr& l, NodeType t, const BoolExpr& r,
bool accumulator) {
if (accumulator && ((t == NT_AND) || (t == NT_OR)) &&
(l.n->same == 0)) {
n = r.n;
n->use++;
return;
}
if (accumulator && ((t == NT_AND) || (t == NT_OR)) &&
(r.n->same == 0)) {
n = l.n;
n->use++;
return;
}
n = new Node;
int ls = ((l.n->t == t) || (l.n->t == NT_VAR)) ? l.n->same : 1;
int rs = ((r.n->t == t) || (r.n->t == NT_VAR)) ? r.n->same : 1;
n->same = ls+rs;
Expand Down Expand Up @@ -520,6 +536,8 @@ namespace Gecode {

NNF*
NNF::nnf(Region& r, Node* n, bool neg) {
if (n->same == 0)
throw MiniModel::TooFewArguments("BoolExpr");
switch (n->t) {
case BoolExpr::NT_VAR:
case BoolExpr::NT_RLIN:
Expand Down Expand Up @@ -600,11 +618,11 @@ namespace Gecode {

BoolExpr
operator &&(const BoolExpr& l, const BoolExpr& r) {
return BoolExpr(l,BoolExpr::NT_AND,r);
return BoolExpr(l,BoolExpr::NT_AND,r,true);
}
BoolExpr
operator ||(const BoolExpr& l, const BoolExpr& r) {
return BoolExpr(l,BoolExpr::NT_OR,r);
return BoolExpr(l,BoolExpr::NT_OR,r,true);
}
BoolExpr
operator ^(const BoolExpr& l, const BoolExpr& r) {
Expand Down
108 changes: 108 additions & 0 deletions test/int/mm-bool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,99 @@ namespace Test { namespace Int {
}
};

/// %Test Boolean expressions with default-constructed accumulators
class BoolExprDefaultAccumulator : public Test {
protected:
/// Boolean operation to accumulate
BoolOpcode o;
/// Whether the default expression starts on the right hand side
bool rhs;
public:
/// Create and register test
BoolExprDefaultAccumulator(BoolOpcode o0, bool rhs0)
: Test("MiniModel::BoolExpr::Default::"+
std::string(o0 == BO_AND ? "And" : "Or")+
(rhs0 ? "::Rhs" : "::Lhs"),3,0,1), o(o0), rhs(rhs0) {}
/// %Test whether \a x is solution
virtual bool solution(const Assignment& x) const {
int r = (o == BO_AND) ? (x[0] & x[1]) : (x[0] | x[1]);
return r == x[2];
}
/// Post constraint on \a x
virtual void post(Gecode::Space& home, Gecode::IntVarArray& x) {
using namespace Gecode;
Gecode::BoolExpr e;
Gecode::BoolExpr x0(channel(home,x[0]));
Gecode::BoolExpr x1(channel(home,x[1]));
if (o == BO_AND) {
e = rhs ? (x0 && e) : (e && x0);
e = e && x1;
} else {
e = rhs ? (x0 || e) : (e || x0);
e = e || x1;
}
rel(home, expr(home,e), IRT_EQ, channel(home,x[2]));
}
};

/// Invalid Boolean expressions that must fail when materialized
enum BoolExprInvalidKind {
BE_EMPTY,
BE_NOT_EMPTY,
BE_IMP_EMPTY_LHS,
BE_IMP_EMPTY_RHS,
BE_RIMP_EMPTY_LHS,
BE_RIMP_EMPTY_RHS,
BE_NOT_AND,
BE_NOT_OR,
BE_XOR_EMPTY,
BE_EQV_EMPTY,
BE_NEQ_EMPTY
};

/// %Test invalid Boolean expressions
class BoolExprInvalid : public Test {
protected:
/// Kind of invalid expression
BoolExprInvalidKind k;
public:
/// Create and register test
BoolExprInvalid(BoolExprInvalidKind k0, const std::string& s)
: Test("MiniModel::BoolExpr::Invalid::"+s,1,0,1), k(k0) {}
/// %Test whether an assignment is a solution
virtual bool solution(const Assignment&) const {
return false;
}
/// Post expression and check that materialization fails
virtual void post(Gecode::Space& home, Gecode::IntVarArray& x) {
using namespace Gecode;
BoolExpr empty;
BoolExpr v(channel(home,x[0]));
BoolExpr e;
switch (k) {
case BE_EMPTY: e = empty; break;
case BE_NOT_EMPTY: e = !empty; break;
case BE_IMP_EMPTY_LHS: e = empty >> v; break;
case BE_IMP_EMPTY_RHS: e = v >> empty; break;
case BE_RIMP_EMPTY_LHS: e = empty << v; break;
case BE_RIMP_EMPTY_RHS: e = v << empty; break;
case BE_NOT_AND: e = !empty && v; break;
case BE_NOT_OR: e = !empty || v; break;
case BE_XOR_EMPTY: e = empty ^ v; break;
case BE_EQV_EMPTY: e = empty == v; break;
case BE_NEQ_EMPTY: e = empty != v; break;
default: GECODE_NEVER;
}
try {
(void) expr(home,e);
} catch (MiniModel::TooFewArguments&) {
home.fail();
return;
}
home.fail();
}
};

const BoolInstr bi000[] = {
{BO_AND,0,1,0},{BO_AND,2,3,1},{BO_AND,0,1,0},
{BO_HLT,0,0,0}
Expand Down Expand Up @@ -4382,6 +4475,21 @@ namespace Test { namespace Int {
(void) new BoolElement("Expr::A",0);
(void) new BoolElement("Expr::B",1);
(void) new BoolElement("Rel",2);
(void) new BoolExprDefaultAccumulator(BO_AND,false);
(void) new BoolExprDefaultAccumulator(BO_AND,true);
(void) new BoolExprDefaultAccumulator(BO_OR,false);
(void) new BoolExprDefaultAccumulator(BO_OR,true);
(void) new BoolExprInvalid(BE_EMPTY,"Empty");
(void) new BoolExprInvalid(BE_NOT_EMPTY,"NotEmpty");
(void) new BoolExprInvalid(BE_IMP_EMPTY_LHS,"Imp::EmptyLhs");
(void) new BoolExprInvalid(BE_IMP_EMPTY_RHS,"Imp::EmptyRhs");
(void) new BoolExprInvalid(BE_RIMP_EMPTY_LHS,"RImp::EmptyLhs");
(void) new BoolExprInvalid(BE_RIMP_EMPTY_RHS,"RImp::EmptyRhs");
(void) new BoolExprInvalid(BE_NOT_AND,"NotAnd");
(void) new BoolExprInvalid(BE_NOT_OR,"NotOr");
(void) new BoolExprInvalid(BE_XOR_EMPTY,"Xor");
(void) new BoolExprInvalid(BE_EQV_EMPTY,"Eqv");
(void) new BoolExprInvalid(BE_NEQ_EMPTY,"Neq");
}
};

Expand Down
Loading