RCA: `set () = { … }` drops its value's dynamic kind; code is lost
Maintainers usually reply within 1 day
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 20/100
Research direction
Start with qsc_rca/src/core.rs, especially analyze_expr_assign around lines 149–171, and trace how assignment kinds are computed from the assignee and value. The issue reports that an open pull request (#4004) already addresses this bug. The described fix should restore dynamic features for assignments to (); the two repros should then produce 5 and true on Adaptive_RI, Adaptive_RIF, and Adaptive.
Written by the indexing model from the issue text.
Description
Summary
RCA computes the kind of an assignment only from the locals it updates. set () = … updates no local, so the statement is Static even when its value contains measurement-dependent code. Partial evaluation then evaluates the block classically: a store into a mutable and gates inside the block are silently dropped. This gives wrong results on Adaptive_RI, Adaptive_RIF and Adaptive.
Minimal repro
operation Main() : Int {
use q = Qubit();
X(q);
let d = MResetZ(q) == One;
mutable z = 3;
set () = { if d { set z = 5; } () }; // assigns nothing; means: if d { set z = 5; }
return z;
}
Gate variant (the X(t) is dropped):
operation Main() : Bool {
use (q, t) = (Qubit(), Qubit());
X(q);
let d = MResetZ(q) == One;
set () = { if d { X(t); } () };
return MResetZ(t) == One;
}
Run with python check.py prog.qs "Main()"; check.py is in #4001. It runs the program on the simulator, then compiles it for Adaptive_RI, Adaptive_RIF and Adaptive and runs the QIR with qir-runner, one shot each.
Expected vs actual
| Program | Simulator (expected) | Adaptive_RI / Adaptive_RIF / Adaptive |
|---|---|---|
| store | 5 |
3 |
| gate variant | true |
false |
Output on main 1b8de0d17 (same on the earlier main commits a90b8f777 and 48449939c and on open PR #3902 577dc9382; qir-runner 0.9.7):
### store
simulator: 5
Adaptive_RI: OUTPUT INT 3 0_i
Adaptive_RIF: OUTPUT INT 3 0_i
Adaptive: OUTPUT INT 3 0_i
### gate variant
simulator: True
Adaptive_RI: OUTPUT BOOL false 0_b
Adaptive_RIF: OUTPUT BOOL false 0_b
Adaptive: OUTPUT BOOL false 0_b
The results are the same with #3977 (head 6228ff9ee): that PR does not change this path.
Root cause
analyze_expr_assign builds the statement's kind only from update_locals_compute_kind, the kind of the locals in the assignee (qsc_rca/src/core.rs#L149-L171, lines 160–169). With the assignee (), no local is updated, so the value's runtime features are discarded and the statement is Static.
History (Adaptive_RI): at #3733 (95c039f4b) and #3885 (45d8e3038) both programs were rejected with PartialEval.EvaluationFailed; since #3887 (96388a0da) they compile and give the wrong answer.
Suggested fix
In analyze_expr_assign, also aggregate the value expression's runtime features into the assignment's kind. For an assignee without () parts this adds nothing new, since the value's kind already reaches the statement through the updated locals (reasoned; consistent with no existing expect-test changing); for () (or a tuple with a () element) it restores the value's dynamic features.
I have this implemented (9 lines in core.rs, plus tests) and will open a PR. With it, the repros give 5 and true on Adaptive_RI, Adaptive_RIF and Adaptive. It is independent of #3977 and merges with it without conflicts.
How it was found
While proving properties of a formal model of the lowering: the proof required the kind of an assignment to include the kind of its value. Random differential testing did not find it (our program generator has no set ()).
Related issues
- No existing issue found. Searches: "set () unit", "unit tuple assignment", "assign to unit", "set () unit tuple", "assign unit tuple dynamic".
- #3887: the partial-evaluation change after which these programs compile to wrong QIR instead of being rejected.
- #3976 / #3977: a different RCA bug in the right operand of a dynamic
and/or; #3977 does not fix this one.
- Dominant language
- Rust
- Stars
- 1k
- Forks
- 219
- Avg merge
- 3d 19h
- Merged PRs (30d)
- 93
Getting set up
Starts the project's dev container in your browser, under your own GitHub account.
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from microsoft/qdk
-
bug needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 1 day
-
[Samples] Investigate redundant CheckMay be free again @theARCania claimed this 41 days ago, and no pull request is open. Openenhancement good first issue low-priority
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
microsoft/qdk#1933 · 4 comments ·
Maintainers usually reply within 1 day
-
Fuzz: "" (ubuntu-latest)Possibly taken @swernli claimed this today. Openbug
microsoft/qdk#4019 · 1 assignee ·
Maintainers usually reply within 1 day
-
bug needs triage
Difficulty 4/5 3-5 days Newbie friendliness 35/100
Maintainers usually reply within 1 day
-
FIR `tuple_decompose`: `{ set p = new P { … } }` (an assignment without a semicolon) panics with `LocalVarId consistency: … not bound`Possibly taken @domorale claimed this 1 day ago. Openbug needs triage
Difficulty 4/5 3-5 days Newbie friendliness 35/100
Maintainers usually reply within 1 day
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
foxglove/mcap#1907 · 1 comment ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 66/100
Maintainers usually reply within 1 day
-
good first issue help wanted r-feature
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
servo/rust-smallvec#792 ·
Maintainers usually reply within 1 day
-
A-Unicode C-bug P-low
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
rust-lang/rustfmt#7210 · 1 comment ·
Maintainers usually reply within 1 day
-
callTracer with withLog: true includes logs from a reverted top-level call (diverges from geth)Open
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
paradigmxyz/revm-inspectors#570 ·
Maintainers usually reply within 1 day