1
0
Fork 0
mirror of synced 2025-09-24 04:40:05 +00:00

move slice bound checking from ast conversion to semantic checking to fail more gracefully

This commit is contained in:
schaeff 2020-07-31 14:36:55 +02:00
parent 0567e20408
commit 69e56a9403
4 changed files with 46 additions and 26 deletions

View file

@ -409,23 +409,13 @@ impl<'ast, T: Field> From<pest::Spread<'ast>> for absy::SpreadNode<'ast, T> {
}
}
impl<'ast, T: Field> From<pest::Range<'ast>> for absy::RangeNode<T> {
fn from(range: pest::Range<'ast>) -> absy::RangeNode<T> {
impl<'ast, T: Field> From<pest::Range<'ast>> for absy::RangeNode<'ast, T> {
fn from(range: pest::Range<'ast>) -> absy::RangeNode<'ast, T> {
use absy::NodeValue;
let from = range
.from
.map(|e| match absy::ExpressionNode::from(e.0).value {
absy::Expression::FieldConstant(n) => n,
e => unimplemented!("Range bounds should be constants, found {}", e),
});
let from = range.from.map(|e| absy::ExpressionNode::from(e.0));
let to = range
.to
.map(|e| match absy::ExpressionNode::from(e.0).value {
absy::Expression::FieldConstant(n) => n,
e => unimplemented!("Range bounds should be constants, found {}", e),
});
let to = range.to.map(|e| absy::ExpressionNode::from(e.0));
absy::Range { from, to }.span(range.span)
}

View file

@ -395,7 +395,7 @@ impl<'ast, T: fmt::Debug> fmt::Debug for SpreadOrExpression<'ast, T> {
/// The index in an array selector. Can be a range or an expression.
#[derive(Clone, PartialEq)]
pub enum RangeOrExpression<'ast, T> {
Range(RangeNode<T>),
Range(RangeNode<'ast, T>),
Expression(ExpressionNode<'ast, T>),
}
@ -439,14 +439,14 @@ pub struct Spread<'ast, T> {
/// A range
#[derive(Clone, PartialEq)]
pub struct Range<T> {
pub from: Option<T>,
pub to: Option<T>,
pub struct Range<'ast, T> {
pub from: Option<ExpressionNode<'ast, T>>,
pub to: Option<ExpressionNode<'ast, T>>,
}
pub type RangeNode<T> = Node<Range<T>>;
pub type RangeNode<'ast, T> = Node<Range<'ast, T>>;
impl<'ast, T: fmt::Display> fmt::Display for Range<T> {
impl<'ast, T: fmt::Display> fmt::Display for Range<'ast, T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
@ -463,7 +463,7 @@ impl<'ast, T: fmt::Display> fmt::Display for Range<T> {
}
}
impl<'ast, T: fmt::Debug> fmt::Debug for Range<T> {
impl<'ast, T: fmt::Debug> fmt::Debug for Range<'ast, T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "Range({:?}, {:?})", self.from, self.to)
}

View file

@ -91,7 +91,7 @@ impl<'ast> NodeValue for Variable<'ast> {}
impl<'ast> NodeValue for Parameter<'ast> {}
impl<'ast> NodeValue for Import<'ast> {}
impl<'ast, T: fmt::Display + fmt::Debug + PartialEq> NodeValue for Spread<'ast, T> {}
impl<T: fmt::Display + fmt::Debug + PartialEq> NodeValue for Range<T> {}
impl<'ast, T: fmt::Display + fmt::Debug + PartialEq> NodeValue for Range<'ast, T> {}
impl<T: PartialEq> PartialEq for Node<T> {
fn eq(&self, other: &Node<T>) -> bool {

View file

@ -1644,17 +1644,47 @@ impl<'ast> Checker<'ast> {
let array_size = array.size();
let inner_type = array.inner_type().clone();
// check that the bounds are valid expressions
let from = r
.value
.from
.map(|v| v.to_dec_string().parse::<usize>().unwrap())
.unwrap_or(0);
.map(|e| self.check_expression(e, module_id, &types))
.unwrap_or(Ok(FieldElementExpression::Number(T::from(0)).into()))?;
let to = r
.value
.to
.map(|v| v.to_dec_string().parse::<usize>().unwrap())
.unwrap_or(array_size);
.map(|e| self.check_expression(e, module_id, &types))
.unwrap_or(Ok(FieldElementExpression::Number(T::from(0)).into()))?;
// check the bounds are field constants
// Note: it would be nice to allow any field expression, and check it's a constant after constant propagation,
// but it's tricky from a type perspective: the size of the slice changes the type of the resulting array,
// which doesn't work well with our static array approach. Enabling arrays to have unknown size introduces a lot
// of complexity in the compiler, as function selection in inlining requires knowledge of the array size, but
// determining array size potentially requires inlining and propagating. This suggests we would need semantic checking
// to happen iteratively with inlining and propagation, which we can't do now as we go from absy to typed_absy
let from = match from {
TypedExpression::FieldElement(FieldElementExpression::Number(n)) => Ok(n.to_dec_string().parse::<usize>().unwrap()),
e => Err(ErrorInner {
pos: Some(pos),
message: format!(
"Expected the lower bound of the range to be a constant field, found {}",
e
),
})
}?;
let to = match to {
TypedExpression::FieldElement(FieldElementExpression::Number(n)) => Ok(n.to_dec_string().parse::<usize>().unwrap()),
e => Err(ErrorInner {
pos: Some(pos),
message: format!(
"Expected the higher bound of the range to be a constant field, found {}",
e
),
})
}?;
match (from, to, array_size) {
(f, _, s) if f > s => Err(ErrorInner {