Skip to content

Commit

Permalink
refactor(ir): improve error messages and other minor refactors
Browse files Browse the repository at this point in the history
  • Loading branch information
@tohrnii
tohrnii committed Feb 24, 2023
1 parent 7fee3bd commit f6752b2
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 71 deletions.
5 changes: 4 additions & 1 deletion ir/src/constraints/constraint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ impl ConstraintDomain {

/// Returns true if this domain is a boundary domain (FirstRow or LastRow).
pub fn is_boundary(&self) -> bool {
*self == ConstraintDomain::FirstRow || *self == ConstraintDomain::LastRow
matches!(
*self,
ConstraintDomain::FirstRow | ConstraintDomain::LastRow
)
}
}

Expand Down
56 changes: 29 additions & 27 deletions ir/src/constraints/list_comprehension.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,15 @@ fn parse_named_trace_access(
named_trace_access.row_offset(),
)))
}
_ => Err(SemanticError::InvalidListComprehension(
"Iterable should be a trace column".to_string(),
))?,
_ => Err(SemanticError::InvalidListComprehension(format!(
"Iterable {ident} should be trace columns"
)))?,
}
}
Iterable::Range(_) => Err(SemanticError::InvalidListComprehension(
"Iterable cannot be of range type here".to_string(),
)),
Iterable::Range(_) => Err(SemanticError::InvalidListComprehension(format!(
"Iterable cannot be of type Range for named trace access {}",
named_trace_access.name()
))),
Iterable::Slice(ident, range) => {
let ident_type = symbol_table.get_type(ident.name())?;
match ident_type {
Expand All @@ -164,9 +165,9 @@ fn parse_named_trace_access(
named_trace_access.row_offset(),
)))
}
_ => Err(SemanticError::InvalidListComprehension(
"Iterable should be a trace column".to_string(),
))?,
_ => Err(SemanticError::InvalidListComprehension(format!(
"Iterable {ident} should be trace columns"
)))?,
}
}
},
Expand Down Expand Up @@ -239,15 +240,16 @@ fn get_iterable_len(
match ident_type {
IdentifierType::Variable(_, var_type) => match var_type.value() {
VariableType::Vector(vector) => Ok(vector.len()),
_ => Err(SemanticError::InvalidListComprehension(
"List comprehensions only allowed for vector variables.".to_string(),
)),
_ => Err(SemanticError::InvalidListComprehension(format!(
"Variable {} should be a vector for a valid list comprehension.",
ident.name()
))),
},
IdentifierType::PublicInput(size) => Ok(*size),
IdentifierType::TraceColumns(trace_columns) => Ok(trace_columns.size()),
_ => Err(SemanticError::InvalidListComprehension(
"IdentifierType {ident_type} not supported for list comprehensions".to_string(),
)),
_ => Err(SemanticError::InvalidListComprehension(format!(
"IdentifierType {ident_type} not supported for list comprehensions"
))),
}
}
Iterable::Range(range) | Iterable::Slice(_, range) => Ok(range.end() - range.start()),
Expand All @@ -263,9 +265,9 @@ fn validate_access(i: usize, size: usize) -> Result<(), SemanticError> {
if i < size {
Ok(())
} else {
Err(SemanticError::IndexOutOfRange(
"Invalid access index used in list comprehension".to_string(),
))
Err(SemanticError::IndexOutOfRange(format!(
"Invalid access index {i} used in list comprehension"
)))
}
}

Expand All @@ -288,7 +290,7 @@ fn build_iterable_context(lc: &ListComprehension) -> Result<IterableContext, Sem
Ok(iterable_context)
}

