🚀 Clear assertion greyed-out lines on restart and highlight issue locations #232
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/common/parsing/CodePreprocessing.cpp`:
- Around line 97-103: The current substring search using lineText.find(target)
can match inside other tokens; update the search to find a whole-token match for
target (e.g., use a regex with word boundaries or tokenize lineText and scan
tokens) so the matched position corresponds to the exact identifier rather than
a substring. Locate the code that uses target, lineText, and location and
replace the find() call with a whole-word search that returns the starting index
of the exact token, then set location.column = (startIndex + 1) and return
location as before.
- Around line 78-109: The code duplicates the logic for computing a line's start
in lineColumnForTarget that already exists in lineColumnForOffset; extract that
logic into a small helper (e.g., findLineStart(const std::string& code, size_t
offset)) and replace the duplicated block in lineColumnForTarget with a call to
this helper, updating lineStart and any related casts, and consider using the
same helper from lineColumnForOffset to remove duplication.
♻️ Duplicate comments (11)
src/common/parsing/CodePreprocessing.cpp (2)
163-211: Empty target validation behavior needs clarification.At lines 169-174, empty targets now throw a
ParsingErrorwith "Empty target". Previous review discussion questioned whether this is the right behavior since trailing/consecutive commas are syntax errors. IfsplitStringpreserves empty entries from input likecx q[0],, the current code flags "Empty target" rather than "invalid syntax". Confirm this is the intended error message.
192-199: Consider narrowing exception catch tostd::out_of_range.Since
isDigitscheck at line 187 already validates the string contains only digits,std::stoulcan only throwstd::out_of_rangefor extremely large values. Catchingstd::exceptionis defensive but overly broad.♻️ Narrower exception handling
try { registerIndex = std::stoul(indexText); - } catch (const std::exception&) { + } catch (const std::out_of_range&) { throw ParsingError(formatParseError(code, instructionStart, invalidTargetDetail(target, context), target)); }python/mqt/debugger/dap/dap_server.py (4)
469-476: PotentialAttributeErrorifsource_codeis not set.The fallback highlight creation (lines 469-476) is outside the
if self.source_code:guard (line 454), but_build_highlight_entryaccessesself.source_codeat line 497. Sincesource_codeis now initialized to""in__init__, this shouldn't crash, but the fallback may produce entries with empty snippets.
370-377: Consider extracting highlight emission into a helper method.The pattern of creating
HighlightError, sending it, and setting_prevent_exitis duplicated here and at lines 263-274. Extracting into a helper would improve maintainability.♻️ Proposed helper method
def _emit_highlights(self, highlights: list[dict[str, Any]], connection: socket.socket) -> None: """Emit highlight entries to the client.""" if not highlights: return try: highlight_event = mqt.debugger.dap.messages.HighlightError(highlights, self.source_file) send_message(json.dumps(highlight_event.encode()), connection) self._prevent_exit = True except (TypeError, ValueError): pass
560-566: Redundant condition check at line 566.Since
column = max(1, column)at line 545 andend_column = max(column, len(line_text) + 1)at line 560,end_columnis always ≥ 1. The checkend_column if end_column > 0 else columnis redundant.♻️ Simplification
"range": { "start": {"line": line, "column": column}, - "end": {"line": line, "column": end_column if end_column > 0 else column}, + "end": {"line": line, "column": end_column}, },
623-624: Potential"None"string in output.
title = str(message.get("title", ""))will render"None"iftitleis explicitly set toNone. Consider checking the type before conversion.🛡️ Proposed fix
- title = str(message.get("title", "")) + raw_title = message.get("title", "") + title = raw_title if isinstance(raw_title, str) else ""src/backend/dd/DDSimDebug.cpp (5)
118-127:std::stoullcan throw on overflow despite digit validation.The
parseIndexlambda validates digits but doesn't wrapstd::stoullin try-catch. Extremely large numeric strings could throwstd::out_of_range. This was flagged in a past review.🛡️ Suggested fix
- value = std::stoull(text); - return true; + try { + value = std::stoull(text); + return true; + } catch (const std::exception&) { + return false; + }
149-163: Map access withoperator[]can silently insert default entries.Using
ddsim->variables[bitName](line 150) andddsim->variables[name](line 161) withoperator[]will insert a default-constructed entry if the key doesn't exist, corrupting state and returningfalseinstead ofstd::nullopt. This was flagged in a past review.🐛 Proposed fix using find()
const auto bitName = base + "[" + std::to_string(bitIndex) + "]"; - const auto& value = ddsim->variables[bitName].value.boolValue; - registerValue = value ? 1ULL : 0ULL; + const auto varIt = ddsim->variables.find(bitName); + if (varIt == ddsim->variables.end()) { + return std::nullopt; + } + registerValue = varIt->second.value.boolValue ? 1ULL : 0ULL;Apply similar fix for the loop at lines 159-163.
201-202:std::stoulcalls should be wrapped in try-catch.Although digit validation is performed, extremely large values can still cause
std::out_of_range. This was noted in a past review.🛡️ Suggested fix
+ size_t line = 0; + size_t column = 0; + try { + line = std::stoul(lineStr); + column = std::stoul(columnStr); + } catch (const std::exception&) { + return {0, 0, trimmed}; + } - const size_t line = std::stoul(lineStr); - const size_t column = std::stoul(columnStr);
772-778: Consider a more context-specific fallback error message.The fallback message "An error occurred while executing the operation" is generic for a code loading context. A more descriptive message like "Failed to parse or load the provided code" would improve diagnostics.
1170-1193: Consider extracting duplicated condition evaluation logic.The condition evaluation logic here (lines 1170-1193) and in
stepBackward(lines 1268-1291) is nearly identical. Extracting this into a helper function would reduce duplication and ensure consistent behavior.♻️ Suggested helper extraction
bool evaluateIfElseCondition(DDSimulationState* ddsim, size_t instructionIndex, const qc::IfElseOperation* op) { const auto condition = evaluateClassicConditionFromCode(ddsim, instructionIndex); if (condition.has_value()) { return condition.value(); } // Fallback to control-bit/bitmask logic const auto& exp = op->getExpectedValueRegister(); size_t registerValue = 0; // ... rest of fallback logic return (registerValue == exp); }
| /** | ||
| * @brief Compute the 1-based line and column for a target within a line. | ||
| * @param code The source code to inspect. | ||
| * @param instructionStart The zero-based offset of the instruction start. | ||
| * @param target The target token to locate on the line. | ||
| * @return The line and column of the target, or the first non-space column. | ||
| */ | ||
| LineColumn lineColumnForTarget(const std::string& code, size_t instructionStart, | ||
| const std::string& target) { | ||
| LineColumn location = lineColumnForOffset(code, instructionStart); | ||
| const auto lineStartPos = code.rfind('\n', instructionStart); | ||
| const size_t lineStart = (lineStartPos == std::string::npos) | ||
| ? 0 | ||
| : static_cast<size_t>(lineStartPos + 1); | ||
| auto lineEndPos = code.find('\n', instructionStart); | ||
| const size_t lineEnd = (lineEndPos == std::string::npos) | ||
| ? code.size() | ||
| : static_cast<size_t>(lineEndPos); | ||
| const auto lineText = code.substr(lineStart, lineEnd - lineStart); | ||
| if (!target.empty()) { | ||
| const auto targetPos = lineText.find(target); | ||
| if (targetPos != std::string::npos) { | ||
| location.column = targetPos + 1; | ||
| return location; | ||
| } | ||
| } | ||
| const auto nonSpace = lineText.find_first_not_of(" \t"); | ||
| if (nonSpace != std::string::npos) { | ||
| location.column = nonSpace + 1; | ||
| } | ||
| return location; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting duplicated lineStart calculation.
The lineStart calculation logic (lines 88-91) duplicates the logic from lineColumnForOffset (lines 64-67). This was noted in a past review. A small helper like findLineStart(code, offset) would reduce duplication.
♻️ Optional helper extraction
size_t findLineStart(const std::string& code, size_t offset) {
const auto lineStartPos = code.rfind('\n', offset);
return (lineStartPos == std::string::npos) ? 0 : lineStartPos + 1;
}🤖 Prompt for AI Agents
In `@src/common/parsing/CodePreprocessing.cpp` around lines 78 - 109, The code
duplicates the logic for computing a line's start in lineColumnForTarget that
already exists in lineColumnForOffset; extract that logic into a small helper
(e.g., findLineStart(const std::string& code, size_t offset)) and replace the
duplicated block in lineColumnForTarget with a call to this helper, updating
lineStart and any related casts, and consider using the same helper from
lineColumnForOffset to remove duplication.
| if (!target.empty()) { | ||
| const auto targetPos = lineText.find(target); | ||
| if (targetPos != std::string::npos) { | ||
| location.column = targetPos + 1; | ||
| return location; | ||
| } | ||
| } |
There was a problem hiding this comment.
Substring matching may report incorrect column for certain targets.
Using lineText.find(target) can match a substring rather than the exact token. For example, if target = "q" and the line contains qreg q[2], this returns column 1 (matching q in qreg) rather than the actual q register. The line number will still be correct, so this is a minor UX concern.
🤖 Prompt for AI Agents
In `@src/common/parsing/CodePreprocessing.cpp` around lines 97 - 103, The current
substring search using lineText.find(target) can match inside other tokens;
update the search to find a whole-token match for target (e.g., use a regex with
word boundaries or tokenize lineText and scan tokens) so the matched position
corresponds to the exact identifier rather than a substring. Locate the code
that uses target, lineText, and location and replace the find() call with a
whole-word search that returns the starting index of the exact token, then set
location.column = (startIndex + 1) and return location as before.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/backend/dd/DDSimDebug.cpp`:
- Around line 93-112: evaluateClassicCondition currently uses
ddsim->variables[name] which can insert/default-construct entries (and won't
compile if variables is const); change both lookups to use variables.find(name)
(or variables.at(name) if variables is const and you want an exception on
missing) and explicitly handle the missing-key case instead of silently
defaulting—e.g., log/report an error (or return an explicit failure) when find()
== end(), then read the boolValue from the found iterator; keep references to
evaluateClassicCondition, getClassicalBitName and ddsim->variables to locate the
fixes and ensure the changes compile with the function's const
DDSimulationState* signature.
♻️ Duplicate comments (2)
src/backend/dd/DDSimDebug.cpp (2)
1492-1496: Add a comment documenting the breakpoint snapping behavior.This logic snaps breakpoints to the first instruction when
desiredPositionfalls before it. While the behavior is correct (DAP clients receive the actual position), a brief inline comment would help maintainers understand the design intent without needing to trace through the DAP response handling.📝 Suggested documentation
for (auto i = 0ULL; i < ddsim->instructionTypes.size(); i++) { const size_t start = ddsim->instructionStarts[i]; const size_t end = ddsim->instructionEnds[i]; + // Snap breakpoints requested before the first instruction to that instruction. + // The actual breakpoint position is returned to the caller/DAP client. if (desiredPosition < start) { *targetInstruction = i; ddsim->breakpoints.insert(i); return OK; }
146-147: Wrapstd::stoulin try-catch to handle overflow gracefully.The digit validation at lines 140-143 prevents
std::invalid_argumentbut notstd::out_of_rangefor extremely large numeric strings. Since this function is designed to return a safe default{0, 0, trimmed}on parse failures, unhandled exceptions break that contract.🛡️ Suggested fix
- const size_t line = std::stoul(lineStr); - const size_t column = std::stoul(columnStr); + size_t line = 0; + size_t column = 0; + try { + line = std::stoul(lineStr); + column = std::stoul(columnStr); + } catch (const std::exception&) { + return {.line = 0, .column = 0, .detail = trimmed}; + }
svenjeschmitt-ops
force-pushed
the
main
branch
from
146ea50 to
6b9c1d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/dd/DDSimDebug.cpp (1)
1497-1507: Consider documenting the breakpoint snapping behavior.The code correctly snaps breakpoints to the nearest instruction when the requested position is before an instruction start. As discussed in previous reviews, this behavior should be documented in the function's docstring to help maintainers understand the design intent:
- Positions before the first instruction snap to that instruction
- Positions after all instructions return ERROR (line 1535)
- DAP clients receive the actual instruction position via
targetInstruction
🤖 Fix all issues with AI agents
In `@src/backend/dd/DDSimDebug.cpp`:
- Around line 720-742: Add a brief explanatory comment above the catch(...)
block clarifying its purpose: note that it is intended to catch
non-std::exception throws from third‑party or C libraries and provide a safe
fallback error message via lastLoadErrorDetail and the returned
LOAD_INTERNAL_ERROR, similar to how parseLoadErrorMessage and the std::exception
handler populate line/column and message/LOAD_PARSE_ERROR; place the comment
immediately before the catch(...) to make intent clear to reviewers.
♻️ Duplicate comments (2)
src/backend/dd/DDSimDebug.cpp (2)
94-113: Map access viaoperator[]can corrupt state if key is missing.Lines 101 and 108 use
ddsim->variables[name]which will insert a default-constructed entry if the key doesn't exist, silently corrupting state and returningfalseinstead of properly signaling an error. This issue was flagged in a previous review but remains unaddressed.🐛 Proposed fix using find()
bool evaluateClassicCondition(DDSimulationState* ddsim, const qc::IfElseOperation* op) { const auto& exp = op->getExpectedValueRegister(); size_t registerValue = 0; if (op->getControlBit().has_value()) { const auto controlBit = op->getControlBit().value(); const auto name = getClassicalBitName(ddsim, controlBit); - const auto& value = ddsim->variables[name].value.boolValue; - registerValue = value ? 1ULL : 0ULL; + const auto it = ddsim->variables.find(name); + if (it == ddsim->variables.end()) { + // Variable not found - this indicates an internal error + return false; + } + registerValue = it->second.value.boolValue ? 1ULL : 0ULL; } else { const auto& controls = op->getControlRegister(); for (size_t i = 0; i < controls->getSize(); i++) { const auto name = getClassicalBitName(ddsim, controls->getStartIndex() + i); - const auto& value = ddsim->variables[name].value.boolValue; - registerValue |= (value ? 1ULL : 0ULL) << i; + const auto it = ddsim->variables.find(name); + if (it == ddsim->variables.end()) { + return false; + } + registerValue |= (it->second.value.boolValue ? 1ULL : 0ULL) << i; } } return registerValue == exp; }
158-172: Wrapstd::stoulcalls in try-catch to handle overflow gracefully.The digit validation (lines 159-162) prevents most malformed input, but doesn't protect against numeric overflow. A string like
"99999999999999999999"passes the digit check but causesstd::stoulto throwstd::out_of_range. A previous review indicated this was addressed, but the current code doesn't show the protection.🛡️ Suggested defensive fix
if (lineStr.empty() || columnStr.empty() || !std::ranges::all_of(lineStr, isDigit) || !std::ranges::all_of(columnStr, isDigit)) { return {.line = 0, .column = 0, .detail = trimmed}; } - const size_t line = std::stoul(lineStr); - const size_t column = std::stoul(columnStr); + size_t line = 0; + size_t column = 0; + try { + line = std::stoul(lineStr); + column = std::stoul(columnStr); + } catch (const std::exception&) { + return {.line = 0, .column = 0, .detail = trimmed}; + } std::string detail = trim(trimmed.substr(columnEnd + 1));
| } catch (const std::exception& e) { | ||
| std::cerr << e.what() << "\n"; | ||
| return ERROR; | ||
| std::string message = e.what(); | ||
| if (message.empty()) { | ||
| message = "An error occurred while executing the operation"; | ||
| } | ||
| const auto parsed = parseLoadErrorMessage(message); | ||
| lastLoadErrorDetail = parsed.detail; | ||
| const LoadResultStatus status = (parsed.line > 0 || parsed.column > 0) | ||
| ? LOAD_PARSE_ERROR | ||
| : LOAD_INTERNAL_ERROR; | ||
| return {.status = status, | ||
| .line = parsed.line, | ||
| .column = parsed.column, | ||
| .message = lastLoadErrorDetail.empty() | ||
| ? nullptr | ||
| : lastLoadErrorDetail.c_str()}; | ||
| } catch (...) { | ||
| lastLoadErrorDetail = "An error occurred while executing the operation"; | ||
| return {.status = LOAD_INTERNAL_ERROR, | ||
| .line = 0, | ||
| .column = 0, | ||
| .message = lastLoadErrorDetail.c_str()}; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good structured error handling with location extraction.
The error handling correctly:
- Captures exception messages and provides fallback for empty messages
- Parses location information from error messages
- Distinguishes between parse errors (with line/column) and internal errors
- Uses
catch(...)as a defensive measure against non-standard exceptions from imported libraries
Consider adding a brief comment explaining why catch(...) is needed (e.g., // Catch non-std::exception throws from third-party code) to address reviewer questions about its purpose.
🤖 Prompt for AI Agents
In `@src/backend/dd/DDSimDebug.cpp` around lines 720 - 742, Add a brief
explanatory comment above the catch(...) block clarifying its purpose: note that
it is intended to catch non-std::exception throws from third‑party or C
libraries and provide a safe fallback error message via lastLoadErrorDetail and
the returned LOAD_INTERNAL_ERROR, similar to how parseLoadErrorMessage and the
std::exception handler populate line/column and message/LOAD_PARSE_ERROR; place
the comment immediately before the catch(...) to make intent clear to reviewers.
783f0d3 to
c1eaccd
Compare
svenjeschmitt-ops
force-pushed
the
main
branch
from
997e481 to
fa522a6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/cli/CliFrontEnd.cpp (1)
195-195: UncheckedloadCodereturn value could mask failures.With the new
LoadResultAPI, this call (and similar ones at lines 231 and 248) should check the return status. If code loading fails here, subsequent operations likerunAllorgetInstructionPositionmay operate on invalid state, leading to undefined behavior or confusing errors.🔧 Suggested fix pattern
- state->loadCode(state, newCode.c_str()); + if (state->loadCode(state, newCode.c_str()).status != LOAD_OK) { + std::cout << "Error loading updated code\n"; + return; + }Apply similar checks at lines 231 and 248.
♻️ Duplicate comments (6)
python/mqt/debugger/dap/messages/launch_dap_message.py (2)
69-69: Use an absolutesource_file.pathto keep editor mapping stable.
Ifself.programis relative, breakpoints/highlights can drift with CWD changes. Consider normalizing toprogram_path.resolve().♻️ Proposed fix
- server.source_file = {"name": program_path.name, "path": self.program} + server.source_file = {"name": program_path.name, "path": str(program_path.resolve())}
71-71: Fix FBT003: use a keyword argument forgetpreferredencoding.
Ruff flags the boolean positional argument.🐛 Proposed fix
- code = program_path.read_text(encoding=locale.getpreferredencoding(False)) + code = program_path.read_text(encoding=locale.getpreferredencoding(do_setlocale=False))src/backend/dd/DDSimDebug.cpp (4)
118-127: Guardstd::stoullconversions to keep parsing non-throwing.
Digit checks don’t preventstd::stoullfrom throwing on overflow;parseIndexshould returnfalseinstead of propagating exceptions.🛠️ Suggested fix
const auto parseIndex = [](const std::string& text, size_t& value) -> bool { if (text.empty()) { return false; } if (std::ranges::any_of(text, [](unsigned char c) { return !std::isdigit(c); })) { return false; } - value = std::stoull(text); - return true; + try { + value = static_cast<size_t>(std::stoull(text)); + return true; + } catch (const std::exception&) { + return false; + } };std::stoull out_of_range invalid_argument exception behavior
149-162: Avoidvariables[...]lookups that mutate state.
operator[]inserts defaults when keys are missing, which can silently create variables and skew condition evaluation. Usefind()/contains()and handle missing keys explicitly (e.g., returnstd::nulloptinevaluateClassicConditionFromCode, and returnERRORor fail-safe in the fallback blocks).🛠️ Suggested fix (apply same pattern in the fallback blocks)
- const auto& value = ddsim->variables[bitName].value.boolValue; - registerValue = value ? 1ULL : 0ULL; + const auto it = ddsim->variables.find(bitName); + if (it == ddsim->variables.end()) { + return std::nullopt; + } + registerValue = it->second.value.boolValue ? 1ULL : 0ULL; ... - const auto& value = ddsim->variables[name].value.boolValue; - registerValue |= (value ? 1ULL : 0ULL) << i; + const auto it = ddsim->variables.find(name); + if (it == ddsim->variables.end()) { + return std::nullopt; + } + registerValue |= (it->second.value.boolValue ? 1ULL : 0ULL) << i;std::map operator[] inserts default elementAlso applies to: 1176-1189, 1276-1288
191-203: Handlestd::stoulexceptions inparseLoadErrorMessage.
Even with digit checks,std::stoulcan throw on overflow; return a safeParsedLoadErrorinstead of letting the exception escape.🛠️ Suggested fix
- const size_t line = std::stoul(lineStr); - const size_t column = std::stoul(columnStr); + size_t line = 0; + size_t column = 0; + try { + line = std::stoul(lineStr); + column = std::stoul(columnStr); + } catch (const std::exception&) { + return {.line = 0, .column = 0, .detail = trimmed}; + }std::stoul out_of_range invalid_argument exception behavior
1593-1597: Document the breakpoint “snap-forward” behavior.
Consider a short comment or docstring clarifying that positions before the next instruction are snapped forward, while requests past the last instruction returnERROR.📝 Suggested doc hint
- if (desiredPosition < start) { + // Snap to the next instruction if the requested position is before it. + if (desiredPosition < start) {
DRovara
left a comment
DRovara
left a comment
There was a problem hiding this comment.
Thanks for the update. I guess not all of my previous comments have been addressed yet. Let me know when you're done with that!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/common/parsing/CodePreprocessing.cpp`:
- Around line 168-178: The loop over targets in CodePreprocessing.cpp currently
skips validation when no '[' is found, allowing unknown unindexed targets to
pass; replace the early continue in the branch where open == std::string::npos
by checking whether target exists in shadowedRegisters or definedRegisters and
if not, construct the same kind of ParsingError (use instructionStart,
formatParseError, code and context) and throw it; keep existing behavior for
known targets but ensure the check uses the exact variables shadowedRegisters
and definedRegisters and produces a clear "Unknown target" detail string before
throwing.
- Around line 536-540: The inlining step exposes that parseClassicControlledGate
currently calls replaceString(code, "if", "") which removes every "if"
substring; change parseClassicControlledGate to only remove the leading "if"
keyword (and any immediate whitespace) from the beginning of the code string
instead of global replacement. Locate parseClassicControlledGate and replace the
global replaceString call with logic that trims leading whitespace and, if the
string starts with "if" as a standalone token, erase only that prefix (or remove
the first token) so nested "if" occurrences and identifiers remain intact;
ensure this operates on the same variable passed to block.code so the subsequent
line.append(...) receives the corrected code.
♻️ Duplicate comments (2)
src/common/parsing/CodePreprocessing.cpp (2)
62-75: Potential column=0 when offset hits a newline.If the offset points directly at
\n, the computed column can become 0 instead of 1. Consider adjusting the lookup to search before the offset or clamp the offset to the previous character.
85-107: Substring match can misplace the reported column.
lineText.find(target)can match inside other tokens (e.g.,qinsideqreg), yielding the wrong column. A whole-token match would avoid this.
| for (const auto& target : targets) { | ||
| if (target.empty()) { | ||
| std::string detail = "Empty target"; | ||
| detail += context; | ||
| detail += "."; | ||
| throw ParsingError(formatParseError(code, instructionStart, detail)); | ||
| } | ||
| const auto open = target.find('['); | ||
| if (open == std::string::npos) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Unindexed targets bypass validation.
When no [ is present, the target is skipped entirely. That lets unknown targets like foo pass without error unless they’re shadowed or defined. Consider validating unindexed targets against shadowedRegisters and definedRegisters before continuing.
🐛 Proposed fix
const auto open = target.find('[');
if (open == std::string::npos) {
- continue;
+ if (std::ranges::find(shadowedRegisters, target) !=
+ shadowedRegisters.end()) {
+ continue;
+ }
+ if (definedRegisters.contains(target)) {
+ continue;
+ }
+ throw ParsingError(formatParseError(code, instructionStart,
+ invalidTargetDetail(target, context),
+ target));
}🤖 Prompt for AI Agents
In `@src/common/parsing/CodePreprocessing.cpp` around lines 168 - 178, The loop
over targets in CodePreprocessing.cpp currently skips validation when no '[' is
found, allowing unknown unindexed targets to pass; replace the early continue in
the branch where open == std::string::npos by checking whether target exists in
shadowedRegisters or definedRegisters and if not, construct the same kind of
ParsingError (use instructionStart, formatParseError, code and context) and
throw it; keep existing behavior for known targets but ensure the check uses the
exact variables shadowedRegisters and definedRegisters and produces a clear
"Unknown target" detail string before throwing.
| if (block.valid && isClassicControlledGate(line)) { | ||
| line.append(" { ").append(block.code).append(" }"); | ||
| block.valid = false; | ||
| block.code.clear(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CodePreprocessing.cpp" -type fRepository: munich-quantum-toolkit/debugger
Length of output: 117
🏁 Script executed:
wc -l ./src/common/parsing/CodePreprocessing.cppRepository: munich-quantum-toolkit/debugger
Length of output: 121
🏁 Script executed:
sed -n '530,550p' ./src/common/parsing/CodePreprocessing.cpp | cat -nRepository: munich-quantum-toolkit/debugger
Length of output: 945
🏁 Script executed:
grep -n "parseClassicControlledGate" ./src/common/parsing/CodePreprocessing.cpp | head -20Repository: munich-quantum-toolkit/debugger
Length of output: 224
🏁 Script executed:
sed -n '387,420p' ./src/common/parsing/CodePreprocessing.cpp | cat -nRepository: munich-quantum-toolkit/debugger
Length of output: 1442
🏁 Script executed:
sed -n '536,545p' ./src/common/parsing/CodePreprocessing.cpp | cat -nRepository: munich-quantum-toolkit/debugger
Length of output: 415
🏁 Script executed:
sed -n '425,440p' ./src/common/parsing/CodePreprocessing.cpp | cat -nRepository: munich-quantum-toolkit/debugger
Length of output: 730
🏁 Script executed:
grep -r "parseClassicControlledGate\|if_" ./src --include="*.cpp" --include="*.h" -n | head -30Repository: munich-quantum-toolkit/debugger
Length of output: 729
🏁 Script executed:
grep -r "if.*{" ./src/common/parsing --include="*.cpp" --include="*.h" -A 2 -B 2 | head -50Repository: munich-quantum-toolkit/debugger
Length of output: 3453
🏁 Script executed:
find . -name "*.cpp" -o -name "*.h" -o -name "*.mlir" -o -name "*.qasm" | xargs grep -l "if\s*{" | head -10Repository: munich-quantum-toolkit/debugger
Length of output: 57
🏁 Script executed:
grep -r "if\s*(" ./src --include="*.cpp" -A 2 | grep -E "(if\s*\(|parseClassicControlledGate)" | head -20Repository: munich-quantum-toolkit/debugger
Length of output: 1827
🏁 Script executed:
find . -name "*.cpp" -o -name "*.h" | xargs grep -l "if.*{.*}" | xargs grep -B5 -A5 "classic.*control\|control.*gate" | head -40Repository: munich-quantum-toolkit/debugger
Length of output: 1978
🏁 Script executed:
grep -A 10 "if_.*{" ./src --include="*.cpp" -r | head -50Repository: munich-quantum-toolkit/debugger
Length of output: 57
🏁 Script executed:
find . -name "*.cpp" -path "*/test/*" | xargs grep -l "if.*{" | head -5Repository: munich-quantum-toolkit/debugger
Length of output: 250
🏁 Script executed:
grep -r "if\s*(" ./test --include="*.cpp" | grep -i "gate\|control" | head -20Repository: munich-quantum-toolkit/debugger
Length of output: 57
🏁 Script executed:
grep -B 5 -A 10 "parseClassicControlledGate" ./src/backend/dd/DDSimDebug.cppRepository: munich-quantum-toolkit/debugger
Length of output: 632
Only strip the leading if keyword in parseClassicControlledGate, not all occurrences.
Block inlining (line 538) exposes a bug where replaceString(code, "if", "") strips ALL "if" substrings in the entire code, including nested structures and identifiers. This corrupts parsing of inline blocks containing nested classic-controlled gates or other "if" occurrences.
Suggested fix in parseClassicControlledGate
- const auto codeSanitized = trim(replaceString(code, "if", ""));
+ auto codeSanitized = trim(code);
+ if (codeSanitized.starts_with("if")) {
+ codeSanitized = trim(codeSanitized.substr(2));
+ }🤖 Prompt for AI Agents
In `@src/common/parsing/CodePreprocessing.cpp` around lines 536 - 540, The
inlining step exposes that parseClassicControlledGate currently calls
replaceString(code, "if", "") which removes every "if" substring; change
parseClassicControlledGate to only remove the leading "if" keyword (and any
immediate whitespace) from the beginning of the code string instead of global
replacement. Locate parseClassicControlledGate and replace the global
replaceString call with logic that trims leading whitespace and, if the string
starts with "if" as a standalone token, erase only that prefix (or remove the
first token) so nested "if" occurrences and identifiers remain intact; ensure
this operates on the same variable passed to block.code so the subsequent
line.append(...) receives the corrected code.
421aa6a to
50abd27
Compare
svenjeschmitt-ops
force-pushed
the
main
branch
2 times, most recently
from
54bf9cf to
50abd27
Compare
This reverts commit dd89f76.
3 participants
Description
This pull request improves the debugging highlight functionality and fixes an issue where greyed-out highlights were not correctly reset between executions.
When an issue is detected in the code, the extension highlights the affected line in red and displays a descriptive error message explaining the nature of the problem. This visual feedback helps users quickly identify both the location of the error and its underlying cause.
A second part of the implementation concerns the assertion functionality. After an assertion failure, the extension greys out all lines of code that cannot have caused the error. While this behaviour is intended, the greyed-out highlights were previously not cleared when a new execution started. As a result, lines remained greyed out until the editor was closed and reopened.
Overall, the highlighting functionality provides an efficient way to show where an error occurred and what caused it. This change introduces an explicit reset of all greyed-out highlights on launch and restart events, ensuring that each execution starts with a clean editor state.
Fixes #(issue)
Checklist: