[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