Merge pull request #254 from 0xPolygonMiden/grjte-refactor-clarity-3

refactor: change access types to BindingAccess

Author: grjte

air-script-core/src/access.rs

@@ -1,61 +1,64 @@
 use super::Identifier;
+use std::fmt::Display;
 
-/// [VectorAccess] is used to represent an element inside vector at the specified index.
-#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone)]
-pub struct VectorAccess {
-    name: Identifier,
-    idx: usize,
+/// Defines the type of an access into a binding such as a [ConstantBinding] or a [VariableBinding].
+///
+/// - Default: accesses the entire bound value, which could be a scalar, vector, or matrix.
+/// - Vector: indexes into the bound value at the specified index. The result could be either a
+///   single value or a vector, depending on the type of the original binding. This is not allowed
+///   for bindings to scalar values and will result in an error.
+/// - Matrix: indexes into the bound value at the specified row and column. The result is a single
+///   value. This [AccessType] is not allowed for bindings to scalar or vector values and will
+///   result in an error.
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub enum AccessType {
+    Default,
+    Vector(usize),
+    /// Access into a matrix, with the values referring to the row and column indices respectively.
+    Matrix(usize, usize),
 }
 
-impl VectorAccess {
-    /// 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.name()
-    }
-
-    /// Returns the index of the vector access.
-    pub fn idx(&self) -> usize {
-        self.idx
+impl Display for AccessType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Default => write!(f, "direct reference by name"),
+            Self::Vector(_) => write!(f, "vector"),
+            Self::Matrix(_, _) => write!(f, "matrix"),
+        }
     }
 }
 
-/// [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 {
+/// [BindingAccess] is used to indicate referencing all or part of an identifier that is bound to a
+/// value, such as a [ConstantBinding] or a [VariableBinding].
+///
+/// - `name`: is the identifier of the [ConstantBinding] or [VariableBinding] being accessed.
+/// - `access_type`: specifies the [AccessType] by which the identifier is being accessed.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub struct BindingAccess {
     name: Identifier,
-    row_idx: usize,
-    col_idx: usize,
+    access_type: AccessType,
 }
 
