← 输入框|github.com|刷新
Skip to content

Fix #15165 Tokens copied by typedef lose their macro name - #8988

Open
wjakobsson wants to merge 2 commits into
cppcheck-opensource:mainfrom
wjakobsson:fixexpandedmacro
Open

wjakobsson wants to merge 2 commits into
cppcheck-opensource:mainfrom
wjakobsson:fixexpandedmacro

Conversation

@wjakobsson

Copy link
Copy Markdown
Collaborator

No description provided.

@dmcppcheck

Copy link
Copy Markdown
Contributor

Test results for commit 84bb93c (tools/test-my-pr.py, main compared to this PR):

Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8988/
packages: 1000
Differing warnings: 0 (+0 -0)
Timing: +0.0% (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8988/2026-11-02_15-16-43-84bb93cbd028_timing.html)

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.

@dmcppcheck dmcppcheck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

I found no correctness problems. Both changes now carry the macro name over to the copied tokens, and I found no input where the analysis results change.

One gap in test coverage: the new test simplifyTypedefMacroName only exercises the change in TypedefSimplifier::insertTokens (lib/tokenize.cpp). The change in TokenList::copyTokens (lib/tokenlist.cpp) is not tested by anything. If I revert it, every test still passes. That function is used by simplifyUsing, simplifyTypedefCpp, struct splitting and template alias and default-argument copying. Its effect is easy to see, for example with using F = void (*)(A x);, so a test for it would be worth adding.

Testing

Builds: make -j8 cppcheck testrunner succeeded in pr/, and make -j8 cppcheck succeeded in base/.

Unit tests:

  • ./testrunner TestSimplifyTypedef TestSimplifyUsing TestTokenList TestSimplifyTemplate TestTokenizer: 937 tests, 0 failures.
  • Full ./testrunner -q: 5388 tests, 0 failures.

Which change the new test covers:

  • I temporarily reverted only the lib/tokenlist.cpp change and rebuilt. TestSimplifyTypedef still passed (0 failures), so the copyTokens change is untested.
  • I temporarily reverted only the insertTokens change in lib/tokenize.cpp. simplifyTypedefMacroName then failed as expected.
  • I restored both files and rebuilt afterwards.

The copyTokens path changes behaviour compared with base:

#define A int
using F = void (*)(A x);
void f(F g);

cppcheck --debug-normal t3.cpp:

  • PR: void f ( void ( * g@var1 ) ( $int ) ) ;
  • base: void f ( void ( * g@var1 ) ( int ) ) ;

Comparing results: I wrote a file that mixes macros with typedef/using arrays, anonymous struct typedefs, a typedef inside a macro, template default arguments from macros, and an unsigned typedef from a macro (d1.cpp). I ran it with --enable=all --inconclusive --check-level=exhaustive, and PR and base gave the same warnings. I also checked the --debug-normal output for typedef function pointers, arrays and struct typedefs. The $ markers now appear on the copied tokens, and I saw nothing wrong.


Automated review of 84bb93c by Claude. The pull request was built and tested locally.

Comment thread lib/tokenlist.cpp
tok2->varId(tok->varId());
tok2->setTokenDebug(tok->getTokenDebug());
if (tok->isExpandedMacro())
tok2->setMacroName(tok->getMacroName());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

No test covers this change. If I revert it, TestSimplifyTypedef (including the new simplifyTypedefMacroName) and the rest of the testrunner still pass, because the new test only goes through TypedefSimplifier::insertTokens. copyTokens is used by simplifyUsing, simplifyTypedefCpp and the template simplifier, and the effect is visible there. For example, with #define A int / using F = void (*)(A x); / void f(F g);, the PR gives ( $int ) where base gives ( int ). Please add a test for this path, for example in TestSimplifyUsing.

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.

2 participants