Skip to content

Forward defs from qasm2.inline to python Lowering. #151

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

Closed
wants to merge 0 commits into from

Conversation

weinbe58
Copy link
Member

in order to close #150 I added a flag that allows you to obtrain the defs from the last frame used in qasm2 lowering. The idea is those defs need to be given to the python lowering frame to make sure those names are availible to continue lowering the rest of the python code. This is OK because all declarations in qasm2 are global.

@weinbe58 weinbe58 requested a review from Roger-luo April 15, 2025 01:26
Copy link
Contributor

github-actions bot commented Apr 15, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
6143 5266 86% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/bloqade/qasm2/dialects/inline.py 75% 🟢
src/bloqade/qasm2/parse/lowering.py 59% 🟢
TOTAL 67% 🟢

updated for commit: 2e9b46d by action🐍

Copy link

codecov bot commented Apr 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@Roger-luo
Copy link
Member

Roger-luo commented Apr 15, 2025

while this fixes #150 (but somewhat dirty) I don't think this is bug that should be fixed. I fixed the wrong behaviour on propose during the upgrade. It should have errroed but it didn't so I didn't considered it breaking. Let's move the conversation in #150 first.

@@ -27,6 +27,7 @@ def run(
lineno_offset: int = 0,
col_offset: int = 0,
compactify: bool = True,
return_defs: bool = False,
Copy link
Member

Choose a reason for hiding this comment

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

this is breaking the interface too. pyright would have errored here for you locally

@Roger-luo Roger-luo added the do not merge do not merge pending on other issues to resolve label Apr 15, 2025
@weinbe58 weinbe58 closed this Apr 15, 2025
@weinbe58 weinbe58 force-pushed the phil/fix-linline-qasm2-defs branch from 2e9b46d to 0253a76 Compare April 15, 2025 18:05
@weinbe58 weinbe58 deleted the phil/fix-linline-qasm2-defs branch April 15, 2025 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge do not merge pending on other issues to resolve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inline QASM2 is not sharing the scope with python code
2 participants