/// Builds an [Expression] from a given identifier.
/// Builds an [Expression] from a given identifier and the index i at which it is being accessed.
///
/// # Errors
/// - Returns an error if the identifier is not of type in set:
Expand Down Expand Up @@ -317,9 +319,9 @@ fn build_ident_expression(
Ok(vector[i].clone())
}
// TODO: Handle matrix access
_ => Err(SemanticError::InvalidListComprehension(
"Iterables should be vectors".to_string(),
))?,
_ => Err(SemanticError::InvalidListComprehension(format!(
"Iterable {ident} should be a vector"
)))?,
}
}
IdentifierType::PublicInput(size) => {
Expand All @@ -337,7 +339,7 @@ fn build_ident_expression(
)))
}
_ => Err(SemanticError::InvalidListComprehension(
"Invalid type for a vector".to_string(),
"{ident_type} is an invalid type for a vector".to_string(),
))?,
}
}
Expand Down Expand Up @@ -371,9 +373,9 @@ fn build_slice_ident_expression(
Ok(vector[range_start + i].clone())
}
// TODO: Handle matrix access
_ => Err(SemanticError::InvalidListComprehension(
"Iterables should be vectors".to_string(),
))?,
_ => Err(SemanticError::InvalidListComprehension(format!(
"Variable {ident} should be a vector for a valid list comprehension"
)))?,
}
}
IdentifierType::PublicInput(size) => {
Expand All @@ -391,7 +393,7 @@ fn build_slice_ident_expression(
)))
}
_ => Err(SemanticError::InvalidListComprehension(
"Invalid type for a vector".to_string(),
"{ident_type} is an invalid type for a vector".to_string(),
))?,
}
}
21 changes: 21 additions & 0 deletions ir/src/tests/integrity_constraints.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,24 @@ fn ic_op_exp() {
let result = AirIR::new(&parsed);
assert!(result.is_ok());
}

#[test]
fn err_non_const_exp_outside_lc() {
// non const exponents are not allowed outside of list comprehensions
let source = "
trace_columns:
main: [clk, fmp[2], ctx]
aux: [a, b, c[4], d[4]]
public_inputs:
stack_inputs: [16]
boundary_constraints:
enf c[2].first = 0
integrity_constraints:
enf clk = 2^ctx";

let parsed = parse(source).expect("Parsing failed");
let result = AirIR::new(&parsed);
assert!(result.is_err());
}
46 changes: 3 additions & 43 deletions ir/src/tests/list_comprehension.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,28 +32,8 @@ fn lc_with_const_exp() {
integrity_constraints:
let y = [col^7 for col in c]
enf clk = y[1]";

let parsed = parse(source).expect("Parsing failed");
let result = AirIR::new(&parsed);
assert!(result.is_ok());
}

#[test]
fn lc_with_named_trace_access_const_exp() {
let source = "
trace_columns:
main: [clk, fmp[2], ctx]
aux: [a, b, c[4], d[4]]
public_inputs:
stack_inputs: [16]
boundary_constraints:
enf c[2].first = 0
integrity_constraints:
let y = [col'^7 - col for col in c]
enf clk = y[1]";
let z = [col'^7 - col for col in c]
enf clk = y[1] + z[1]";

let parsed = parse(source).expect("Parsing failed");
let result = AirIR::new(&parsed);
Expand Down Expand Up @@ -177,7 +157,7 @@ fn lf_in_lc() {

let parsed = parse(source).expect("Parsing failed");
let result = AirIR::new(&parsed);
println!("{:?}", result);

assert!(result.is_ok());
}

Expand Down Expand Up @@ -222,26 +202,6 @@ fn err_index_out_of_range_lc_slice() {
assert!(result.is_err());
}

#[test]
fn err_non_const_exp_outside_lc() {
let source = "
trace_columns:
main: [clk, fmp[2], ctx]
aux: [a, b, c[4], d[4]]
public_inputs:
stack_inputs: [16]
boundary_constraints:
enf c[2].first = 0
integrity_constraints:
enf clk = 2^ctx";

let parsed = parse(source).expect("Parsing failed");
let result = AirIR::new(&parsed);
assert!(result.is_err());
}

#[test]
fn err_non_const_exp_ident_iterable() {
let source = "
Expand Down

0 comments on commit f6752b2

Please sign in to comment.