Skip to content

Lex/parse/analyze array literals correctly #279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jlapeyre merged 2 commits into from
Dec 1, 2025
Merged

Lex/parse/analyze array literals correctly #279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7eb789
Lex/parse/analyze array literals correctly
jlapeyre Nov 30, 2025
23ca5e8
Remove cruft introduced in last commit
jlapeyre Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/oq3_parser/src/grammar/expressions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub(crate) fn stmt(p: &mut Parser<'_>) {
if p.at(VERSION_STRING) {
p.bump_any();
if !p.eat(T![;]) {
p.error("Expecting semicolon terminating statement");
p.error("Expecting semicolon terminating version declaration statement");
}
m.complete(p, VERSION_STRING);
return;
Expand Down
8 changes: 7 additions & 1 deletion crates/oq3_parser/src/grammar/items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ pub(crate) fn _returns_bool_classical_declaration_stmt(p: &mut Parser<'_>, m: Ma
// type spec is still contained in the surrounding marker `m`.
let mexpr = p.start();
// parse type
let have_array_decl = p.at(T![array]);
expressions::type_spec(p);
// An opening paren means this is actually a cast
if p.current() == T!['('] {
Expand Down Expand Up @@ -323,7 +324,12 @@ pub(crate) fn _returns_bool_classical_declaration_stmt(p: &mut Parser<'_>, m: Ma
m.abandon(p);
return false;
}
expressions::expr(p);
// RHS of declaration, that is, the initializer
if have_array_decl && p.at(T!['{']) {
params::array_literal(p);
} else {
expressions::expr(p);
}
p.expect(T![;]);
m.complete(p, CLASSICAL_DECLARATION_STATEMENT);
true
Expand Down
199 changes: 44 additions & 155 deletions crates/oq3_parser/src/grammar/params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,57 +76,25 @@ pub(super) fn case_value_list(p: &mut Parser<'_>) {
_param_list_openqasm(p, DefFlavor::CaseValues);
}

pub(super) fn array_literal(p: &mut Parser<'_>) {
// - curlies: yes
// - typed: no
// - terminator: '}'
_param_list_openqasm(p, DefFlavor::ArrayLiteral);
}

// Here and elsewhere "Gate" means gate def, and "GateCall" means gate call.
#[derive(Debug, Clone, Copy)]
enum DefFlavor {
// Gate definition parameter list: (p0, p1, ...)
// - parens: yes
// - typed: no
// - terminator: ')'
GateParams,

// Gate definition qubit list: q0, q1, ... {
// - parens: no
// - typed: no
// - terminator: '{'
GateQubits,

// Gate call qubit list: q0, q1, ...;
// - parens: no
// - typed: no
// - terminator: ';'
GateCallQubits,

// Function definition parameter list: (t0 p0, t1 p1, ...)
// - parens: yes
// - typed: yes
// - terminator: ')'
DefParams,

// DefCal parameter list: (p0, t1 p1, ...)
// - parens: yes
// - typed: optional
// - terminator: ')'
DefCalParams,

// DefCal qubit list: q0, q1, ... { or q0, q1, ... -> ...
// - parens: no
// - typed: no
// - terminators: '{' or '->'
DefCalQubits,

// General expression list (e.g., in index operators): [e0, e1, ...]
// - parens: no
// - typed: no
// - terminator: ']'
ExpressionList,

// Switch case control values: case e0, e1, ... {
// - parens: no
// - typed: no
// - terminator: '{'
ArrayLiteral,
CaseValues,

TypeListFlavor,
}

Expand All @@ -140,16 +108,6 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor) {
use DefFlavor::*;
let list_marker = p.start();

// Only GateParams, DefParams, and DefCalParams open with '(' ... ')'.
let want_parens = matches!(
flavor,
GateParams | DefParams | DefCalParams | TypeListFlavor
);
match flavor {
GateParams | DefParams | DefCalParams | TypeListFlavor => p.bump(T!['(']),
GateQubits | GateCallQubits | DefCalQubits | ExpressionList | CaseValues => (),
}

// End tokens for each flavor.
let list_end_tokens = match flavor {
ExpressionList => [T![']'], T![']']],
Expand All @@ -159,16 +117,31 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor) {
GateQubits => [T!['{'], T!['{']],
GateCallQubits => [SEMICOLON, SEMICOLON],
DefCalQubits => [T!['{'], T![->]],
ArrayLiteral => [T!['}'], T!['}']],
};

let mut num_params: usize = 0;

let need_parens = matches!(
flavor,
GateParams | DefParams | DefCalParams | TypeListFlavor
);
let need_curlies = matches!(flavor, ArrayLiteral);

if need_parens {
p.expect(T!['(']);
} else if need_curlies {
p.expect(T!['{']);
}

// Parse items until EOF or an end token is seen.
while !p.at(EOF) && !list_end_tokens.iter().any(|x| p.at(*x)) {
let m = p.start();

// Allowed starts for an item: either a type or a first-token of a param/expression.
if !(p.current().is_type() || p.at_ts(PARAM_FIRST)) {
let inner_array_literal = p.at(T!['{']);
// Allowed starts for an item: either a type or a first-token of a param/expression,
// or first token of array literal.
if !(p.current().is_type() || p.at_ts(PARAM_FIRST) || inner_array_literal) {
p.error("expected value parameter");
m.abandon(p);
break;
Expand All @@ -188,6 +161,17 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor) {
// Untyped parameters/qubits.
GateParams | GateQubits => param_untyped(p, m),
DefCalQubits => param_untyped_or_hardware_qubit(p, m),
ArrayLiteral => {
if inner_array_literal {
m.abandon(p);
array_literal(p);
true
} else {
m.abandon(p);
expressions::expr(p);
true
}
}
};
if !found_param {
break;
Expand All @@ -212,17 +196,15 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor) {
}
}

match flavor {
GateParams | ExpressionList | CaseValues if num_params < 1 => {
p.error("expected one or more parameters");
}
GateParams | ExpressionList | CaseValues => {}
GateQubits | GateCallQubits | DefParams | DefCalParams | DefCalQubits | TypeListFlavor => {}
};
if num_params < 1 && matches!(flavor, GateParams | ExpressionList | CaseValues) {
p.error("expected one or more parameters");
}

// Close parens for the paren-using flavors.
if want_parens {
// Some flavors expect closing paren.
if need_parens {
p.expect(T![')']);
} else if matches!(flavor, ArrayLiteral) {
p.expect(T!['}']);
}

// Complete with the correct node kind for this flavor.
Expand All @@ -234,104 +216,11 @@ fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor) {
DefParams | DefCalParams => TYPED_PARAM_LIST,
GateParams => PARAM_LIST,
TypeListFlavor => TYPE_LIST,
ArrayLiteral => ARRAY_LITERAL,
};
list_marker.complete(p, kind);
}

// // Parse a list of parameters.
// fn _param_list_openqasm(p: &mut Parser<'_>, flavor: DefFlavor) {
// use DefFlavor::*;
// let list_marker = p.start();
// let want_parens = matches!(flavor, GateParams | DefParams | DefCalParams);
// match flavor {
// GateParams | DefParams | DefCalParams => p.bump(T!['(']),
// GateQubits | GateCallQubits | DefCalQubits | ExpressionList | CaseValues => (),
// }
// // FIXME: find implementation that does not require [T![')'], T![')']]
// // I tried using TokenSet, which should give exactly the same result.
// // But it does not. May be because -> is a compound token.
// let list_end_tokens = match flavor {
// // GateParams | DefParams | DefCalParams => {TokenSet::new(&[T![')']])},
// // GateQubits => {TokenSet::new(&[T!['{']])},
// // DefCalQubits => {TokenSet::new(&[T!['{'], T![->]])},
// ExpressionList => [T![']'], T![']']],
// CaseValues => [T!['{'], T!['{']],
// GateParams | DefParams | DefCalParams => [T![')'], T![')']],
// // When no parens are present `{` terminates the list of parameters.
// GateQubits => [T!['{'], T!['{']],
// GateCallQubits => [SEMICOLON, SEMICOLON],
// DefCalQubits => [T!['{'], T![->]],
// };
// let mut num_params: usize = 0;
// // Would be nice if we could used the following line instead of hacked roll your own two lines down.
// // while !p.at(EOF) && !p.at_ts(list_end_tokens) {
// while !p.at(EOF) && !list_end_tokens.iter().any(|x| p.at(*x)) {
// let m = p.start();
// if !(p.current().is_type() || p.at_ts(PARAM_FIRST)) {
// p.error("expected value parameter");
// m.abandon(p);
// break;
// }
// let found_param = match flavor {
// ExpressionList | CaseValues => {
// m.abandon(p);
// expressions::expr_or_range_expr(p);
// true
// }
// GateCallQubits => arg_gate_call_qubit(p, m),
// // FIXME: this can't work. These two variants have different reqs on params
// DefParams | DefCalParams => param_typed(p, m),
// // The following is pretty ugly. Probably inefficient as well
// GateParams | GateQubits | DefCalQubits => param_untyped(p, m),
// };
// if !found_param {
// break;
// }
// num_params += 1;
// // FIXME: This is only needed to support `->` as terminating tokens.
// // Not for `{`. But I don't know why. prbly because `->` is compound.
// // FIXME: use at_ts()
// if list_end_tokens.iter().any(|x| p.at(*x)) {
// // if p.at_ts(list_end_tokens) {
// break;
// }
// // Params must be separated by commas.
// if !p.at(T![,]) {
// if p.at_ts(PARAM_FIRST) {
// p.error("Expected `,`");
// } else {
// break;
// }
// } else {
// // We found the expected comma, so consume it.
// p.bump(T![,]);
// }
// }
// match flavor {
// GateParams | ExpressionList | CaseValues if num_params < 1 => {
// p.error("expected one or more parameters");
// }
// GateParams | ExpressionList | CaseValues => {}
// GateQubits | GateCallQubits | DefParams | DefCalParams | DefCalQubits => {}
// };
// // if let Some(m) = param_marker {
// // m.abandon(p);
// // }
// // Error if we don't find closing paren.
// if want_parens {
// p.expect(T![')']);
// }
// let kind = match flavor {
// GateQubits => PARAM_LIST,
// DefCalQubits => QUBIT_LIST,
// GateCallQubits => QUBIT_LIST,
// ExpressionList | CaseValues => EXPRESSION_LIST,
// DefParams | DefCalParams => TYPED_PARAM_LIST,
// GateParams => PARAM_LIST,
// };
// list_marker.complete(p, kind);
// }

const PATTERN_FIRST: TokenSet = expressions::LITERAL_FIRST
.union(expressions::atom::PATH_FIRST)
.union(TokenSet::new(&[
Expand Down
27 changes: 27 additions & 0 deletions ...sts/snapshots/runner__tests__snippets__invalid_oq3p__declaration__bit_array.qasm-lex.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
source: crates/pipeline-tests/tests/runner.rs
expression: lex_snap
---
id: tests/snippets/invalid_oq3p/declaration/bit_array.qasm
expect-lex: Ok
--- source ---

array[bit[8], 2] x;
--- lexer ---
ok: true
errors: 0
[0] Whitespace "\n" @0..1
[1] Ident "array" @1..6
[2] OpenBracket "[" @6..7
[3] Ident "bit" @7..10
[4] OpenBracket "[" @10..11
[5] Literal { kind: Int { base: Decimal, empty_int: false }, suffix_start: 1 } "8" @11..12
[6] CloseBracket "]" @12..13
[7] Comma "," @13..14
[8] Whitespace " " @14..15
[9] Literal { kind: Int { base: Decimal, empty_int: false }, suffix_start: 1 } "2" @15..16
[10] CloseBracket "]" @16..17
[11] Whitespace " " @17..18
[12] Ident "x" @18..19
[13] Semi ";" @19..20
[14] Whitespace "\n" @20..21
21 changes: 21 additions & 0 deletions ...s/snapshots/runner__tests__snippets__invalid_oq3p__declaration__bit_array.qasm-parse.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: crates/pipeline-tests/tests/runner.rs
expression: parse_snap
---
id: tests/snippets/invalid_oq3p/declaration/bit_array.qasm
expect-parse: Diag
--- parser ---
ok: false
panicked: false
errors: 1
--- ast ---
SOURCE_FILE@0..21:
array[bit[8], 2] x;

CLASSICAL_DECLARATION_STATEMENT@1..20: array[bit[8], 2] x;
ARRAY_TYPE@1..17: array[bit[8], 2]
SCALAR_TYPE@7..13: bit[8]
DESIGNATOR@10..13: [8]
LITERAL@11..12: 8
LITERAL@15..16: 2
NAME@18..19: x
Loading
Loading