Skip to content
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.

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

fix: Wrong Evaluation of Constraint Expressions using Expansion #242

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 10 additions & 10 deletions src/transformer/ifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ fn raise_ifs(mut e: Node) -> Node {
.fold(true, |b, e| b && !matches!(e.e(), Expression::Void)));
//
match func {
Intrinsic::Add
Intrinsic::Neg
| Intrinsic::Inv
| Intrinsic::Normalize
| Intrinsic::Exp
| Intrinsic::Add
| Intrinsic::Sub
| Intrinsic::Mul
| Intrinsic::VectorAdd
Expand All @@ -147,7 +151,7 @@ fn raise_ifs(mut e: Node) -> Node {
let new_then = func.unchecked_call(&then_args).unwrap();
let mut new_args = vec![cond, new_then];
// Pull out false branch (if applicable):
// (func a b (if cond then else) c)
// (func a b (if cond c d) e)
// ==> (if !cond (func a b d e))
if let Some(arg_else) = args_if.get(2).cloned() {
let mut else_args = args.clone();
Expand All @@ -163,13 +167,7 @@ fn raise_ifs(mut e: Node) -> Node {
}
e
}
Intrinsic::IfZero
| Intrinsic::IfNotZero
| Intrinsic::Neg
| Intrinsic::Inv
| Intrinsic::Normalize
| Intrinsic::Exp
| Intrinsic::Begin => e,
Intrinsic::IfZero | Intrinsic::IfNotZero | Intrinsic::Begin => e,
}
}
Expression::List(xs) => {
Expand Down Expand Up @@ -330,7 +328,9 @@ pub fn expand_ifs(cs: &mut ConstraintSet) {
// Raise ifs
for c in cs.constraints.iter_mut() {
if let Constraint::Vanishes { expr, .. } = c {
*expr = Box::new(raise_ifs(*expr.clone()));
let nexpr = raise_ifs(*expr.clone());
// Replace old expression with new
*expr = Box::new(nexpr);
}
}
for c in cs.constraints.iter_mut() {
Expand Down
5 changes: 5 additions & 0 deletions tests/issue219_a.lisp
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
(defcolumns X)

(defconstraint Constraint ()
(neq! (if (is-zero 1) X X) X)
)
4 changes: 4 additions & 0 deletions tests/issue219_b.lisp
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
(defcolumns ST X)

(defconstraint constraint-test ()
(if-not-zero ST (is-not-zero! (if (vanishes! X) 1 1))))
4 changes: 4 additions & 0 deletions tests/issue219_c.lisp
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
(defcolumns ST X)

(defconstraint constraint-test ()
(if-not-zero ST (vanishes! (- 1 (if (vanishes! X) 1 1)))))
4 changes: 4 additions & 0 deletions tests/issue219_d.lisp
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
(defcolumns ST X Y)

(defconstraint constraint-test ()
(if-not-zero ST (vanishes! (- 1 (if (vanishes! X) Y 1)))))
7 changes: 7 additions & 0 deletions tests/issue241_a.lisp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
(defcolumns X)

(defconstraint constraint-1 () (is-not-zero! (if (is-zero 1) 1 1)))
(defconstraint constraint-2 () (eq! (is-not-zero! (if (is-zero X) 1 X)) 0))
(defconstraint constraint-3 () (eq! (~and! (is-not-zero! (if (is-zero 0) 1 0)) 0) 0))
(defconstraint constraint-4 () (is-not-zero! (if (is-zero 0) 1 0)))
(defconstraint constraint-5 () (is-not-zero! (if (is-zero X) 1 X)))
6 changes: 6 additions & 0 deletions tests/issue241_b.lisp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
(defcolumns X ST)

;; ST ==> x != 1
(defconstraint constraint-6 () (if-not-zero ST (neq! 1 (if (is-zero 0) X X))))
;; ST ==> x != 0
(defconstraint constraint-7 () (if-not-zero ST (neq! 0 (if (is-zero X) X X))))
75 changes: 69 additions & 6 deletions tests/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
/// be accepted and which should be rejected. In essence, we are
/// comparing a Rust function (the model) against the constraints
/// evaluated on the trace.
struct Model {

Check warning on line 7 in tests/models.rs

View workflow job for this annotation

GitHub Actions / build

struct `Model` is never constructed
/// The name of this test (which should correspond to a lisp
/// file).
name: &'static str,
Expand All @@ -16,7 +16,7 @@
}

impl Model {
const MIN_ELEMENT: isize = -1;

Check warning on line 19 in tests/models.rs

View workflow job for this annotation

GitHub Actions / build

multiple associated items are never used
const MAX_ELEMENT: isize = 1;

/// Generate all traces matching the model configuration upto
Expand Down Expand Up @@ -111,7 +111,7 @@

/// Represents an individial trace which, for a given number of rows,
/// contains values for each of the columns.
struct Trace {

Check warning on line 114 in tests/models.rs

View workflow job for this annotation

GitHub Actions / build

struct `Trace` is never constructed
/// Simplistic column schema.
cols: &'static [&'static str],
/// The trace data, whose length must be divisible by `width`.
Expand All @@ -122,7 +122,7 @@
}

impl Trace {
pub fn new(cols: &'static [&'static str], data: Vec<isize>) -> Self {

Check warning on line 125 in tests/models.rs

View workflow job for this annotation

GitHub Actions / build

associated items `new`, `height`, `width`, `col`, and `get` are never used
Self {
cols,
data,
Expand Down Expand Up @@ -163,7 +163,7 @@

/// Represents a single column which can contain zero (or more) rows
/// of padding at the beginning.
struct Column<'a> {

Check warning on line 166 in tests/models.rs

View workflow job for this annotation

GitHub Actions / build

struct `Column` is never constructed
// The raw data underlying this column
data: &'a [isize],
// The number of rows of passing which are assumed to be prepended
Expand All @@ -189,7 +189,7 @@

/// The master list of active models. Tests will be automatically
/// generated for each item in this list.
static MODELS: &[Model] = &[

Check warning on line 192 in tests/models.rs

View workflow job for this annotation

GitHub Actions / build

static `MODELS` is never used
Model {
name: "arrays_1",
cols: &["A", "B_1", "B_2", "B_3"],
Expand Down Expand Up @@ -230,6 +230,36 @@
cols: &["X"],
oracle: Some(|_| false),
},
Model {
name: "issue241_a",
cols: &["X"],
oracle: Some(|_| true),
},
Model {
name: "issue241_b",
cols: &["ST", "X"],
oracle: Some(issue241_b_oracle),
},
Model {
name: "issue219_a",
cols: &["X"],
oracle: Some(|_| false),
},
Model {
name: "issue219_b",
cols: &["ST", "X"],
oracle: Some(|_| true),
},
Model {
name: "issue219_c",
cols: &["ST", "X"],
oracle: Some(|_| true),
},
Model {
name: "issue219_d",
cols: &["ST", "X", "Y"],
oracle: Some(issue219_d_oracle),
},
];

// ===================================================================
Expand All @@ -237,7 +267,7 @@
// ===================================================================

#[allow(non_snake_case)]
fn arrays_1_oracle(tr: &Trace) -> bool {

Check warning on line 270 in tests/models.rs

View workflow job for this annotation

GitHub Actions / build

function `arrays_1_oracle` is never used
let (A, B_1, B_2, B_3) = (tr.col("A"), tr.col("B_1"), tr.col("B_2"), tr.col("B_3"));

for k in 0..tr.height() {
Expand All @@ -251,12 +281,12 @@
true
}

// // ===================================================================
// // IsZero
// // ===================================================================
// ===================================================================
// IsZero
// ===================================================================

#[allow(non_snake_case)]
fn iszero_oracle(tr: &Trace) -> bool {

Check warning on line 289 in tests/models.rs

View workflow job for this annotation

GitHub Actions / build

function `iszero_oracle` is never used
let (A, B) = (tr.col("A"), tr.col("B"));

for k in 0..tr.height() {
Expand All @@ -267,9 +297,9 @@
true
}

// // ===================================================================
// // Shift
// // ===================================================================
// ===================================================================
// Shift
// ===================================================================

#[allow(non_snake_case)]
fn shift_1_oracle(tr: &Trace) -> bool {
Expand Down Expand Up @@ -323,3 +353,36 @@
}
true
}

// ===================================================================
// Issue 241
// ===================================================================

#[allow(non_snake_case)]
fn issue241_b_oracle(tr: &Trace) -> bool {
let (X, ST) = (tr.col("X"), tr.col("ST"));
for k in 0..tr.height() {
let c1 = ST[k] == 0 || X[k] != 1;
let c2 = ST[k] == 0 || X[k] != 0;
if !c1 || !c2 {
return false;
}
}
true
}

// ===================================================================
// Issue 219
// ===================================================================

#[allow(non_snake_case)]
fn issue219_d_oracle(tr: &Trace) -> bool {
let (ST, X, Y) = (tr.col("ST"), tr.col("X"), tr.col("Y"));

for k in 0..tr.height() {
if ST[k] != 0 && X[k] == 0 && Y[k] != 1 {
return false;
}
}
true
}
Loading