-impl MatrixAccess {
-    /// Creates a new [MatrixAccess] instance with the specified identifier name and indices.
-    pub fn new(name: Identifier, row_idx: usize, col_idx: usize) -> Self {
-        Self {
-            name,
-            row_idx,
-            col_idx,
-        }
+impl BindingAccess {
+    pub fn new(name: Identifier, access_type: AccessType) -> Self {
+        Self { name, access_type }
+    }
+
+    pub fn ident(&self) -> &Identifier {
+        &self.name
     }
 
-    /// Returns the name of the matrix.
     pub fn name(&self) -> &str {
         self.name.name()
     }
 
-    /// Returns the row index of the matrix access.
-    pub fn row_idx(&self) -> usize {
-        self.row_idx
+    /// Gets the access type of this [BindingAccess].
+    pub fn access_type(&self) -> &AccessType {
+        &self.access_type
     }
 
-    /// Returns the column index of the matrix access.
-    pub fn col_idx(&self) -> usize {
-        self.col_idx
+    pub fn into_parts(self) -> (Identifier, AccessType) {
+        (self.name, self.access_type)
     }
 }
 

air-script-core/src/expression.rs

@@ -1,17 +1,11 @@
-use super::{Identifier, ListFolding, MatrixAccess, TraceAccess, TraceBindingAccess, VectorAccess};
+use super::{BindingAccess, Identifier, ListFolding, TraceAccess, TraceBindingAccess};
 
 /// Arithmetic expressions for evaluation of constraints.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub enum Expression {
     Const(u64),
-    /// Represents any named constant or variable.
-    Elem(Identifier),
-    /// 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 reference to all or part of a constant, variable, or trace binding.
+    BindingAccess(BindingAccess),
     TraceAccess(TraceAccess),
     TraceBindingAccess(TraceBindingAccess),
     /// Represents a random value provided by the verifier. The first inner value is the name of

air-script-core/src/lib.rs

@@ -1,5 +1,5 @@
 mod access;
-pub use access::{Iterable, MatrixAccess, Range, VectorAccess};
+pub use access::{AccessType, BindingAccess, Iterable, Range};
 
 mod constant;
 pub use constant::{ConstantBinding, ConstantValueExpr};

air-script-core/src/trace.rs

@@ -4,55 +4,17 @@ use super::{Identifier, Range};
 // ================================================================================================
 pub type TraceSegment = u8;
 
-/// [TraceBinding] is used to represent one or more columns in the execution trace that are bound to
-/// a name. For single columns, the size is 1. For groups, the size is the number of columns in the
-/// group. The offset is the column index in the trace where the first column of the binding starts.
-#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd)]
-pub struct TraceBinding {
-    binding: Identifier,
-    trace_segment: TraceSegment,
-    offset: usize,
-    size: usize,
-}
-
-impl TraceBinding {
-    /// Creates a new trace binding.
-    pub fn new(binding: Identifier, trace_segment: usize, offset: usize, size: u64) -> Self {
-        Self {
-            binding,
-            trace_segment: trace_segment as TraceSegment,
-            offset,
-            size: size as usize,
-        }
-    }
-
-    /// Returns the name of the trace binding.
-    pub fn name(&self) -> &str {
-        self.binding.name()
-    }
-
-    /// Returns the trace segment of the trace binding.
-    pub fn trace_segment(&self) -> TraceSegment {
-        self.trace_segment
-    }
-
-    /// Returns the offset of the trace binding.
-    pub fn offset(&self) -> usize {
-        self.offset
-    }
-
-    /// Returns the size of the trace binding.
-    pub fn size(&self) -> usize {
-        self.size
-    }
-}
-
-/// [TraceAccess] is used to represent accessing an element in the execution trace during
-/// constraint evaluation. The trace_segment specifies
-/// how many trace commitments have preceded the specified segment. `col_idx` specifies the index
-/// of the column within that trace segment, and `row_offset` specifies the offset from the current
-/// row. For example, an element in the "next" row of the "main" trace would be specified by
-/// a trace_segment of 0 and a row_offset of 1.
+/// [TraceAccess] is used to represent accessing one or more elements in the execution trace during
+/// constraint evaluation.
+///
+/// - `trace_segment`: specifies how many trace commitments have preceded the specified segment.
+/// - `col_idx`: specifies the index of the column within that trace segment at which the access
+///   starts.
+/// - `size`: refers to how many columns are being accessed.
+/// - `row_offset`: specifies the offset from the current row.
+///
+/// For example, a single element in the "next" row of
+/// the "main" trace would be specified by a trace_segment of 0, a size of 1, and a row_offset of 1.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 pub struct TraceAccess {
     trace_segment: TraceSegment,
@@ -98,24 +60,79 @@ impl TraceAccess {
     }
 }
 
-/// [TraceBindingAccess] is used to indicate a column in the trace by specifying its offset within
-/// a set of trace columns with the given identifier. If the identifier refers to a single column
-/// then the index is always zero.
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
-pub struct TraceBindingAccess {
+/// [TraceBinding] is used to represent one or more columns in the execution trace that are bound to
+/// a name. For single columns, the size is 1. For groups, the size is the number of columns in the
+/// group. The offset is the column index in the trace where the first column of the binding starts.
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub struct TraceBinding {
     binding: Identifier,
-    col_offset: usize,
-    size: TraceBindingAccessSize,
-    row_offset: usize,
+    trace_segment: TraceSegment,
+    offset: usize,
+    size: usize,
+}
+
+impl TraceBinding {
+    /// Creates a new trace binding.
+    pub fn new(binding: Identifier, trace_segment: usize, offset: usize, size: u64) -> Self {
+        Self {
+            binding,
+            trace_segment: trace_segment as TraceSegment,
+            offset,
+            size: size as usize,
+        }
+    }
+
+    /// Returns the name of the trace binding.
+    pub fn name(&self) -> &str {
+        self.binding.name()
+    }
+
+    /// Returns the trace segment of the trace binding.
+    pub fn trace_segment(&self) -> TraceSegment {
+        self.trace_segment
+    }
+
+    /// Returns the offset of the trace binding.
+    pub fn offset(&self) -> usize {
+        self.offset
+    }
+
+    /// Returns the size of the trace binding.
+    pub fn size(&self) -> usize {
+        self.size
+    }
 }
 
+/// Indicates how much of a [TraceBinding] is being accessed.
+///
+/// TODO: check that this is correct for `Single`.
+/// - `Single`: only a single element from the [TraceBinding] is being referenced.
+/// - `Slice`: the specified range of the [TraceBinding] is being referenced.
+/// - `Full`: the entire [TraceBinding] is being referenced.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub enum TraceBindingAccessSize {
     Single,
     Slice(Range),
     Full,
 }
 
+/// [TraceBindingAccess] is used to indicate accessing a [TraceBinding].
+///
+/// - `binding`: is the identifier of the [TraceBinding] being accessed.
+/// - `col_offset`: specifies the column within the [TraceBinding] where the access starts. For
+///   example, if a [TraceBinding] has `offset` = 2 and the [TraceBindingAccess] has
+///   `col_offset` = 2, then the offset of the access within the trace segment will be 4. If the
+///   [TraceBinding] refers to a single column, then this value will be zero.
+/// - `size`: specifies how much of the [TraceBinding] is being accessed.
+/// - `row_offset`: specifies the offset from the current row.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub struct TraceBindingAccess {
+    binding: Identifier,
+    col_offset: usize,
+    size: TraceBindingAccessSize,
+    row_offset: usize,
+}
+
 impl TraceBindingAccess {
     pub fn new(
         binding: Identifier,

codegen/winterfell/src/air/graph.rs

@@ -1,7 +1,6 @@
-use ir::TraceAccess;
-
 use super::{
-    AirIR, ConstantValue, ElemType, IntegrityConstraintDegree, NodeIndex, Operation, Value,
+    AccessType, AirIR, ElemType, IntegrityConstraintDegree, NodeIndex, Operation, TraceAccess,
+    Value,
 };
 
 // RUST STRING GENERATION FOR THE CONSTRAINT GRAPH
@@ -102,42 +101,33 @@ impl Codegen for Value {
     fn to_string(&self, ir: &AirIR, elem_type: ElemType, trace_segment: u8) -> String {
         match self {
             // TODO: move constant handling to a helper function
-            Value::Constant(ConstantValue::Inline(0)) => match elem_type {
+            Value::InlineConstant(0) => match elem_type {
                 ElemType::Base => "Felt::ZERO".to_string(),
                 ElemType::Ext => "E::ZERO".to_string(),
             },
-            Value::Constant(ConstantValue::Inline(1)) => match elem_type {
+            Value::InlineConstant(1) => match elem_type {
                 ElemType::Base => "Felt::ONE".to_string(),
                 ElemType::Ext => "E::ONE".to_string(),
             },
-            Value::Constant(ConstantValue::Inline(value)) => match elem_type {
+            Value::InlineConstant(value) => match elem_type {
                 ElemType::Base => format!("Felt::new({value})"),
                 ElemType::Ext => format!("E::from({value}_u64)"),
             },
-            Value::Constant(ConstantValue::Scalar(ident)) => match elem_type {
-                ElemType::Base => ident.to_string(),
-                ElemType::Ext => format!("E::from({ident})"),
-            },
-            Value::Constant(ConstantValue::Vector(vector_access)) => match elem_type {
-                ElemType::Base => format!("{}[{}]", vector_access.name(), vector_access.idx()),
-                ElemType::Ext => {
-                    format!("E::from({}[{}])", vector_access.name(), vector_access.idx())
+            Value::BoundConstant(binding_access) => {
+                let name = binding_access.name().to_string();
+                let access_type = binding_access.access_type();
+                let base_value = match access_type {
+                    AccessType::Default => name,
+                    AccessType::Vector(idx) => format!("{name}[{idx}]"),
+                    AccessType::Matrix(row_idx, col_idx) => {
+                        format!("{name}[{row_idx}][{col_idx}]",)
+                    }
+                };
+                match elem_type {
+                    ElemType::Base => base_value,
+                    ElemType::Ext => format!("E::from({base_value})"),
                 }
-            },
-            Value::Constant(ConstantValue::Matrix(matrix_access)) => match elem_type {
-                ElemType::Base => format!(
-                    "{}[{}][{}]",
-                    matrix_access.name(),
-                    matrix_access.row_idx(),
-                    matrix_access.col_idx()
-                ),
-                ElemType::Ext => format!(
-                    "E::from({}[{}][{}])",
-                    matrix_access.name(),
-                    matrix_access.row_idx(),
-                    matrix_access.col_idx()
-                ),
-            },
+            }
             Value::TraceElement(trace_access) => {
                 trace_access.to_string(ir, elem_type, trace_segment)
             }

codegen/winterfell/src/air/mod.rs

@@ -1,8 +1,8 @@
 use super::{AirIR, Impl, Scope};
-use air_script_core::{ConstantBinding, ConstantValueExpr, TraceAccess};
+use air_script_core::{AccessType, ConstantBinding, ConstantValueExpr, TraceAccess};
 use ir::{
     constraints::{AlgebraicGraph, ConstraintDomain, Operation},
-    ConstantValue, IntegrityConstraintDegree, NodeIndex, PeriodicColumn, Value,
+    IntegrityConstraintDegree, NodeIndex, PeriodicColumn, Value,
 };
 
 mod constants;