test: add more tests for edge cases for const parsing

codegen/winterfell/src/air/boundary_constraints.rs

@@ -103,7 +103,7 @@ impl Codegen for BoundaryExpr {
                     format!("Felt::new({})", value)
                 }
             }
-            Self::VecElem(vector_access) => {
+            Self::VectorAccess(vector_access) => {
                 format!("self.{}[{}]", vector_access.name(), vector_access.idx())
             }
             Self::Rand(index) => {
@@ -133,7 +133,7 @@ impl Codegen for BoundaryExpr {
             Self::Exp(lhs, rhs) => {
                 format!("({}).exp({})", lhs.to_string(is_aux_constraint), rhs)
             }
-            BoundaryExpr::Elem(_) | BoundaryExpr::MatrixElem(_) => todo!(),
+            BoundaryExpr::Elem(_) | BoundaryExpr::MatrixAccess(_) => todo!(),
         }
     }
 }

ir/src/boundary_constraints.rs

@@ -121,8 +121,8 @@ fn validate_expression(
     expr: &ast::BoundaryExpr,
 ) -> Result<(), SemanticError> {
     match expr {
-        BoundaryExpr::VecElem(vector_access) => {
-            symbol_table.validate_public_input(vector_access.name(), vector_access.idx() as usize)
+        BoundaryExpr::VectorAccess(vector_access) => {
+            symbol_table.validate_public_input(vector_access.name(), vector_access.idx())
         }
         BoundaryExpr::Add(lhs, rhs) | BoundaryExpr::Sub(lhs, rhs) => {
             validate_expression(symbol_table, lhs)?;

ir/src/transition_constraints/graph.rs

@@ -139,7 +139,7 @@ impl AlgebraicGraph {
                 let node_index = self.insert_op(Operation::Exp(lhs, rhs as usize));
                 Ok((constraint_type, node_index))
             }
-            TransitionExpr::VecElem(_) | TransitionExpr::MatrixElem(_) => todo!(),
+            TransitionExpr::VectorAccess(_) | TransitionExpr::MatrixAccess(_) => todo!(),
         }
     }
 

parser/src/ast/boundary_constraints.rs

@@ -63,12 +63,12 @@ pub enum BoundaryExpr {
     Const(u64),
     /// Represents any named constant or variable.
     Elem(Identifier),
-    /// Represents an element inside a constant or variable vector. The index is the index of
-    /// this value inside the vector.
-    VecElem(VectorAccess),
-    /// Represents an element inside a constant or variable matrix. Indices idx_row and idx_col
-    /// are the indices of this value inside the matrix.
-    MatrixElem(MatrixAccess),
+    /// Represents an element inside a constant or variable vector. [VectorAccess] contains the
+    /// name of the vector and the index of the element to access.
+    VectorAccess(VectorAccess),
+    /// Represents an element inside a constant or variable matrix. [MatrixAccess] contains the
+    /// name of the matrix and indices of the element to access.
+    MatrixAccess(MatrixAccess),
     /// Represents a random value provided by the verifier. The inner value is the index of this
     /// random value in the array of all random values.
     Rand(usize),

parser/src/ast/mod.rs

@@ -64,6 +64,13 @@ pub struct TraceCols {
 #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone)]
 pub struct Identifier(pub String);
 
+impl Identifier {
+    /// Returns the name of the identifier.
+    pub fn name(&self) -> &str {
+        &self.0
+    }
+}
+
 impl fmt::Display for Identifier {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", &self.0)
@@ -74,50 +81,57 @@ impl fmt::Display for Identifier {
 #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone)]
 pub struct VectorAccess {
     name: Identifier,
-    idx: u64,
+    idx: usize,
 }
 
 impl VectorAccess {
-    pub fn new(name: Identifier, idx: u64) -> Self {
+    /// Creates a new [VectorAccess] instance with the specified identifier name and index.
+    pub fn new(name: Identifier, idx: usize) -> Self {
         Self { name, idx }
     }
 
+    /// Returns the name of the vector.
     pub fn name(&self) -> &str {
-        &self.name.0
+        self.name.name()
     }
 
-    pub fn idx(&self) -> u64 {
+    /// Returns the index of the vector access.
+    pub fn idx(&self) -> usize {
         self.idx
     }
 }
 
-/// [MatrixAccess] is used to represent an element inside vector at the specified row and column
+/// [MatrixAccess] is used to represent an element inside a matrix at the specified row and column
 /// indices.
 #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone)]
 pub struct MatrixAccess {
     name: Identifier,
-    col_idx: u64,
-    row_idx: u64,
+    row_idx: usize,
+    col_idx: usize,
 }
 
 impl MatrixAccess {
-    pub fn new(name: Identifier, col_idx: u64, row_idx: u64) -> Self {
+    /// Creates a new [MatrixAccess] instance with the specified identifier name and indices.
+    pub fn new(name: Identifier, col_idx: usize, row_idx: usize) -> Self {
         Self {
             name,
-            col_idx,
             row_idx,
+            col_idx,
         }
     }
 
+    /// Returns the name of the matrix.
     pub fn name(&self) -> &str {
         &self.name.0
     }
 
-    pub fn col_idx(&self) -> u64 {
-        self.col_idx
+    /// Returns the row index of the matrix access.
+    pub fn row_idx(&self) -> usize {
+        self.row_idx
     }
 
-    pub fn row_idx(&self) -> u64 {
-        self.row_idx
+    /// Returns the column index of the matrix access.
+    pub fn col_idx(&self) -> usize {
+        self.col_idx
     }
 }

parser/src/ast/transition_constraints.rs

@@ -32,9 +32,14 @@ impl TransitionConstraint {
 #[derive(Debug, PartialEq, Clone)]
 pub enum TransitionExpr {
     Const(u64),
+    /// Represents any named constant or variable.
     Elem(Identifier),
-    VecElem(VectorAccess),
-    MatrixElem(MatrixAccess),
+    /// Represents an element inside a constant or variable vector. [VectorAccess] contains the
+    /// name of the vector and the index of the element to access.
+    VectorAccess(VectorAccess),
+    /// Represents an element inside a constant or variable matrix. [MatrixAccess] contains the
+    /// name of the matrix and indices of the element to access.
+    MatrixAccess(MatrixAccess),
     Next(Identifier),
     /// Represents a random value provided by the verifier. The inner value is the index of this
     /// random value in the array of all random values.

parser/src/error.rs

@@ -11,5 +11,5 @@ pub enum ParseError {
     InvalidInt(String),
     InvalidTraceCols(String),
     MissingMainTraceCols(String),
-    LowercaseConstName(String),
+    InvalidConst(String),
 }

parser/src/parser/grammar.lalrpop

@@ -6,7 +6,7 @@ use crate::{
         Identifier, Source, SourceSection, TraceCols, PublicInput, PeriodicColumn, VectorAccess,
         MatrixAccess
     }, error::{Error, ParseError::{InvalidInt, InvalidTraceCols, MissingMainTraceCols,
-    LowercaseConstName}}, lexer::Token
+    InvalidConst}}, lexer::Token
 };
 use std::str::FromStr;
 use lalrpop_util::ParseError;
@@ -67,8 +67,8 @@ AuxCols: Vec<Identifier> = {
 // CONSTANTS
 // ================================================================================================
 
-// Constants section is optional but cannot be empty. If the section is declared, then at least one
-// constant is required.
+// The constants section is optional but cannot be empty. If the section is declared, then at least
+// one constant is required.
 Constants: Vec<Constant> = {
     "constants" ":" <constants: Constant+> => constants
 }
@@ -83,11 +83,11 @@ Constant: Constant = {
 }
 
 ConstName: Identifier = {
-    <n: Identifier> =>? if n.0.chars().all(|v| v.is_uppercase()) {
-        Ok(Identifier(n.to_string()))
+    <name: Identifier> =>? if name.0.chars().all(|v| v.is_uppercase()) {
+        Ok(Identifier(name.to_string()))
     } else {
         Err(ParseError::User {
-            error: Error::ParseError(LowercaseConstName(
+            error: Error::ParseError(InvalidConst(
                 format!("The constant name should be uppercase: {}", <>).to_string()
             ))
         })
@@ -103,7 +103,7 @@ PublicInputs: Vec<PublicInput> = {
 }
 
 PublicInput: PublicInput = {
-    <name: Identifier> ":" "[" <size: Num_u64> "]" => PublicInput::new(name, size),
+    <name: Identifier> ":" <size: Size> => PublicInput::new(name, size),
 }
 
 // PERIODIC COLUMNS
@@ -156,13 +156,11 @@ BoundaryFactor: BoundaryExpr = {
 BoundaryAtom: BoundaryExpr = {
     "(" <BoundaryExpr> ")",
     <lexpr: BoundaryAtom> "^" <num: Num_u64> => BoundaryExpr::Exp(Box::new(lexpr), num),
-    "$rand" "[" <n: Num_u64> "]" => BoundaryExpr::Rand(n as usize),
+    "$rand" <idx: Index> => BoundaryExpr::Rand(idx),
     <n: Num_u64> => BoundaryExpr::Const(n),
     <ident: Identifier> => BoundaryExpr::Elem(ident),
-    <ident: Identifier> "[" <idx: Num_u64> "]" => 
-        BoundaryExpr::VecElem(VectorAccess::new(ident, idx)),
-    <ident: Identifier> "[" <col_idx: Num_u64> "]" "[" <row_idx: Num_u64> "]" =>
-        BoundaryExpr::MatrixElem(MatrixAccess::new(ident, col_idx, row_idx))
+    <vector_access: VectorAccess> => BoundaryExpr::VectorAccess(vector_access),
+    <matrix_access: MatrixAccess> => BoundaryExpr::MatrixAccess(matrix_access)
 }
 
 // TRANSITION CONSTRAINTS
@@ -196,13 +194,11 @@ TransitionFactor: TransitionExpr = {
 TransitionAtom: TransitionExpr = {
     "(" <TransitionExpr> ")",
     <lexpr: TransitionAtom> "^" <num: Num_u64> => TransitionExpr::Exp(Box::new(lexpr), num),
-    "$rand" "[" <n: Num_u64> "]" => TransitionExpr::Rand(n as usize),
+    "$rand" <idx: Index> => TransitionExpr::Rand(idx),
     <n: Num_u64> => TransitionExpr::Const(n),
     <ident: Identifier> => TransitionExpr::Elem(ident),
-    <ident: Identifier> "[" <idx: Num_u64> "]" =>
-        TransitionExpr::VecElem(VectorAccess::new(ident, idx)),
-    <ident: Identifier> "[" <col_idx: Num_u64> "]" "[" <row_idx: Num_u64> "]" =>
-        TransitionExpr::MatrixElem(MatrixAccess::new(ident, col_idx, row_idx)),
+    <vector_access: VectorAccess> => TransitionExpr::VectorAccess(vector_access),
+    <matrix_access: MatrixAccess> => TransitionExpr::MatrixAccess(matrix_access),
     <ident: Identifier> "'" => TransitionExpr::Next(ident)
 }
 
@@ -225,6 +221,23 @@ Vector<T>: Vec<T> = {
     }
 }
 
+Size: u64 = {
+    "[" <size: Num_u64> "]" => size
+}
+
+Index: usize = {
+    "[" <idx: Num_u64> "]" => idx as usize
+}
+
+VectorAccess: VectorAccess = {
+    <ident: Identifier> <idx: Index> => VectorAccess::new(ident, idx)
+}
+
+MatrixAccess: MatrixAccess = {
+    <ident: Identifier> <row: Index> <col: Index> =>
+        MatrixAccess::new(ident, row, col)
+}
+
 Identifier: Identifier = {
     <n:identifier> => Identifier(n.to_string())
 }

parser/src/parser/tests/boundary_constraints.rs

@@ -93,7 +93,7 @@ fn boundary_constraint_with_pub_input() {
             boundary_constraints: vec![BoundaryConstraint::new(
                 Identifier("clk".to_string()),
                 Boundary::First,
-                BoundaryExpr::VecElem(VectorAccess::new(Identifier("a".to_string()), 0)),
+                BoundaryExpr::VectorAccess(VectorAccess::new(Identifier("a".to_string()), 0)),
             )],
         }),
     ]);
@@ -113,7 +113,7 @@ fn boundary_constraint_with_expr() {
                 BoundaryExpr::Add(
                     Box::new(BoundaryExpr::Add(
                         Box::new(BoundaryExpr::Const(5)),
-                        Box::new(BoundaryExpr::VecElem(VectorAccess::new(
+                        Box::new(BoundaryExpr::VectorAccess(VectorAccess::new(
                             Identifier("a".to_string()),
                             3,
                         ))),
@@ -151,12 +151,12 @@ fn boundary_constraint_with_const() {
                 BoundaryExpr::Sub(
                     Box::new(BoundaryExpr::Add(
                         Box::new(BoundaryExpr::Elem(Identifier("A".to_string()))),
-                        Box::new(BoundaryExpr::VecElem(VectorAccess::new(
+                        Box::new(BoundaryExpr::VectorAccess(VectorAccess::new(
                             Identifier("B".to_string()),
                             1,
                         ))),
                     )),
-                    Box::new(BoundaryExpr::MatrixElem(MatrixAccess::new(
+                    Box::new(BoundaryExpr::MatrixAccess(MatrixAccess::new(
                         Identifier("C".to_string()),
                         0,
                         1,

parser/src/parser/tests/constants.rs

@@ -37,24 +37,6 @@ fn constants_vectors() {
     build_parse_test!(source).expect_ast(expected);
 }
 
-#[test]
-fn constants_vectors_wrong() {
-    let source = "constants:
-        A: [1, 2, 3, 4]
-        B: [5, 6, 7, 8]";
-    let expected = Source(vec![SourceSection::Constants(vec![
-        Constant::new(
-            Identifier("A".to_string()),
-            ConstantType::Vector(vec![1, 2, 3, 4]),
-        ),
-        Constant::new(
-            Identifier("B".to_string()),
-            ConstantType::Vector(vec![5, 6, 7, 8]),
-        ),
-    ])]);
-    build_parse_test!(source).expect_ast(expected);
-}
-
 #[test]
 fn constants_matrices() {
     let source = "constants:
@@ -73,6 +55,19 @@ fn constants_matrices() {
     build_parse_test!(source).expect_ast(expected);
 }
 
+#[test]
+fn const_matrix_unequal_number_of_cols() {
+    // This is invalid since the number of columns for the two rows are unequal. However this
+    // validation happens at the IR level.
+    let source = "constants:
+    A: [[1, 2], [3, 4, 5]]";
+    let expected = Source(vec![SourceSection::Constants(vec![Constant::new(
+        Identifier("A".to_string()),
+        ConstantType::Matrix(vec![vec![1, 2], vec![3, 4, 5]]),
+    )])]);
+    build_parse_test!(source).expect_ast(expected);
+}
+
 #[test]
 fn error_empty_constants_section() {
     let source = "
@@ -86,8 +81,52 @@ fn err_lowercase_constant_name() {
     let source = "constants:
     Ab: [[1, 2], [3, 4]]
     C: [[5, 6], [7, 8]]";
-    let error = Error::ParseError(ParseError::LowercaseConstName(
+    let error = Error::ParseError(ParseError::InvalidConst(
         "The constant name should be uppercase: Ab".to_string(),
     ));
     build_parse_test!(source).expect_error(error);
 }
+
+#[test]
+fn err_consts_with_non_int_values() {
+    let source = "constants:
+        A: a
+        B: 2";
+    build_parse_test!(source).expect_unrecognized_token();
+}
+
+#[test]
+fn err_const_vectors_with_non_int_values() {
+    let source = "constants:
+        A: [1, a]
+        B: [2, 4]";
+    build_parse_test!(source).expect_unrecognized_token();
+}
+
+#[test]
+fn err_vector_with_trailing_comma() {
+    let source = "constants:
+    A: [1, ]";
+    build_parse_test!(source).expect_unrecognized_token();
+}
+
+#[test]
+fn err_matrix_with_trailing_comma() {
+    let source = "constants:
+    A: [[1, 2], ]";
+    build_parse_test!(source).expect_unrecognized_token();
+}
+
+#[test]
+fn err_matrix_mixed_element_types() {
+    let source = "constants:
+    A: [1, [1, 2]]";
+    build_parse_test!(source).expect_unrecognized_token();
+}
+
+#[test]
+fn err_invalid_matrix_element() {
+    let source = "constants:
+    A: [[1, 2], [3, [4, 5]]]";
+    build_parse_test!(source).expect_unrecognized_token();
+}