Repository navigation
Fix #15183 Syntax error with -!! - #9005
chrchr-github wants to merge 2 commits into
Conversation
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
The fix works for ! and ~ after a unary +/-. It is incomplete, though: the same check in concatenateNegativeNumberAndAnyPositive() still reports a false syntaxError on valid code when another prefix operator follows the sign. Examples are return - --i;, return - ++i; and return +&x;. They are the same kind of bug as #15183 and fail the same way on base and on the PR. I found no regressions: invalid code such as return -!; is still rejected, now as internalAstError. return -~; is now accepted silently, but return ~; and return x + ~; are already accepted on base, so that is not new.
Testing
- I built
cppcheckandtestrunnerin pr/, andcppcheckin base/. ./testrunner TestTokenizer: 392 tests, 0 failures.- I compared base and PR on small C files of the form
int x; int f(int i) { <stmt> }, run withcppcheck -q --enable=all. - Fixed by the PR. Base reported
syntaxErroron each of these and the PR reports nothing wrong:return -!!i;return -~i;return +!i;return x * -!i;return x == -!i;return x < -~i;return i ? -!i : -~i;int a[] = {-!i, +~i};case -!0:return sizeof -!i;
- Still a false syntax error on valid code, the same on base and PR:
int x; int f(int i) { return - --i; }
int x; int f(int i) { return - ++i; }
int x; int f(int i) { return +&x; }$ ./cppcheck -q u.c
u.c:1:23: error: syntax error [syntaxError]
- Invalid code:
return -!;now givesinternalAstError; base gavesyntaxError.return -~;is now accepted without any error. Base also acceptsreturn ~;andreturn x + ~;, so this is not a new kind of gap.
Automated review of a05990f by Claude. The pull request was built and tested locally.
| continue; | ||
|
|
||
| if (!tok->tokAt(2) || (tok->tokAt(2)->isOp() && !Token::Match(tok->tokAt(2), "[+-*]"))) | ||
| if (!tok->tokAt(2) || (tok->tokAt(2)->isOp() && !Token::Match(tok->tokAt(2), "[+-*!~]"))) |
There was a problem hiding this comment.
AI review
Valid code with other prefix operators after a unary +/- still gets a false syntaxError here, for the same reason as #15183:
int x; int f(int i) { return - --i; }
int x; int f(int i) { return - ++i; }
int x; int f(int i) { return +&x; }All three still give error: syntax error [syntaxError] with this PR. ++/-- are inc/dec op tokens, and & is an address-of operator here. Consider also allowing ++|--|& after the sign, or only rejecting operators that can't be unary.
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
I found no new problems. The PR now accepts !, ~, &, ++ and -- after a unary +/-. That fixes #15183 and the cases raised in the earlier review (- --i, - ++i, +&x), and the new tokenizer test covers them. Code that is still invalid, such as return -!;, return - ++; and return -!-;, is still rejected, now as internalAstError or syntaxError. One kind of invalid code is newly accepted without an error: return -&;. Base already accepts return &; and return x + &;, so this is the same known gap as -~; from the earlier review and not a new problem.
Earlier comments
| # | Comment | By | Where | Status |
|---|---|---|---|---|
| 1 | False syntaxError still reported for - --i, - ++i, +&x |
AI review | lib/tokenize.cpp:3781 |
✅ Fixed |
Testing
- I built
cppcheckandtestrunnerin pr/, andcppcheckin base/. ./testrunner TestTokenizer: 392 tests, 0 failures../testrunner TestGarbage TestSimplifyTokens TestSymbolDatabase TestValueFlow: 0 failures.- I ran
cppcheck -q --enable=allon 20 small files of the formint x; int f(int i) { <stmt> }. - Valid code, no syntax error now:
-!!i,- --i,+&xx * +~i,x * + !i- - -i,x - - - !i,- *p,-&x == 0-!i ? -~i : +!i,x = -!-!-~i
- Invalid code, compared with base:
| Code | Base | PR |
|---|---|---|
return -!; |
syntaxError | internalAstError |
return - ++; |
syntaxError | internalAstError |
return -!-; |
syntaxError | syntaxError |
return -&; |
syntaxError | no error |
return &; |
no error | no error |
return x + &; |
no error | no error |
return x * &; |
no error | no error |
Base already accepts the last three, so the PR does not create a new kind of gap.
- The GCC extension
return -&&lbl;givessyntaxErroron both base and the PR.
Automated review of f623f9a by Claude. The pull request was built and tested locally.
No description provided.