unusedPrivateFunction: prevent redundant entries in error path - #8914
chrchr-github wants to merge 5 commits into
Conversation
| { | ||
| std::list<const Token *> toks{ tok1 }; | ||
| if (tok2) | ||
| if (tok2 && tok2 != tok1) |
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
The fix looks correct to me. As an optional nit: tok1/tok2 don't say which is the declaration and which the implementation, and the new tok2 != tok1 condition relies on knowing that. The caller passes pf->token, pf->tokenDef. Renaming the parameters to tokImpl/tokDef (here and in checkclass.h) would make the condition self-explanatory: "only add the declaration location if it differs from the implementation location".
| } | ||
|
|
||
| void CheckClassImpl::unusedPrivateFunctionError(const Token* tok1, const Token *tok2, const std::string &classname, const std::string &funcname) | ||
| void CheckClassImpl::unusedPrivateFunctionError(const Token* tokDecl, const Token *tokDef, const std::string &classname, const std::string &funcname) |
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
Thanks for the rename. One problem: the names are now the wrong way round compared with what the caller passes. The call site is unusedPrivateFunctionError(pf->token, pf->tokenDef, ...), and in symboldatabase.h:
Function::tokenDefis the "function name token in class definition" (the declaration)Function::tokenis the "function name token in implementation"
So the first parameter gets the implementation token, but it is called tokDecl. Something like tokImpl, tokDef (here and in checkclass.h) would match the caller. This only changes names, not behavior; the fix itself looks good and can't cause false positives, since it only removes a duplicate location from the error path.
3 participants
No description provided.