This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.
ClosedPublic

Authored by hokein on Mar 7 2016, 5:21 AM.

Diff Detail

Event Timeline

hokein updated this revision to Diff 49951.Mar 7 2016, 5:21 AM
hokein retitled this revision from to [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace..
hokein updated this object.
hokein set the repository for this revision to rL LLVM.
hokein added a subscriber: cfe-commits.
alexfh added inline comments.Mar 7 2016, 6:14 AM
clang-tidy/misc/UnusedParametersCheck.cpp
20

There's a matcher for this: isOverride.

hokein updated this revision to Diff 49954.Mar 7 2016, 6:35 AM

Use internal isOverride method.

hokein marked an inline comment as done.Mar 7 2016, 6:36 AM
alexfh added inline comments.Mar 7 2016, 6:44 AM
clang-tidy/misc/UnusedParametersCheck.cpp
87

I meant, you can use it inside UsedByRef.

hokein added inline comments.Mar 7 2016, 8:33 AM
clang-tidy/misc/UnusedParametersCheck.cpp
87

Looks like we can't make it in UsedByRef.

The ast_matchers::match argument passed in UsedByRef is a TranslationUnitDecl type, which isn't used to match CXXMethodDecl. the code below is always true:

ast_matchers::match(
      cxxMethodDecl(isOverride()),
      *Result.Context->getTranslationUnitDecl(), *Result.Context).empty()
alexfh added inline comments.Mar 24 2016, 8:21 AM
clang-tidy/misc/UnusedParametersCheck.cpp
74

It looks like the two-parameter ast_matchers::match function would be better here (and it will allow to get rid of the decl(hasDescendant( part). Something like this:

return !ast_matchers::match(declRefExpr(...), *Result.Context).empty();
87

That's why I don't like default lambda captures: They make it totally unclear which actual parameters the lambda accepts.

Then the initial solution looks better than the current one using ast_matchers::match. Sorry for the noise.

alexfh requested changes to this revision.Mar 24 2016, 8:21 AM
alexfh edited edge metadata.
This revision now requires changes to proceed.Mar 24 2016, 8:21 AM
hokein updated this revision to Diff 52179.Mar 31 2016, 1:18 AM
hokein edited edge metadata.

Updated.

hokein marked an inline comment as done.Mar 31 2016, 1:21 AM
hokein added inline comments.
clang-tidy/misc/UnusedParametersCheck.cpp
74

Done. I also removed this lambda function.

alexfh accepted this revision.Mar 31 2016, 3:35 PM
alexfh edited edge metadata.

LG. Thank you!

This revision is now accepted and ready to land.Mar 31 2016, 3:35 PM
alexfh added inline comments.Mar 31 2016, 3:36 PM
clang-tidy/misc/UnusedParametersCheck.cpp
86

nit: Please update formatting.

hokein updated this revision to Diff 52332.Apr 1 2016, 12:36 AM
hokein edited edge metadata.

Code format.

This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.Apr 1 2016, 1:03 AM