Modify the parser to make importing to Qiskit easier or possible (#201)

* Modify the parser to importing to Qiskit easier or possible

The parser no longer is expected to parse bogus gate definitions
from stdgates.inc. Rather, symbols are entered as if it had been
parsed. This requires modifying how the Qiskit importer works.

The main change here is implementing `SymbolTable::gates`. This
returns some information on all gates found in the symbol table.
Included is the minimal information required for the Qiskit importer
to map `SymbolId`s to Qiskit gates.

* A smaller convenience:
  Add GateDeclaration::num_params() for convenience

* Remove `struct Identifier` from asg.rs

As noted in a comment, having enum variants `Identifier(Identifier)`
where the field is a `struct Identifier` is not necessary, unless we
want to carry an extra copy of the name as a `String` for convenience.
This is a big price to pay for this convenience, which has not been used
for anything.

This commit removes `struct Identifier`. This also makes asg.rs more
consistent, since some of the variant already have the form `Identifier(SymbolIdResult)`

* Update two out-of-date doc strings

* Refactor symbol lookup for flexibility

Looking up symbols has been refactored a bit. A method was added
that either returns an existing binding or makes a new one. It always
returns a `SymbolId` rather than a `Result<...>`.

This method will be used for hardware qubits.

* Remove methods rendered unneeded after changing data type

The types of the fields of `Type::Gate` were changed to unsigned.

crates/oq3_semantics/src/asg.rs

@@ -120,7 +120,7 @@ pub enum Expr {
     UnaryExpr(Box<UnaryExpr>),
     Literal(Literal),
     Cast(Box<Cast>),
-    Identifier(Identifier),
+    Identifier(SymbolIdResult),
     HardwareQubit(HardwareQubit),
     IndexExpression(Box<IndexExpression>),
     IndexedIdentifier(IndexedIdentifier),
@@ -186,7 +186,7 @@ pub enum Stmt {
     ForStmt(ForStmt),
     GPhaseCall(GPhaseCall),
     GateCall(GateCall), // A statement because a gate call does not return anything
-    GateDeclaration(GateDeclaration),
+    GateDefinition(GateDefinition),
     InputDeclaration(InputDeclaration),
     OutputDeclaration(OutputDeclaration),
     If(If),
@@ -576,21 +576,21 @@ impl Block {
 }
 
 #[derive(Clone, Debug, PartialEq)]
-pub struct GateDeclaration {
+pub struct GateDefinition {
     name: SymbolIdResult,
     params: Option<Vec<SymbolIdResult>>,
     qubits: Vec<SymbolIdResult>,
     block: Block,
 }
 
-impl GateDeclaration {
+impl GateDefinition {
     pub fn new(
         name: SymbolIdResult,
         params: Option<Vec<SymbolIdResult>>,
         qubits: Vec<SymbolIdResult>,
         block: Block,
-    ) -> GateDeclaration {
-        GateDeclaration {
+    ) -> GateDefinition {
+        GateDefinition {
             name,
             params,
             qubits,
@@ -599,7 +599,7 @@ impl GateDeclaration {
     }
 
     pub fn to_stmt(self) -> Stmt {
-        Stmt::GateDeclaration(self)
+        Stmt::GateDefinition(self)
     }
 
     pub fn name(&self) -> &SymbolIdResult {
@@ -610,6 +610,10 @@ impl GateDeclaration {
         self.params.as_deref()
     }
 
+    pub fn num_params(&self) -> usize {
+        self.params.as_ref().map_or(0, Vec::len)
+    }
+
     pub fn qubits(&self) -> &[SymbolIdResult] {
         &self.qubits
     }
@@ -743,7 +747,7 @@ pub enum GateModifier {
 
 #[derive(Clone, Debug, PartialEq)]
 pub enum GateOperand {
-    Identifier(Identifier),
+    Identifier(SymbolIdResult),
     HardwareQubit(HardwareQubit),
     IndexedIdentifier(IndexedIdentifier),
 }
@@ -1272,49 +1276,6 @@ impl BinaryExpr {
     }
 }
 
-// FIXME: Elsewhere, we use `name` for a SymbolIdResult. The actual string can be looked up
-// from the SymbolId. But here we use `name` for the string. This is not uniform
-// and potentially confusing. I think we will have to ditch the approach below.
-// The name of the identifer is stored in both `name` and as a field in `symbol_id`.
-// But this is only true if `symbol_id` is not Err. In case of a semantic error,
-// the symbol_id may not be resolved, which we represent by a value of `Err(SymbolError)`.
-// This happens when attempting to look up the binding of an identifier that is in
-// fact not bound in any visible scope.
-// We carry the name in `name` as well in order to facilitate diagnosing the semantic
-// error. But we have not concrete plan for this. So the field `name` below may be removed.
-// And in that case, the variant `Ident(Ident)` in `enum Expr` above could be replaced with
-// `Ident(SymbolId)`.
-#[derive(Clone, Debug, PartialEq)]
-pub struct Identifier {
-    name: String,
-    symbol: SymbolIdResult,
-}
-
-impl Identifier {
-    pub fn new<T: ToString>(name: T, symbol: SymbolIdResult) -> Identifier {
-        Identifier {
-            name: name.to_string(),
-            symbol,
-        }
-    }
-
-    pub fn to_expr(self) -> Expr {
-        Expr::Identifier(self)
-    }
-
-    pub fn to_texpr(self, typ: Type) -> TExpr {
-        TExpr::new(self.to_expr(), typ)
-    }
-
-    pub fn name(&self) -> &str {
-        self.name.as_ref()
-    }
-
-    pub fn symbol(&self) -> &SymbolIdResult {
-        &self.symbol
-    }
-}
-
 #[derive(Clone, Debug, PartialEq)]
 pub struct RangeExpression {
     start: TExpr,

crates/oq3_semantics/src/symbols.rs

@@ -259,6 +259,36 @@ impl SymbolTable {
             })
             .collect::<Vec<_>>()
     }
+
+    /// Return a Vec of information about all gate declarations. Each element
+    /// is a tuple of (gate name, symbol id, num classical params, num quantum params).
+    pub fn gates(&self) -> Vec<(&str, SymbolId, usize, usize)> {
+        self.all_symbols
+            .iter()
+            .enumerate()
+            .filter_map(|(n, sym)| {
+                if let Type::Gate(num_cl, num_qu) = &sym.symbol_type() {
+                    Some((sym.name(), SymbolId(n), *num_cl, *num_qu))
+                } else {
+                    None
+                }
+            })
+            .collect()
+    }
+
+    pub fn hardware_qubits(&self) -> Vec<(&str, SymbolId)> {
+        self.all_symbols
+            .iter()
+            .enumerate()
+            .filter_map(|(n, sym)| {
+                if let Type::HardwareQubit = &sym.symbol_type() {
+                    Some((sym.name(), SymbolId(n)))
+                } else {
+                    None
+                }
+            })
+            .collect()
+    }
 }
 
 #[allow(dead_code)]
@@ -295,17 +325,7 @@ impl SymbolTable {
         self.symbol_table_stack.pop();
     }
 
-    /// If a binding for `name` exists in the current scope, return `None`.
-    /// Otherwise, create a new Symbol from `name` and `typ`, bind `name` to
-    /// this Symbol in the current scope, and return the Symbol.
-    pub fn new_binding(&mut self, name: &str, typ: &Type) -> Result<SymbolId, SymbolError> {
-        //    pub fn new_binding(&mut self, name: &str, typ: &Type, ast_node: &SyntaxNode) -> Result<SymbolId, SymbolError> {
-
-        // Can't create a binding if it already exists in the current scope.
-        if self.current_scope_contains_name(name) {
-            return Err(SymbolError::AlreadyBound);
-        }
-
+    fn new_binding_no_check(&mut self, name: &str, typ: &Type) -> SymbolId {
         // Create new symbol and symbol id.
         //        let symbol = Symbol::new(name, typ, ast_node);
         let symbol = Symbol::new(name, typ);
@@ -321,7 +341,18 @@ impl SymbolTable {
         // Map `name` to `symbol_id`.
         self.current_scope_mut()
             .insert(name, current_symbol_id.clone());
-        Ok(current_symbol_id)
+        current_symbol_id
+    }
+
+    /// If a binding for `name` exists in the current scope, return `Err(SymbolError::AlreadyBound)`.
+    /// Otherwise, create a new Symbol from `name` and `typ`, bind `name` to
+    /// this Symbol in the current scope, and return the Symbol.
+    pub fn new_binding(&mut self, name: &str, typ: &Type) -> Result<SymbolId, SymbolError> {
+        // Can't create a binding if it already exists in the current scope.
+        if self.current_scope_contains_name(name) {
+            return Err(SymbolError::AlreadyBound);
+        }
+        Ok(self.new_binding_no_check(name, typ))
     }
 
     // Symbol table for current (latest) scope in stack, mutable ref
@@ -356,7 +387,7 @@ impl SymbolTable {
 
     // FIXME: fix awkward scope numbering
     /// Look up `name` in the stack of symbol tables. Return `SymbolRecord`
-    /// if the symbol is found. Otherwise `None`.
+    /// if the symbol is found. Otherwise `Err(SymbolError::MissingBinding)`.
     pub fn lookup(&self, name: &str) -> Result<SymbolRecord, SymbolError> {
         for (scope_level_rev, table) in self.symbol_table_stack.iter().rev().enumerate() {
             if let Some(symbol_id) = table.get_symbol_id(name) {
@@ -367,6 +398,15 @@ impl SymbolTable {
         }
         Err(SymbolError::MissingBinding) // `name` not found in any scope.
     }
+
+    /// Lookup `name` if symbol exists and return the `SymbolId`, otherwise create a new binding
+    /// and return the `SymbolId`.
+    pub fn lookup_or_new_binding(&mut self, name: &str, typ: &Type) -> SymbolId {
+        match self.lookup(name) {
+            Ok(symbol_record) => symbol_record.symbol_id,
+            Err(_) => self.new_binding_no_check(name, typ),
+        }
+    }
 }
 
 impl Default for SymbolTable {

crates/oq3_semantics/src/syntax_to_semantics.rs

@@ -357,12 +357,12 @@ fn from_stmt(stmt: synast::Stmt, context: &mut Context) -> Option<asg::Stmt> {
             let gate_name_symbol_id = context.new_binding(
                 name_node.string().as_ref(),
                 &Type::Gate(
-                    num_params.try_into().unwrap(),
-                    qubits.len().try_into().unwrap(),
+                    num_params,
+                    qubits.len(),
                 ),
                 &name_node,
             );
-            Some(asg::GateDeclaration::new(gate_name_symbol_id, params, qubits, block).to_stmt())
+            Some(asg::GateDefinition::new(gate_name_symbol_id, params, qubits, block).to_stmt())
         }
 
         synast::Stmt::Def(def_stmt) => {
@@ -657,8 +657,8 @@ fn from_expr(expr_maybe: Option<synast::Expr>, context: &mut Context) -> Option<
         }
 
         synast::Expr::Identifier(identifier) => {
-            let (astidentifier, typ) = ast_identifier(&identifier, context);
-            Some(astidentifier.to_texpr(typ))
+            let (sym, typ) = ast_identifier(&identifier, context);
+            Some(asg::TExpr::new(asg::Expr::Identifier(sym), typ))
         }
 
         synast::Expr::HardwareQubit(hwq) => Some(ast_hardware_qubit(&hwq).to_texpr()),
@@ -768,7 +768,7 @@ fn from_gate_call_expr(
             Type::Gate(np, nq) => (np, nq),
             _ => (0, 0),
         };
-        if def_num_params != num_params.try_into().unwrap() {
+        if def_num_params != num_params {
             if num_params != 0 {
                 // If num params is mismatched, locate error at list of params supplied.
                 context.insert_error(NumGateParamsError, &gate_call_expr.arg_list().unwrap());
@@ -778,7 +778,7 @@ fn from_gate_call_expr(
             }
         }
         let num_qubits: usize = gate_operands.len();
-        if def_num_qubits != num_qubits.try_into().unwrap() {
+        if def_num_qubits != num_qubits {
             if num_qubits == 0 {
                 // This probably can't happen because no qubit args is not recognized syntactially
                 // as a gate call.
@@ -805,11 +805,11 @@ fn from_gate_operand(gate_operand: synast::GateOperand, context: &mut Context) -
             asg::GateOperand::HardwareQubit(ast_hardware_qubit(hwq)).to_texpr(Type::HardwareQubit)
         }
         synast::GateOperand::Identifier(ref identifier) => {
-            let (astidentifier, typ) = ast_identifier(identifier, context);
+            let (sym, typ) = ast_identifier(identifier, context);
             if !matches!(typ, Type::Qubit | Type::HardwareQubit | Type::QubitArray(_)) {
                 context.insert_error(IncompatibleTypesError, &gate_operand);
             }
-            asg::GateOperand::Identifier(astidentifier).to_texpr(typ)
+            asg::GateOperand::Identifier(sym).to_texpr(typ)
         }
         synast::GateOperand::IndexedIdentifier(ref indexed_identifier) => {
             let (indexed_identifier, typ) = ast_indexed_identifier(indexed_identifier, context);
@@ -1122,12 +1122,12 @@ fn ast_hardware_qubit(hwq: &synast::HardwareQubit) -> asg::HardwareQubit {
 fn ast_identifier(
     identifier: &synast::Identifier,
     context: &mut Context,
-) -> (asg::Identifier, Type) {
+) -> (SymbolIdResult, Type) {
     let name_str = identifier.string();
     let (symbol_id, typ) = context
         .lookup_symbol(name_str.as_str(), identifier)
         .as_tuple();
-    (asg::Identifier::new(name_str, symbol_id), typ)
+    (symbol_id, typ)
 }
 
 fn ast_indexed_identifier(

crates/oq3_semantics/src/types.rs

@@ -71,7 +71,7 @@ pub enum Type {
     DurationArray(ArrayDims),
 
     // Other
-    Gate(i32, i32), // (num classical args, num quantum args)
+    Gate(usize, usize), // (num classical args, num quantum args)
     Range,
     Set,
     Void,

crates/oq3_semantics/src/validate.rs

@@ -96,8 +96,8 @@ impl<T: SymTrait> WalkSymbols<T> for Stmt {
 
 impl<T: SymTrait> WalkSymbols<T> for Expr {
     fn walk_symbols(&self, context: &mut SymContext<T>) {
-        if let Expr::Identifier(ident) = self {
-            (context.func)(ident.symbol())
+        if let Expr::Identifier(sym) = self {
+            (context.func)(sym)
         }
     }
 }