Repository navigation
Conversation
| // Preserve an asm statement as an analysis barrier, including for empty | ||
| // blocks. Hide assembler # operands before simplecpp stringifies them. | ||
| std::unique_ptr<simplecpp::Token> open(new simplecpp::Token("(", start->location)); | ||
| std::unique_ptr<simplecpp::Token> close(new simplecpp::Token(")", start->location)); |
There was a problem hiding this comment.
This is an AI review. Take it with a grain of salt and feel free to reject it by resolving the comment.
I built the PR and compared it with main. Other asm forms are unchanged: __asm("..."), __asm volatile(...), MS __asm { ... }, one-line __asm nop __endasm;, and int x __asm("sym");. Unbraced if/do bodies with a block still parse. As a bonus, a { inside an asm ; comment no longer gives a syntax error.
One behavior change I noticed: the assembler text is now dropped. For
void f(void) {
__asm
mov ax,bx
__endasm;
}main produces asm ( "mov ax , bx" ) ; (from Tokenizer::simplifyAsm(), see testtokenize.cpp around line 1191). With this PR, the --debug output and the dump file show asm ( "" ) ;. I could not find any checker or addon that reads the asm string, and #pragma asm already produces an empty asm ( ), so this may be fine. But it means the __endasm branch in Tokenizer::simplifyAsm() is now effectively dead for preprocessed input. Is that intended? If so, maybe remove or document that branch. Otherwise, maybe keep the text here by turning the removed tokens into a string literal between ( and )?
There was a problem hiding this comment.
I think the assembler text is used somehow and shouldn't be dropped. However I am not sure what we use it for. One possible idea is when checking if the code in two scopes are the same to find copy paste mistakes, then we want to know if the same assembler instructions are provided.
for this purpose it's enough with a hash or an id string we don't need to see the exact code..
|
Test results for commit b66fae3 (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8892/ Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong. |
3 participants
Addresses the remaining SDCC examples in Trac 6028.
@dptroperands currently cause a syntax error, and#(s_XINIT >> 8)is altered during preprocessing before the late assembler simplifier runs, leaving an unmatched).Normalize complete literal
__asm/__endasmregions in the raw preprocessor tokens, including newly loaded headers. Preserve an opaqueasm()analysis barrier, exactly one terminator and the locations of surrounding C code. Leave ambiguous/missing/nested regions, real preprocessing directives, macro definitions and other assembler dialects on the existing path. The existing pragma-asm handling is unchanged.Add seven preprocessor test methods covering operands, empty/adjacent blocks, comments, missing/nested markers, other dialects, configurations and unbraced control flow. Two CLI regressions cover the main-file and included-header paths and verify that a real division-by-zero diagnostic after the block remains at its original line.
Validation:
@and#(failures; 18 original/patched CLI comparisons pass, including outside-C diagnostic and GNU/MS/macro controls.git diff --checkpass.One earlier Windows full-suite attempt exited with
0xc0000005while displayingTestType::checkTooBigShift_Unix32; the full rerun above passed. The recorded fault offset resolves to the unchangedToken::Match. A bounded debugger run did not reproduce that failure but was stopped after becoming slow inTestValueFlow::valueFlowHang; its outcome is inconclusive. No root cause or baseline attribution is claimed.This is a bounded fix for the demonstrated literal marker-delimited blocks. It does not repair initial lexer errors, macro-generated markers, or ARM assembler-function syntax.
Please assign Trac 6028 to KiritoYG for this patch and confirm whether this remaining-defect fix qualifies under the published USD 10 bounty bracket. After qualifying closure, I can use the documented bounty-request process; please also confirm the available settlement channel.