[PATCH] D17926: [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 24 08:21:48 PDT 2016
alexfh added inline comments.
================
Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:67
@@ -66,3 +66,3 @@
auto UsedByRef = [&] {
return !ast_matchers::match(
decl(hasDescendant(
----------------
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();
================
Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:80
@@ +79,3 @@
+ UsedByRef() ||
+ !ast_matchers::match(cxxMethodDecl(isOverride()), *Function,
+ *Result.Context)
----------------
hokein wrote:
> alexfh wrote:
> > I meant, you can use it inside `UsedByRef`.
> 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()
> ```
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.
http://reviews.llvm.org/D17926
More information about the cfe-commits
mailing list