🐛 Reject nonlinear Hadamard-lifting inputs safely - #2309
Conversation
Validate quantum SSA linearity before applying greedy lifting patterns, leaving invalid input untouched. Assisted-by: GPT-5.6 via Codex
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
burgholzer
left a comment
There was a problem hiding this comment.
I also have my doubts with this one.
Feels to me like it would be reasonable to adjust the audit prompt. It feels like it is generating a lot of changes that we actually do not want.
| if (failed(qco::verifyLinearity(op))) { | ||
| signalPassFailure(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Hm. I am not sure we should actually be adding this in front of every pass.
Passes can expect valid IR. They should not have to verify that it is valid.
I'd rather drop that in cases like here.
Does that make sense?
| TEST_F(QCOHadamardLiftingTest, HandlesUnusedPauliOutput) { | ||
| auto input = parseSourceString<ModuleOp>(R"mlir( | ||
| module { | ||
| func.func @main() { | ||
| %q = qco.static 0 : !qco.qubit | ||
| %unused = qco.x %q : !qco.qubit -> !qco.qubit | ||
| return | ||
| } | ||
| } | ||
| )mlir", |
There was a problem hiding this comment.
This is invalid QCO IR.
We should really enforce the invariant that passes may assume IR to be valid.
We may check in tests that this is actually true before and after passes, but the corner case checking in all of our passes can be reduced.
Even more so can the amount of tests that check these invalid IR programs.
This extends well beyond this PR.
|
🤖 AI text below 🤖 Agreed. These tests use invalid QCO IR, and Hadamard lifting may assume verified linearity. The pass-local validation and invalid-input regressions are outside the revised audit, so I am closing this PR unmerged. The broader audit criteria and disposition are recorded in #2287. |
Move the invalid-QCO finding from pending withdrawal to closed and refresh the focused audit counts. Assisted-by: GPT-5.6 via Codex
🤖 AI text below 🤖
Summary
Part of #2255.
Validation
AI assistance: Codex extracted this focused change from the contract audit branch, rebased it onto current main, and ran the listed validation.
Checklist
I have updated the documentation to reflect these changes.(Not applicable: no user-facing documentation change is needed.)I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals.(Not applicable: this focused fix is labeled skip-changelog.)I have added migration instructions to the upgrade guide (if needed).(Not needed.)If PR contains AI-assisted content:
🤖 *AI text below* 🤖(titles are exempt).