Skip to content

Fix #14471: Improve check: shadowVariable for function argument#8303

Open
ludviggunne wants to merge 8 commits intocppcheck-opensource:mainfrom
ludviggunne:14471
Open

Fix #14471: Improve check: shadowVariable for function argument#8303
ludviggunne wants to merge 8 commits intocppcheck-opensource:mainfrom
ludviggunne:14471

Conversation

@ludviggunne
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread lib/checkother.cpp Fixed
Comment thread lib/checkother.h Fixed
@firewave
Copy link
Copy Markdown
Collaborator

This should allow us to enable -Wshadow for GCC and -Wshadow-field for Clang.

Comment thread lib/symboldatabase.cpp Fixed
@ludviggunne ludviggunne force-pushed the 14471 branch 2 times, most recently from 55fc5b2 to a005907 Compare March 30, 2026 08:29
@firewave
Copy link
Copy Markdown
Collaborator

Since this now reports shadowed functions the error ID is misleading. We probably need a new ID.

@ludviggunne
Copy link
Copy Markdown
Collaborator Author

Since this now reports shadowed functions the error ID is misleading. We probably need a new ID.

The error ID does differ actually, e.g. shadowFunction, shadowArgument etc.

@firewave
Copy link
Copy Markdown
Collaborator

The error ID does differ actually, e.g. shadowFunction, shadowArgument etc.

Ah, sorry. I just went by the title of the PR and did not look at the code.

Comment thread test/testother.cpp
@ludviggunne ludviggunne force-pushed the 14471 branch 3 times, most recently from fc03706 to eeb3eec Compare March 31, 2026 09:17
danmar
danmar previously approved these changes Mar 31, 2026
Copy link
Copy Markdown
Collaborator

@danmar danmar left a comment

Choose a reason for hiding this comment

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

lgtm

Comment thread lib/token.cpp Outdated
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 1, 2026

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Apr 1, 2026

Related premium issue:
https://cppcheck.atlassian.net/browse/ADDON-558

Comment thread .selfcheck_suppressions
Comment on lines +44 to +45
shadowFunction:externals/simplecpp/simplecpp.cpp
shadowFunction:externals/simplecpp/simplecpp.h
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would be great if you could publish an upstream PR which fixes these so we can drop the suppressions on the next bump.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I checked and we have -Wshadow-field from Clang enabled in simplecpp. And -Wshadow from GCC only reports shadowing in constructors (as expected), so I am curious about these.

Comment thread lib/tokenlist.h
* @param tok A '(' or ')' token in a possible function head
* @param endsWith string after function head
* @param end string after function head
* @return token matching with endsWith if syntax seems to be a function head else nullptr
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rename endsWith here in the @return also

Comment thread lib/tokenlist.h
/**
* is token pointing at function head?
* @param tok A '(' or ')' token in a possible function head
* @param endsWith string after function head
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm would suffix be better

Comment thread lib/symboldatabase.h
void setArrayDimensionsUsingValueFlow();

void clangSetVariables(const std::vector<const Variable *> &variableList);
void clangSetVariables(const std::vector<const Variable *> &list);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe we can rename it to "vars" instead.

Comment thread gui/mainwindow.h
* @param recheckFiles files to recheck, empty => check all files
* @param checkLibrary Flag to indicate if the library should be checked.
* @param checkConfiguration Flag to indicate if the configuration should be checked.
* @param doCheckLibrary Flag to indicate if the library should be checked.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

to me the "do" prefix makes it sound more like a function. how about renaming the parameters to checkConfig and checkLib instead.

Comment thread lib/checkcondition.cpp
}

void CheckCondition::compareValueOutOfTypeRangeError(const Token *comparison, const std::string &type, MathLib::bigint value, bool result)
void CheckCondition::compareValueOutOfTypeRangeError(const Token *comp, const std::string &type, MathLib::bigint value, bool result)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I suggest comparisonTok

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.

5 participants