Skip to content

Add parser support for "extern port" and "extern frame" statements #37

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.

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

Conversation

hodgestar
Copy link
Contributor

@hodgestar hodgestar commented Jan 10, 2025

OpenPulse specifies that ports and frame may declared via "extern port" and "extern frame" statements (see https://openqasm.com/language/openpulse.html#ports and https://openqasm.com/language/openpulse.html#frames).

However OpenQASM only supports extern functions (not identifiers) so both of these statements are currently unsupported and raise parsing errors.

The OpenPulse examples also use "extern port". See https://openqasm.com/language/pulses.html#inline-calibration-blocks and https://openqasm.com/language/pulses.html#inline-calibration-blocks for relevant examples.

This merge request adds support for "extern port" and "extern frame" statements. Its intended to be a minimal parse update to match the current OpenPulse specification.

This issue was reported in openqasm/openqasm#577 and I've also encountered it myself in Zurich Instruments' OpenPulse compiler.

@hodgestar hodgestar marked this pull request as draft January 10, 2025 14:01
@hodgestar hodgestar marked this pull request as ready for review January 10, 2025 14:28
@hodgestar
Copy link
Contributor Author

I'll make a separate PR to update the tests to not require Python 3.7.

@hodgestar
Copy link
Contributor Author

Separate PR to fix the CI failures is #38.

@hodgestar
Copy link
Contributor Author

Ready for final review / merging.

@@ -59,3 +61,7 @@ scalarType:
| PORT
| FRAME
;


externFrameStatement: EXTERN FRAME Identifier SEMICOLON;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's overkill for this MR, but if we ever move towards allowing external linkage with other types it might make sense to have one statement type for this

externDataStatement: EXTERN (scalarType | arrayType) Identifier SEMICOLON;

Then the AST would be something like


@dataclass
class ExternDataStatement(QASMNode):
    """
    Node representing an extern data declaration.
    """

    name: Identifier
    type_: ClassicalType

@braised-babbage
Copy link
Collaborator

braised-babbage commented Apr 2, 2025

I'm OK with this as is -- it solves the problem of bringing the reference parser in line with the spec. Re: more general usage of extern, I am curious what the TSC thinks about the plausibility of this. As a reference point, C supports extern linkage for both data and functions, though it's less clear to me whether the data part is a good idea. Not suggesting that we should be taking the cue from C in the OpenPulse context.

@levbishop levbishop merged commit 85eaade into openqasm:main Apr 2, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants