Skip to content

✨ Support if (c) and if (c[k]) as boolean-only conditions - #463

Open
rturrado wants to merge 7 commits into
munich-quantum-toolkit:mainfrom
rturrado:462
Open

✨ Support if (c) and if (c[k]) as boolean-only conditions#463
rturrado wants to merge 7 commits into
munich-quantum-toolkit:mainfrom
rturrado:462

Conversation

@rturrado

@rturrado rturrado commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Description

🤖 AI text below 🤖

Let the debugger evaluate classic-controlled if conditions written without an explicit comparator,
treating the operand as an implicit "non-zero" check.

OpenQASM 3 allows a classical register or a single bit as a boolean condition with no explicit comparator:

if (c) x q[0];
if (c[0]) x q[0];

Before this PR the debugger's parser rejected these forms (returned nullopt), so the classic-controlled gate failed to parse. mqt-core's IfElseOperation already models the shape, so only the debugger front-end needed to catch up.

Implementation

parseClassicConditionExpression scans the condition text for one of the six comparators (==, !=, <, <=, >, >=). If none is found, the whole text is now treated as a bare register or bit reference (c or c[k]) and interpreted as an implicit != 0 check. The bare form populates ClassicCondition with .kind = qc::Neq and .expectedValue = 0, so the existing evaluator handles both forms with the same code path.

The structural parsing of name or name[index] lives in a shared parseRegisterRef helper used both by the classic-condition path and by validateTargets (for qubit targets in gate calls). Numeric parsing goes through a parseUnsignedInt helper backed by std::from_chars, so no exceptions are thrown and no isDigits pre-check is needed.

End-to-end tests in test_custom_code.cpp cover the satisfied case, the unsatisfied case, the single-bit form, and one backward-step case, mirroring the coverage added in PR #456.

Closes #470.

AI assistance

Commit messages, code changes, and this PR body were drafted with Claude Opus 4.7 via Claude Code.
All content was reviewed and edited manually before submission.

Fixes #462.

Checklist

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

If PR contains AI-assisted content:

  • Any agent that created, edited, or submitted GitHub content was explicitly authorized for that scope, as required by our AI Usage Guidelines.
  • Every agent-authored or agent-edited public text body begins with the visible disclosure 🤖 *AI text below* 🤖 (titles are exempt).
  • I have disclosed AI assistance in the PR description.
  • I confirm that I have personally reviewed and understood all AI-generated content and accept full responsibility for it.

`parseClassicConditionExpression` had inline logic for parsing a bare register (`c`) or a single-bit reference (`c[k]`)
after the comparator operand.
Move that logic into a `parseBitRegisterRef` helper in the anonymous namespace and use it at the existing call site.
The helper returns a `BitRegisterRef` where a null `bitIndex` means "the whole register".

Behavior is unchanged; the helper lets the follow-up commit for munich-quantum-toolkit#462 reuse the same shape when the condition has no comparator.

Assisted-by: Claude Opus 4.7 via Claude Code
OpenQASM 3 allows a classical register or a single bit as a boolean condition without an explicit comparator:
`if (c) x q[0];` and `if (c[0]) x q[0];`.
The parser used to reject these forms.

When no comparator is found,
use `parseBitRegisterRef` to parse the operand and populate `ClassicCondition` with `.kind = qc::Neq` and `.expectedValue = 0`,
so the existing evaluator treats the condition as "the value is non-zero".

Add end-to-end tests covering the satisfied and unsatisfied cases for a bare register, a bare bit, and a backward step.

Closes munich-quantum-toolkit#462.

Assisted-by: Claude Opus 4.7 via Claude Code
@rturrado

rturrado commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI text below 🤖

A few follow-up ideas that surfaced during the review of this PR. Sharing them in case any of them is worth its own issue.

  1. Validate that the register name in a classic condition is a well-formed identifier.
    Today parseClassicConditionExpression accepts anything that does not contain [ as a register name, so inputs like c2] (dangling ]) or @# pass through as if they were valid references.
    The condition then fails later in the DD backend when the "register" is not found in variables, but the parser itself never rejects the shape.

  2. Validate the classic-condition register reference against declared registers.
    Neither the register name nor the bit index is checked against definedRegisters (the std::map<std::string, size_t> of declared classical registers). validateTargets already does this for qubit targets in preprocessCode, but nothing similar runs for the condition of an if.
    The fix would need to reach definedRegisters from wherever the check happens; possible approaches include passing it into parseClassicConditionExpression or adding a separate validateClassicCondition pass in preprocessCode.

  3. Add unit-level coverage for parseClassicConditionExpression.
    All existing tests exercise this function end-to-end via runSimulation in test_custom_code.cpp.
    Direct unit tests (probably in test_parsing.cpp) would cover the valid cases (one per comparator, bracket forms, whitespace, leading (), the invalid cases (empty, no comparator, non-numeric rhs, malformed bracket, etc.), and document the currently permissive behavior flagged in items 1 and 2 with tests that flip to expect nullopt once those issues are fixed.

Happy to open any of these as separate issues.

@rturrado

rturrado commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: e6634561-6ec9-4fc8-87c6-bc35bda79166

📥 Commits

Reviewing files that changed from the base of the PR and between 061ec7d and 60a044c.

📒 Files selected for processing (2)
  • src/common/parsing/CodePreprocessing.cpp
  • test/test_custom_code.cpp

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Summary

Summary by CodeRabbit

  • New Features

    • Classical conditions can now use a bare register or individual bit without an explicit comparison operator.
    • These conditions correctly control operations based on whether the referenced value is nonzero.
  • Bug Fixes

    • Improved handling of classical conditions during forward and backward execution.
    • Invalid register and bit-reference formats are handled safely.

Walkthrough

The parser now accepts bare classical registers and indexed classical bits as implicit non-zero conditions. Tests cover triggered, skipped, indexed-bit, and backward-step execution.

Changes

Bare classical conditions

Layer / File(s) Summary
Condition reference parsing and construction
src/common/parsing/CodePreprocessing.cpp
Adds shared parsing for bare and indexed classical references. Bare references become ClassicCondition values with an implicit != 0 comparison.
Execution and backward-step coverage
test/test_custom_code.cpp
Adds tests for satisfied and unsatisfied register conditions, indexed-bit conditions, and backward execution.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 60a04

Bare classical register and bit conditions now execute as non-zero checks, with covered satisfied, unsatisfied, indexed-bit, and backward-step behavior. No current merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant CustomCodeTest
  participant parseClassicConditionExpression
  participant parseBitRegisterRef
  CustomCodeTest->>parseClassicConditionExpression: parse if(c) or if(c[0])
  parseClassicConditionExpression->>parseBitRegisterRef: parse classical reference
  parseBitRegisterRef-->>parseClassicConditionExpression: return parsed reference
  parseClassicConditionExpression-->>CustomCodeTest: create implicit != 0 condition
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The implementation satisfies issue #462 by supporting bare classical registers and bits as implicit non-zero conditions. Tests cover satisfied and unsatisfied registers, a single bit, and backward ste…
Out of Scope Changes check ✅ Passed The parser changes and end-to-end tests are directly related to issue #462. No unrelated code or feature changes are present.
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.
Title check ✅ Passed The title clearly and concisely describes the main change: support for comparator-free if (c) and if (c[k]) conditions.
Description check ✅ Passed The description explains the change, motivation, implementation, test coverage, AI assistance, linked issue, and checklist status. Documentation and CI checks are marked incomplete, but these are non-…

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the register bright
Bare bits now guide the gate just right
Zero stays still, one hops through
Backward steps undo the chew
Tests thump paws: the parser grew!

Comment @coderabbitai help to get the list of available commands.

@rturrado rturrado changed the title ✨ Support if (c) and if (c[k]) as boolean-only conditions ✨ Support if (c) and if (c[k]) as boolean-only conditions Sep 7, 2026
rturrado and others added 3 commits September 7, 2026 18:14
`parseClassicConditionExpression` was calling `parseBitRegisterRef` twice
(once for the operand of the comparator form and once for the whole condition in the bare form)
and building the `ClassicCondition` in two places, with only the expected value and comparison kind differing between them.

Compute the operand text, the expected value, and the comparison kind first (with defaults suitable for the bare case),
then parse the register/bit reference and build the `ClassicCondition` once at the end.

Behavior is unchanged.

Assisted-by: Claude Opus 4.7 via Claude Code
Replace the two `std::stoull` + `try/catch` blocks (in `parseClassicConditionExpression` and `parseBitRegisterRef`)
with a `parseUnsignedInt(std::string_view)` helper backed by `std::from_chars`.

`std::from_chars` on integer types is non-throwing (returns `std::errc` in a struct), faster (no locale, no exception machinery),
and strict about the input (rejects empty text, leading signs, trailing garbage).
The previous `isDigits()` guards at these two sites are no longer needed and go away;
`isDigits()` itself stays because `validateTargets` still uses it.

`from_chars` needs raw pointers rather than iterators.
`std::to_address(text.begin())` / `end()` extracts the pointer on all standard libraries,
so no pointer arithmetic and no `NOLINT` are required.

Behavior is unchanged.

Assisted-by: Claude Opus 4.7 via Claude Code
@rturrado
rturrado marked this pull request as ready for review September 8, 2026 10:05
`parseBitRegisterRef` was dereferencing the `std::optional<size_t>` returned by `parseUnsignedInt`
and immediately re-wrapping it as the `.bitIndex` member of `BitRegisterRef`.
clang-tidy's `bugprone-optional-value-conversion` flagged the pattern as potentially error-prone.
Assign the optional directly instead.

Assisted-by: Claude Opus 4.7 via Claude Code
…al conditions

`parseBitRegisterRef` (introduced in munich-quantum-toolkit#463) and the parsing loop inside `validateTargets` both walked a `name` or `name[index]` shape with almost identical logic.
`parseBitRegisterRef` served only the classical-condition path and `validateTargets` served only qubit targets, so the shared shape lived in two places and could drift.

Rename `BitRegisterRef` and `parseBitRegisterRef` to the generic `RegisterRef` and `parseRegisterRef`, and use the helper inside `validateTargets` for the structural parsing.
`validateTargets` keeps its qubit-specific semantic checks (shadowedRegisters, definedRegisters existence, index bounds) on top of the parsed result.

Also drop the `<stdexcept>` include: `parseRegisterRef` uses `std::from_chars` (no exceptions), so the direct dependency on the exception types is gone.

Behavior is unchanged.

Closes munich-quantum-toolkit#470.

Assisted-by: Claude Opus 4.7 via Claude Code
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.

✨ Support if (c) and if (c[k]) as boolean-only conditions ♻️ Unify register-reference parsing between qubit targets and classical conditions

1 participant