Skip to content

Fix #15153 FP constParameterPointer with unused attribute - #8976

Merged
danmar merged 2 commits into
cppcheck-opensource:mainfrom
mptre:attribute-unused
Nov 8, 2026
Merged

danmar merged 2 commits into
cppcheck-opensource:mainfrom
mptre:attribute-unused

Conversation

@mptre

@mptre mptre commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Unused parameters annotated with the unused attribute were reported as they could be declared as pointer to const, while [[maybe_unused]] parameters were not.

@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 problems that need fixing, and the change looks correct.

The change to getVariableTokenAfterAttributes reaches further than the parameter case the PR is about. It also fixes local variables and struct members whose declaration lists several names, because the attribute used to land on the next name:

  • int x __attribute__((unused)), y; used to mark y and report unusedVariable for x. Now x is marked and that false positive is gone.
  • struct S { int a __attribute__((unused)), b; }; used to mark b and hide its warning. Now S::b is correctly reported as unusedStructMember.

Neither of these effects is covered by a test. A tokenizer or unused-variable test for them would protect the fix from regressions.

One more effect: isMaybeUnused() is now true for local variables with __attribute__((unused)), so constVariable and unreadVariable are skipped for them in the same way as for [[maybe_unused]]. That is consistent with the existing behaviour.

Testing

  • I built cppcheck and testrunner from pr/ and cppcheck from base/; both built without errors.

  • ./testrunner TestOther TestTokenizer TestUnusedVar TestSymbolDatabase TestUnusedFunctions TestUnusedPrivateFunction TestGarbage TestVarID ran 1841 tests with 0 failures.

  • I ran both binaries with cppcheck -q --enable=all --inconclusive on a scratch file with these cases:

    • the attribute before and after the parameter name
    • __unused__, and unused combined with aligned(4)
    • used, which still warns as expected
    • an unnamed attributed parameter
    • a function pointer parameter
    • an array parameter
    • a parameter that is actually dereferenced (now suppressed, the same as [[maybe_unused]] today)
    • local declarations with several names
    • struct members
    • sizeof(int __attribute__((unused)))

    Differences from base:

    • All constParameterPointer/constParameter reports for __attribute__((unused)) parameters are gone, which is the intended fix.
    • a.c:11:9: style: Unused variable: x [unusedVariable] for int x __attribute__((unused)), y; is no longer reported (correct).
    • struct member 'S::b' is never used. [unusedStructMember] for struct S { int a __attribute__((unused)), b; }; is now reported (correct; base had marked b).
  • With --dump, for the unnamed parameter in void f(int __attribute__((unused)), int *q) the attribute now lands on the type token int. Base put it on the next parameter q, which was wrong. I found no check that is affected by this.

  • I found no crashes or hangs.


Automated review of 3c56ef2 by Claude. The pull request was built and tested locally.

Comment thread lib/tokenize.cpp Resolved
@dmcppcheck

Copy link
Copy Markdown
Contributor

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

Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8976/
packages: 1000
Differing warnings: 929 (+0 -929)
AI review: 50 improvement (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8976/2026-10-28_19-27-18-3c56ef2e9d4f-review.txt)
Timing: -0.1% (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8976/2026-10-28_19-27-18-3c56ef2e9d4f_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.

@mptre

mptre commented Nov 7, 2026 via email

Copy link
Copy Markdown
Contributor Author

@danmar

danmar commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator

spontanously to me the AI review about the comma operator makes sense. Don't you agree with it?

#8976 (comment)

mptre and others added 2 commits November 7, 2026 19:47
Unused parameters annotated with the unused attribute were reported as they
could be declared as pointer to const, while [[maybe_unused]] parameters were
not.
The unused attribute in declarations with several names now marks the
correct variable or struct member.
@mptre

mptre commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

spontanously to me the AI review about the comma operator makes sense. Don't you agree with it?

#8976 (comment)

Sure, fixed.

@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 new problems. Parameters marked with __attribute__((unused)) no longer get constParameterPointer. The attribute now lands on the right name in declarations with several names: local variables, struct members and enumerators. The earlier review comment asking for tests of those declarations is fixed, because the PR now has tests for both cases. I compared this PR with base on many cases and found no regressions, crashes or new false positives.

Earlier comments

# Comment By Where Status
1 No test covers the attribute in declarations with several names (locals and struct members) AI review lib/tokenize.cpp:9722 ✅ Fixed

Testing

  • I built cppcheck and testrunner from pr/ and cppcheck from base/; both built without errors.

  • ./testrunner TestOther TestUnusedVar TestTokenizer TestSymbolDatabase TestGarbage TestVarID: 1763 tests, 0 failed.

  • I ran both binaries with cppcheck -q --enable=all --inconclusive on scratch files with these cases:

    • attributed parameters before and after the name
    • an unnamed attributed parameter
    • an array parameter
    • a parameter that is dereferenced
    • cleanup(...) on a parameter
    • a function pointer parameter whose own parameter has the attribute
    • a cast (int __attribute__((unused)))v
    • an enum enum E { A __attribute__((unused)), B };
    • globals, locals and struct members with several names, mixing unused and used
    • the attribute only on the prototype and not on the definition

    Differences from base:

    • The constParameterPointer false positive for the attributed parameter is gone (k6).
    • S::m1 and S::m2 are no longer falsely reported; S::m3 is now correctly reported as unusedStructMember.
    • For locals, a and b (marked unused and used) are no longer reported; the plain c now is.

    All of these are correct.

  • When the attribute is only on the prototype, the definition still gets constParameterPointer. This was already so in base, and GCC also applies the attribute only to the parameters of the definition. A local int *a __attribute__((unused)) = 0; still gets constVariablePointer, the same as [[maybe_unused]] does today.

  • I found no crashes or hangs.


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

@danmar
danmar merged commit 9074c47 into cppcheck-opensource:main Nov 8, 2026
71 checks passed
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.

3 participants