Mostly implement subroutine definition (def)

This commit roughly implements parsing subroutine definition end-to-end.
There are likely loose ends and edge cases that will become apparent.

A few tests are included. One test exposes a bug. The corresponding issue
will be opened once this commit merges.

crates/oq3_parser/src/grammar.rs

@@ -94,23 +94,27 @@ impl BlockLike {
     }
 }
 
-fn scalar_type(p: &mut Parser<'_>) {
-    assert!(p.current().is_scalar_type());
-    let r = p.start();
-    p.bump_any();
-    r.complete(p, SCALAR_TYPE);
-}
+// FIXME: was only used in opt_ret_type. But no longer even there
+// fn scalar_type(p: &mut Parser<'_>) {
+//     assert!(p.current().is_scalar_type());
+//     let r = p.start();
+//     p.bump_any();
+//     r.complete(p, SCALAR_TYPE);
+// }
 
 /// Parse the optional return signature of a `defcal` or `def` definition.
 /// Return `true` if the return type was found, else `false.
 fn opt_ret_type(p: &mut Parser<'_>) -> bool {
     if p.at(T![->]) {
         let m = p.start();
         p.bump(T![->]);
+        if !p.current().is_scalar_type() {
+            p.error("Expected scalar return type after ->");
+        }
+        // Allow any type, with possible error.
         if p.current().is_type() {
-            scalar_type(p);
+            expressions::type_spec(p);
         } else {
-            p.error("Expected return type after ->");
             m.abandon(p);
             return false;
         }

crates/oq3_parser/src/grammar/expressions.rs

@@ -363,6 +363,10 @@ fn call_expr(p: &mut Parser<'_>, lhs: CompletedMarker) -> CompletedMarker {
 fn type_name(p: &mut Parser<'_>) {
     if !p.current().is_type() {
         p.error("Expected type name.");
+        // Return without parsing whatever is here.
+        // Probably an identifier because the entire type spec is missing.
+        // If we eat it now. We will get another (false) error for a missing parameter.
+        return;
     }
     p.bump(p.current());
 }
@@ -454,7 +458,12 @@ pub(crate) fn designator(p: &mut Parser<'_>) -> bool {
 
 pub(crate) fn var_name(p: &mut Parser<'_>) {
     let m = p.start();
-    p.expect(IDENT); // The declared identifier, ie variable name
+    if p.at(IDENT) {
+        // The declared identifier, ie variable name
+        p.bump_any();
+    } else {
+        p.error("Expecting parameter name.");
+    }
     m.complete(p, NAME);
 }
 

crates/oq3_parser/src/grammar/items.rs

@@ -89,7 +89,7 @@ pub(super) fn opt_item(p: &mut Parser<'_>, m: Marker) -> Result<(), Marker> {
         T![if] => if_stmt(p, m),
         T![while] => while_stmt(p, m),
         T![for] => for_stmt(p, m),
-        T![def] => def_(p, m),
+        T![def] => def_stmt(p, m),
         T![defcal] => defcal_(p, m),
         T![cal] => cal_(p, m),
         T![defcalgrammar] => defcalgrammar_(p, m),
@@ -336,8 +336,9 @@ fn io_declaration_stmt(p: &mut Parser<'_>, m: Marker) {
     m.complete(p, I_O_DECLARATION_STATEMENT);
 }
 
-fn def_(p: &mut Parser<'_>, m: Marker) {
-    p.bump(T![def]);
+fn def_stmt(p: &mut Parser<'_>, m: Marker) {
+    assert!(p.at(T![def]));
+    p.bump_any();
 
     // Read the name of the subroutine. (This records an error message on failure.)
     name_r(p, ITEM_RECOVERY_SET);

crates/oq3_parser/src/grammar/params.rs

@@ -133,6 +133,7 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor, m: Option<Marker>
         DefCalQubits => QUBIT_LIST,
         GateCallQubits => QUBIT_LIST,
         ExpressionList => EXPRESSION_LIST,
+        DefParams | DefCalParams => TYPED_PARAM_LIST,
         _ => PARAM_LIST,
     };
     list_marker.complete(p, kind);
@@ -181,21 +182,10 @@ fn param_untyped(p: &mut Parser<'_>, m: Marker) -> bool {
 }
 
 fn param_typed(p: &mut Parser<'_>, m: Marker) -> bool {
-    let mut success = true;
-    if p.current().is_type() {
-        p.bump_any();
-    } else {
-        p.error("expected type annotation");
-        success = false;
-    }
-    if !p.at(IDENT) {
-        p.error("expected parameter name");
-        m.abandon(p);
-        return false;
-    }
-    p.bump(IDENT);
-    m.complete(p, PARAM);
-    success
+    expressions::type_spec(p);
+    expressions::var_name(p);
+    m.complete(p, TYPED_PARAM);
+    true
 }
 
 // These can be cast to GateOperand

crates/oq3_parser/src/syntax_kind/syntax_kind_enum.rs

@@ -179,9 +179,11 @@ pub enum SyntaxKind {
     EXPR_STMT,
     TYPE,
     PARAM_LIST,
+    TYPED_PARAM_LIST,
     QUBIT_LIST,
     FILE_PATH,
     PARAM,
+    TYPED_PARAM,
     ARG_LIST,
     VERSION,
     VERSION_STRING,

crates/oq3_semantics/src/asg.rs

@@ -177,7 +177,7 @@ pub enum Stmt {
     DeclareClassical(Box<DeclareClassical>),
     DeclareQuantum(DeclareQuantum),
     DeclareHardwareQubit(DeclareHardwareQubit),
-    Def,    // stub
+    DefStmt(DefStmt),
     DefCal, // stub
     Delay(DelayStmt),
     End,
@@ -602,6 +602,50 @@ impl GateDeclaration {
     }
 }
 
+#[derive(Clone, Debug, PartialEq)]
+pub struct DefStmt {
+    name: SymbolIdResult,
+    params: Vec<SymbolIdResult>,
+    block: Block,
+    return_type: Option<Type>,
+}
+
+impl DefStmt {
+    pub fn new(
+        name: SymbolIdResult,
+        params: Vec<SymbolIdResult>,
+        block: Block,
+        return_type: Option<Type>,
+    ) -> DefStmt {
+        DefStmt {
+            name,
+            params,
+            block,
+            return_type,
+        }
+    }
+
+    pub fn to_stmt(self) -> Stmt {
+        Stmt::DefStmt(self)
+    }
+
+    pub fn name(&self) -> &SymbolIdResult {
+        &self.name
+    }
+
+    pub fn params(&self) -> &[SymbolIdResult] {
+        self.params.as_ref()
+    }
+
+    pub fn block(&self) -> &Block {
+        &self.block
+    }
+
+    pub fn return_type(&self) -> Option<&Type> {
+        self.return_type.as_ref()
+    }
+}
+
 #[derive(Clone, Debug, PartialEq)]
 pub struct MeasureExpression {
     operand: TExpr,

crates/oq3_semantics/src/syntax_to_semantics.rs

@@ -339,9 +339,9 @@ fn from_stmt(stmt: synast::Stmt, context: &mut Context) -> Option<asg::Stmt> {
             //    1. a sequnce of semicolon-separated stmts.
             // or 2. a single block. But the block has a scope of course.
             with_scope!(context,  ScopeType::Subroutine,
-                          let params = bind_parameter_list(gate.angle_params(), &Type::Angle(None, IsConst::True), context);
-                          let qubits = bind_parameter_list(gate.qubit_params(), &Type::Qubit, context).unwrap();
-                          let block = from_block_expr(gate.body().unwrap(), context);
+                        let params = bind_parameter_list(gate.angle_params(), &Type::Angle(None, IsConst::True), context);
+                        let qubits = bind_parameter_list(gate.qubit_params(), &Type::Qubit, context).unwrap();
+                        let block = from_block_expr(gate.body().unwrap(), context);
             );
             let num_params = match params {
                 Some(ref params) => params.len(),
@@ -355,10 +355,33 @@ fn from_stmt(stmt: synast::Stmt, context: &mut Context) -> Option<asg::Stmt> {
                 ),
                 &name_node,
             );
-
             Some(asg::GateDeclaration::new(gate_name_symbol_id, params, qubits, block).to_stmt())
         }
 
+        synast::Stmt::Def(def_stmt) => {
+            let name_node = def_stmt.name().unwrap();
+            with_scope!(context,  ScopeType::Subroutine,
+                        let params = bind_typed_parameter_list(def_stmt.typed_param_list(), context);
+                        let block = from_block_expr(def_stmt.body().unwrap(), context);
+            );
+            // FIXME: Should we let subroutines have a parameterized type?
+            // This would be very convenient for matching signatures.
+            // let num_params = match params {
+            //     Some(ref params) => params.len(),
+            //     None => 0,
+            // };
+            let ret_type = def_stmt.return_signature()
+                .and_then(|x| x.scalar_type())
+                .map(|x| from_scalar_type(&x, true, context));
+
+            let def_name_symbol_id = context.new_binding(
+                name_node.string().as_ref(),
+                &Type::Void,
+                &name_node,
+            );
+            Some(asg::DefStmt::new(def_name_symbol_id, params.unwrap(), block, ret_type).to_stmt())
+        }
+
         synast::Stmt::Barrier(barrier) => {
             let gate_operands = barrier.qubit_list().map(|operands| {
                 operands
@@ -433,7 +456,6 @@ fn from_stmt(stmt: synast::Stmt, context: &mut Context) -> Option<asg::Stmt> {
         }
 
         synast::Stmt::Cal(_)
-            | synast::Stmt::Def(_)
             | synast::Stmt::DefCal(_)
             | synast::Stmt::DefCalGrammar(_)
             // The following two should probably be removed from the syntax parser.
@@ -909,6 +931,7 @@ fn from_scalar_type(
         synast::ScalarTypeKind::None => panic!("You have found a bug in oq3_parser"),
         synast::ScalarTypeKind::Stretch => Type::Stretch(isconst.into()),
         synast::ScalarTypeKind::UInt => Type::UInt(width, isconst.into()),
+        synast::ScalarTypeKind::Qubit => Type::Qubit,
     }
 }
 
@@ -1039,6 +1062,22 @@ fn bind_parameter_list(
     })
 }
 
+fn bind_typed_parameter_list(
+    inparam_list: Option<synast::TypedParamList>,
+    context: &mut Context,
+) -> Option<Vec<SymbolIdResult>> {
+    inparam_list.map(|param_list| {
+        param_list
+            .typed_params()
+            .map(|param| {
+                let typ = from_scalar_type(&param.scalar_type().unwrap(), false, context);
+                let namestr = param.name().unwrap().string();
+                context.new_binding(namestr.as_ref(), &typ, &param)
+            })
+            .collect()
+    })
+}
+
 // This works, but using it is pretty clumsy.
 // fn with_scope<F>(context: &mut Context, scope: ScopeType, func: F) where
 //     F: FnOnce(&mut Context)

crates/oq3_semantics/src/types.rs

@@ -72,14 +72,13 @@ pub enum Type {
 
     // Other
     Gate(i32, i32), // (num classical args, num quantum args)
-    Range,          // temporary placeholder, perhaps
+    Range,
     Set,
     Void,
     ToDo, // not yet implemented
     // Undefined means a type that is erroneously non-existent. This is not the same as unknown.
     // The prototypical application is trying to resolve an unbound identifier.
     Undefined,
-    // Void, do we need this?
 }
 
 // OQ3 supports arrays with number of dims up to seven.

crates/oq3_semantics/tests/from_string_tests.rs

@@ -619,3 +619,48 @@ delay[x] q;
     assert_eq!(errors.len(), 1);
     assert_eq!(program.len(), 3);
 }
+
+#[test]
+fn test_from_string_def_1() {
+    let code = r##"
+gate h q {}
+def xmeasure(qubit q) -> bit { h q; return measure q; }
+"##;
+    let (program, errors, _symbol_table) = parse_string(code);
+    assert_eq!(errors.len(), 0);
+    assert_eq!(program.len(), 2);
+}
+
+#[test]
+fn test_from_string_def_2() {
+    let code = r##"
+gate h q {}
+gate rz(theta) q {}
+
+def pmeasure(angle[32] theta, qubit q) -> bit {
+  rz(theta) q;
+  h q;
+  return measure q;
+}
+"##;
+    let (program, errors, _symbol_table) = parse_string(code);
+    assert_eq!(errors.len(), 0);
+    assert_eq!(program.len(), 3);
+}
+
+// Issue #183
+#[test]
+fn test_from_string_def_3() {
+    let code = r##"
+gate cx p, q {}
+
+def xcheck(qubit[4] d, qubit a) -> bit {
+  reset a;
+  for int i in [0: 3] {cx d[i], a;}
+  return measure a;
+}
+"##;
+    let (program, errors, _symbol_table) = parse_string(code);
+    assert_eq!(errors.len(), 1);
+    assert_eq!(program.len(), 2);
+}

crates/oq3_syntax/examples/itemparse.rs

@@ -160,8 +160,8 @@ fn print_defcal(defcal: ast::DefCal) {
 
 fn print_def(def: ast::Def) {
     println!("Def\ndef name: '{}'", def.name().unwrap());
-    if def.param_list().is_some() {
-        println!("parameters: '{}'", def.param_list().unwrap());
+    if def.typed_param_list().is_some() {
+        println!("parameters: '{}'", def.typed_param_list().unwrap());
     }
     if def.return_signature().is_some() {
         println!("return type: '{}'", def.return_signature().unwrap());

crates/oq3_syntax/openqasm3.ungram

@@ -154,7 +154,7 @@ FilePath =
 
 // Subroutine definition
 Def =
- 'def' Name ParamList ReturnSignature?
+ 'def' Name TypedParamList ReturnSignature?
  (body:BlockExpr | ';')
 
 // Defcal definition
@@ -179,6 +179,12 @@ Gate =
 ParamList =
   '(' (Param (',' Param)* ','?)? ')'
 
+TypedParamList =
+  '(' (TypedParam (',' TypedParam)* ','?)? ')'
+
+TypedParam =
+  ScalarType Name
+
 // List with no delimeters
 QubitList =
     (GateOperand (',' GateOperand)* ','?)?