Qiskit · jlapeyre · Feb 21, 2024 · Feb 20, 2024
diff --git a/crates/oq3_parser/src/grammar/expressions.rs b/crates/oq3_parser/src/grammar/expressions.rs
@@ -5,6 +5,7 @@ pub mod atom;
 
 use super::*;
 
+pub(crate) use atom::block_expr;
 pub(crate) use atom::try_block_expr;
 pub(super) use atom::LITERAL_FIRST;
 // Pretty sure semicolon is always required in OQ3
@@ -22,6 +23,27 @@ pub(super) fn expr(p: &mut Parser<'_>) -> Option<CompletedMarker> {
     expr_bp(p, None, r, 1).map(|(m, _)| m)
 }
 
+// This inlcudes square brackets
+pub(crate) fn range_expr(p: &mut Parser<'_>) -> Option<CompletedMarker> {
+    let r = Restrictions { prefer_stmt: false };
+    let m = p.start();
+    assert!(p.at(T!['[']));
+    p.bump(T!['[']);
+    expr_bp(p, None, r, 1).map(|(m, _)| m);
+    if p.at(COLON) {
+        p.bump(COLON);
+        expr_bp(p, None, r, 1).map(|(m, _)| m);
+        if p.at(COLON) {
+            p.bump(COLON);
+            expr_bp(p, None, r, 1).map(|(m, _)| m);
+        }
+    } else {
+        p.error("Expecting colon in range expression.");
+    }
+    p.expect(T![']']);
+    Some(m.complete(p, RANGE_EXPR))
+}
+
 pub(super) fn expr_or_range_expr(p: &mut Parser<'_>) -> Option<CompletedMarker> {
     let r = Restrictions { prefer_stmt: false };
     let m = p.start();
@@ -460,16 +482,21 @@ pub(crate) fn indexed_identifer(p: &mut Parser<'_>, lhs: CompletedMarker) -> Com
     m.complete(p, INDEXED_IDENTIFIER)
 }
 
+pub(crate) fn set_expression(p: &mut Parser<'_>) {
+    assert!(p.at(T!['{']));
+    let m = p.start();
+    p.bump(T!['{']);
+    params::expression_list(p);
+    p.bump(T!['}']);
+    m.complete(p, SET_EXPRESSION);
+}
+
 pub(crate) fn index_operator(p: &mut Parser<'_>) {
     assert!(p.at(T!['[']));
     let m = p.start();
     p.expect(T!['[']);
     if p.at(T!['{']) {
-        let m1 = p.start();
-        p.bump(T!['{']);
-        params::expression_list(p);
-        p.bump(T!['}']);
-        m1.complete(p, SET_EXPRESSION);
+        set_expression(p);
     } else {
         params::expression_list(p);
     }

diff --git a/crates/oq3_parser/src/grammar/expressions/atom.rs b/crates/oq3_parser/src/grammar/expressions/atom.rs
@@ -84,7 +84,6 @@ pub(super) fn atom_expr(
         T![measure] => measure_expression(p),
         T![return] => return_expr(p),
         T!['{'] => block_expr(p),
-        T![for] => for_expr(p, None),
         T![inv] | T![pow] | T![ctrl] | T![negctrl] => modified_gate_call_expr(p),
         T![gphase] => gphase_call_expr(p),
         IDENT if (la == IDENT || la == HARDWAREIDENT) => gate_call_expr(p),
@@ -94,7 +93,7 @@ pub(super) fn atom_expr(
         // Also `NAME` is probably not correct.
         IDENT => identifier(p),
         _ => {
-            p.err_and_bump("expected expression");
+            p.err_and_bump("atom_expr: expected expression");
             return None;
         }
     };
@@ -235,7 +234,7 @@ fn tuple_expr(p: &mut Parser<'_>) -> CompletedMarker {
     let mut saw_expr = false;
 
     if p.eat(T![,]) {
-        p.error("expected expression");
+        p.error("expected expression, found comma instead");
         saw_comma = true;
     }
 
@@ -292,40 +291,6 @@ fn array_expr(p: &mut Parser<'_>) -> CompletedMarker {
     m.complete(p, ARRAY_EXPR)
 }
 
-// FIXME: finish this. need to disambiguate from block of statments
-// fn set_expr(p: &mut Parser<'_>) -> CompletedMarker {
-//     assert!(p.at(T!['{']));
-//     if p.at(INT_NUMBER) {
-//         p.bump(INT_NUMBER)
-//     } else if p.at(T!['}']) {
-//         p.bump(T!['}']);
-//         m.complete(p, SET_EXPR);
-//         return ();
-//     }
-//     // FIXME find ,
-//     p.error("expecting an integer or '}'");
-//     // todo eat until ';' maybe,  for recovery
-//     m.abandon(p);
-//     return ();
-//     p.expect(T!['}']);
-//     m.complete(p, SET_EXPR)
-// }
-
-// test for_expr
-// fn foo() {
-//     for x in [] {};
-// }
-fn for_expr(p: &mut Parser<'_>, m: Option<Marker>) -> CompletedMarker {
-    assert!(p.at(T![for]));
-    let m = m.unwrap_or_else(|| p.start());
-    p.bump(T![for]);
-    p.expect(IDENT);
-    p.expect(T![in]);
-    expr_no_struct(p);
-    block_expr(p);
-    m.complete(p, FOR_STMT)
-}
-
 pub(crate) fn try_block_expr(p: &mut Parser<'_>) {
     if !p.at(T!['{']) {
         p.error("expected a block");

diff --git a/crates/oq3_parser/src/grammar/items.rs b/crates/oq3_parser/src/grammar/items.rs
@@ -85,6 +85,7 @@ pub(super) fn opt_item(p: &mut Parser<'_>, m: Marker) -> Result<(), Marker> {
         T![end] => end_(p, m),
         T![if] => if_stmt(p, m),
         T![while] => while_stmt(p, m),
+        T![for] => for_stmt(p, m),
         T![def] => def_(p, m),
         T![defcal] => defcal_(p, m),
         T![cal] => cal_(p, m),
@@ -152,6 +153,33 @@ fn while_stmt(p: &mut Parser<'_>, m: Marker) {
     m.complete(p, WHILE_STMT);
 }
 
+fn for_stmt(p: &mut Parser<'_>, m: Marker) {
+    assert!(p.at(T![for]));
+    p.bump(T![for]);
+    // Type specifier of the loop variable.
+    expressions::type_spec(p);
+    // The loop variable.
+    name(p);
+    p.expect(T![in]);
+    // The "iterator"
+    let m1 = p.start();
+    if p.at(T!['{']) {
+        expressions::set_expression(p);
+    } else if p.at(T!['[']) {
+        expressions::range_expr(p);
+    } else {
+        expressions::expr(p);
+    }
+    m1.complete(p, FOR_ITERABLE);
+    // The body of the for loop
+    if p.at(T!['{']) {
+        expressions::block_expr(p);
+    } else {
+        expressions::stmt(p, expressions::Semicolon::Required);
+    }
+    m.complete(p, FOR_STMT);
+}
+
 // Called from atom::atom_expr
 // FIXME: return CompletedMarker because this is called from `atom_expr`
 // Where functions are called and what they return should be made more uniform.

diff --git a/crates/oq3_parser/src/grammar/params.rs b/crates/oq3_parser/src/grammar/params.rs
@@ -48,10 +48,6 @@ pub(super) fn param_list_defcal_qubits(p: &mut Parser<'_>) {
     _param_list_openqasm(p, DefFlavor::DefCalQubits, None);
 }
 
-// pub(super) fn set_expression(p: &mut Parser<'_>) {
-//     _param_list_openqasm(p, DefFlavor::SetExpression, None);
-// }
-
 pub(super) fn expression_list(p: &mut Parser<'_>) {
     _param_list_openqasm(p, DefFlavor::ExpressionList, None);
 }
@@ -69,7 +65,6 @@ enum DefFlavor {
     DefParams,      // parens,    type
     DefCalParams,   // parens,    opt type
     DefCalQubits,   // no parens, no type, '{' or '->' terminates
-    //    SetExpression,
     ExpressionList,
 }
 
@@ -81,7 +76,6 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor, m: Option<Marker>
     let want_parens = matches!(flavor, GateParams | DefParams | DefCalParams);
     match flavor {
         GateParams | DefParams | DefCalParams => p.bump(T!['(']),
-        //        SetExpression => p.bump(T!['{']),
         _ => (),
     }
     // FIXME: find implementation that does not require [T![')'], T![')']]
@@ -97,7 +91,6 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor, m: Option<Marker>
         GateQubits => [T!['{'], T!['{']],
         GateCallQubits => [SEMICOLON, SEMICOLON],
         DefCalQubits => [T!['{'], T![->]],
-        //        SetExpression => [T!['}'], T!['}']],
     };
     let mut param_marker = m;
     // let mut param_marker = None;
@@ -119,6 +112,7 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor, m: Option<Marker>
             }
             GateCallQubits => arg_gate_call_qubit(p, m),
             DefParams | DefCalParams => param_typed(p, m),
+            // The following is pretty ugly. Probably inefficient as well
             _ => param_untyped(p, m),
         };
         if !found_param {
@@ -158,11 +152,7 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor, m: Option<Marker>
     if want_parens {
         p.expect(T![')']);
     }
-    // if matches!(flavor, SetExpression) {
-    //     p.expect(T!['}']);
-    // }
     let kind = match flavor {
-        //        SetExpression => SET_EXPRESSION,
         GateQubits => PARAM_LIST,
         DefCalQubits => QUBIT_LIST,
         GateCallQubits => QUBIT_LIST,
@@ -241,7 +231,7 @@ pub(crate) fn arg_gate_call_qubit(p: &mut Parser<'_>, m: Marker) -> bool {
     }
 
     if !p.at(IDENT) {
-        p.error("Expected parameter name");
+        p.error("Expected name in qubit argument");
         m.abandon(p);
         return false;
     }

diff --git a/crates/oq3_parser/src/syntax_kind/syntax_kind_enum.rs b/crates/oq3_parser/src/syntax_kind/syntax_kind_enum.rs
@@ -151,6 +151,7 @@ pub enum SyntaxKind {
     IF_STMT,
     WHILE_STMT,
     FOR_STMT,
+    FOR_ITERABLE,
     END_STMT,
     CONTINUE_STMT,
     BREAK_STMT,

diff --git a/crates/oq3_semantics/src/asg.rs b/crates/oq3_semantics/src/asg.rs
@@ -126,10 +126,6 @@ pub enum Expr {
     IndexExpression(IndexExpression),
     IndexedIdentifier(IndexedIdentifier),
     GateOperand(GateOperand),
-    // FIXME: for Range and similiar, it is easiest to shove everything in expression tree and give it a useless type.
-    // But we need to handle these in a consistent way. Is there any situation where the "type" of Range is meaningful or useful?
-    // For example, in syntax_to_semantics, have a routine that handles out-of-tree expressions.
-    Range(Range),
     Return(Box<ReturnExpression>),
     Call, // stub function (def) call
     Set,  // stub
@@ -185,8 +181,8 @@ pub enum Stmt {
     Delay,  // stub
     End,
     ExprStmt(TExpr),
-    Extern, // stub
-    For,    // stub
+    Extern,           // stub
+    ForStmt(ForStmt), // stub
     GPhaseCall(GPhaseCall),
     GateCall(GateCall), // A statement because a gate call does not return anything
     GateDeclaration(GateDeclaration),
@@ -400,15 +396,15 @@ pub enum LValue {
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum ArraySliceIndex {
     Expr(TExpr),
-    Range(TExpr),
+    RangeExpression(RangeExpression),
 }
 
 // FIXME: Is this the slice, or just the index ???
 // Same form as ArraySliceIndex, but they have different semantics.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum RegisterSlice {
     Expr(TExpr),
-    Range(Range),
+    RangeExpression(RangeExpression),
 }
 
 // example: v[3:4]. Includes multidimensional index
@@ -1087,16 +1083,21 @@ impl Identifier {
     }
 }
 
+// RangeExpression has been moved in and out of the expression tree. It is now out.
+//   Advantages: 1) I think it's maybe just as easy or easier to move RangeExpression out of the tree. 2) Increases correctness.
+//   Disadvantage: No type. Having a type associated with RangeExpression will probably be useful even if it doesn't mesh perfectly
+//      with the larger type system. We can probably do type checking compating to the declared iterator.
+// So implement this out of the tree and implement type information later if needed.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
-pub struct Range {
+pub struct RangeExpression {
     start: Box<TExpr>,
     step: Box<Option<TExpr>>,
     stop: Box<TExpr>,
 }
 
-impl Range {
-    pub fn new(start: TExpr, step: Option<TExpr>, stop: TExpr) -> Range {
-        Range {
+impl RangeExpression {
+    pub fn new(start: TExpr, step: Option<TExpr>, stop: TExpr) -> RangeExpression {
+        RangeExpression {
             start: Box::new(start),
             step: Box::new(step),
             stop: Box::new(stop),
@@ -1114,10 +1115,6 @@ impl Range {
     pub fn stop(&self) -> &TExpr {
         &self.stop
     }
-
-    pub fn to_texpr(self, typ: Type) -> TExpr {
-        TExpr::new(Expr::Range(self), typ)
-    }
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
@@ -1180,6 +1177,46 @@ impl While {
     }
 }
 
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub enum ForIterable {
+    SetExpression(SetExpression),
+    RangeExpression(RangeExpression),
+    Expr(TExpr),
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub struct ForStmt {
+    loop_var: SymbolIdResult,
+    iterable: ForIterable,
+    loop_body: Block,
+}
+
+impl ForStmt {
+    pub fn new(loop_var: SymbolIdResult, iterable: ForIterable, loop_body: Block) -> ForStmt {
+        ForStmt {
+            loop_var,
+            iterable,
+            loop_body,
+        }
+    }
+
+    pub fn loop_var(&self) -> &SymbolIdResult {
+        &self.loop_var
+    }
+
+    pub fn iterable(&self) -> &ForIterable {
+        &self.iterable
+    }
+
+    pub fn loop_body(&self) -> &Block {
+        &self.loop_body
+    }
+
+    pub fn to_stmt(self) -> Stmt {
+        Stmt::ForStmt(self)
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct SwitchCaseStmt {
     control: TExpr,