Fix #15XXX syntaxError for ternary with two inequalities - #8913
chrchr-github wants to merge 2 commits into
Conversation
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
This is an AI review take it with a grain of salt. Please feel free to reject it by clicking on Resolve button
Verdict: the logic looks correct to me. An unmatched : cannot appear in valid template arguments, so returning 0 there only makes template detection stricter and should not introduce false positives. Real ternary template arguments still work (existing tests cover std::array<int, B ? 1 : 2> and nested ternaries), parenthesized ternaries are skipped via the link, and :: is a separate token so it is not affected. I built the branch locally and TestSimplifyTemplate, TestTokenizer, TestSimplifyTypedef, TestVarID and TestGarbage pass (1321 tests, 0 failed). The inline comments are only style/naming nits.
The PR title still says #15XXX; please replace it with the actual trac ticket number.
| return 0; | ||
|
|
||
| unsigned int level = 0; | ||
| unsigned int inTernary = 0; |
There was a problem hiding this comment.
This is an AI review take it with a grain of salt. Please feel free to reject it by clicking on Resolve button
Minor naming nit: inTernary reads like a bool, but it is a nesting counter. Something like ternaryLevel would match the existing level variable in this function and make --ternaryLevel read more naturally.
| // Skip '=', '?', ':' | ||
| if (Token::Match(tok, "=|?|:")) | ||
| if (Token::Match(tok, "=|?|:")) { | ||
| if (tok->str()[0] == '?') |
There was a problem hiding this comment.
This is an AI review take it with a grain of salt. Please feel free to reject it by clicking on Resolve button
Since the surrounding code compares whole strings (tok->str() == ">", tok->str() == ","), tok->str() == "?" / tok->str() == ":" would be more consistent and would not rely on the Token::Match above to rule out other tokens starting with those characters. It might also be worth updating the // Skip '=', '?', ':' comment to say that an unmatched : means this is not a template argument list.
2 participants
No description provided.