Implement gate modifiers

[jlapeyre]

This PR implements parsing gate modifiers

To do:

• inv
• ctrl
• negctrl
• pow
• multiple modifiers on one gate call

Status of this PR

Crufty. Needs a bit lot of tidying up

Design

• In oq3_parser, all gate calls are now expressions, rather than statements as they were before this PR. This reverts a previous change. This has a few advantages.

  • @jakelishman mentioned wanting this anyway
  • We can remove this got_call hack:

    openqasm3_parser/crates/oq3_parser/src/grammar/expressions.rs

    Line 230 in 9678194

    Some((lhs, blocklike, got_call)) => {

  • We represent modified gate calls as an expression wrapping a gate call expression. The entire modified gate call statement is then an expression statement. It would be difficult to do this if gate calls were already statements and no expressions.
  • The four modified gate types are not variants of any structure. Rather each is implemented independently at every level. They could be grouped together somehow, but there is little advantage to be gained. Although one could avoid a bit of code duplication

• In the ASG, one could implement gate modifiers via a Option<Modifier> field in the gate calls. But I guess that in typical use cases modifiers would be rather rare. It would be more efficient to not carry this unused field. Furthermore, we probably want separate representations for, say, parameterless one- and two-qubit gates. For these the extra field would represent a larger marginal penalty. This is another reason why I represent modifiers as wrappers around gate calls. Of course, this question of how to represent gates efficiently and at the same time minimizing the proliferation of combinations remains to be solved.

[jakelishman]

With GateCall becoming an expression, have you considered making @ a right-associative binary op (like Python's **, as opposed to / which is left-associative) and then parsing the syntax tree like that? A modified gate expression could be represented in the ungrammar's CST as BinaryOp: Expr @ Expr, where inv, pow, ctrl and negctrl all become Expr branches, and then we sort it out in semantic analysis, where @ is marked as a binary function with the signature (ModifierExpr, GateCallExpr) -> GateCallExpr?

That would let us use the general Pratt-parsing machinery of binary-expression parsing, and the types of valid expressions would flow fairly neatly through semantic analysis, I think, with fewer lexer and parser special cases needed.

[jlapeyre]

My first thought was that @ should be implemented as a binary operator. And I started coding it this way. Because logically or theoretically it is probably best viewed this way. But, I found it easy and straightforward to view them more as black-box expressions that are recognized by a keyword. For example, upon seeing the keyword token inv, you consume the inv, then must consume @, then consume a gate call expression.

We could represent them as binary op expressions at any layer as well. But for now, I am preserving the representation throughout. Some advantages are

• This is simple. It does not require thinking about or setting or tweaking binding powers
• It allows narrowing allowed syntax earlier. Syntactically incorrect modifier statements will fail to parse immediately.
• Rust analyzer takes a similar approach in the crates. For example, binary assignment operators =, += etc. will parse to binary expressions. But at the same time several assignment-like uses of = are parsed as special cases. It seems to work well for r-a

[jakelishman]

I'm fine with you wanting to proceed with whichever approach is easier for you immediately. My comments on how I'd handle the CST can wait til I'd have more time to give a complete example of how I'd present the tree.

It seems to work well for r-a

This is one part of the reasoning where I think it would be best to try and step away from. Rust is a completely different language to OQ3 syntactically and semantically, and the goals of this parser are different to the goals of Rust Analyzer. I think there's a lot to learn from them for sure, but imo the benefits are in why they do stuff, and whether that same justification applies to OQ3 too - from what I've seen, I think "it works for Rust Analyzer" is not necessarily leading to the cleanest parse structures for OQ3.

[jlapeyre]

The arguments that Rust is a different language and that compilers are different from IDEs are certainly valid when applied to concrete choices. For example, the three lower crates are designed to allow repeatedly, and efficiently, generating structures representing fragments of syntactically incorrect code, which is something we are not interested in at the moment.

But OQ3 and Rust are in some ways similar, and some design choices may be appropriate for both. Returning to the question of binary expressions. In Rust, + can be overloaded and the operands are expressions. It seems that the best or perhaps only way to handle this is via machinery for generic binary expressions. On the other hand, the = in let statements are handled specially.

In our case, modified gate calls only exist as single expressions (if they are expressions at all) in a statement. And they have restricted syntax. We are far from being forced to look at them as binary expressions.

But after going back and forth with some of the design in this project, I'm becoming more interested in finding practical implementations. Eg, at the moment, I'm not too concerned with whether, on the most abstract level, a gate call is an expression or not. (Of course I'm aware that often concise theoretical descriptions drive practical implementations.)

[jakelishman]

I'm fine postponing further discussion on those kind of concerns til at a minimum after the release of Qiskit 1.0 proper and to just keep doing what you're doing - I wanted to raise the question of whether to use a binop, but that's all. I agree that the form you've got here is also a fine way of describing the tree.

After the release, I can maybe sketch out a little bit of an architecture/responsibility diagram that's my working model that I feel the internals and the interface could benefit from with our goals of improved error messages and ease of use as a library, including how I see the CST I'm gesturing towards supporting those goals (esp the error messages). That might help a bit in talking about it - I'm not purely going on a philosophical kick about it, I think a slightly tweaked responsibility split between tokenisation / CST / parsing / AST / semantic analysis could help us drive even better error messages and make it easier to extend.

[jlapeyre]

not purely going on a philosophical kick about it

I was referring more to revising my own thinking about trying to put elements in the correct category. For example, originally I made gate calls expressions. Then I thought, no they are really statements, so I made them statements. But that was awkward and complicated. Finally, adding modifiers made me want to revert.

I think it's useful to log discussions here about issues such as how to parse gate modifiers. I don't think we have to postpone the discussion. But now that you mention it, limiting it a bit until the 1.0 release is a good idea.

tweaked responsibility split between tokenisation / CST / parsing / AST / semantic analysis could help us

I'm looking forward to hearing about this.

[jakelishman]

I think exposing the changed parse structure into the semantic structure is a mistake - I think we ought to revert the changes in the exposed ASG. I didn't realise when I wrote the comments above that this PR changed the ASG.

It's fine the parse the syntax however we like, but the exposed semantic graph absolutely should have gate statements resolved into separate objects - they're going to be by far the most common statement in an OQ3 file, and gate calls aren't semantically expressions in OQ3 (but they can be considered so syntactically). Exposing them as an expression is harder for consumers, and allows the typing of the ASG to represent far more invalid programs (even if in theory such values are ever generated), which means consumer code will have more and more assumptions it has to make about things that can't happen, or more and more boilerplate to handle things that ought to be impossible.