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

[hokein]

Summary

Fixes PR26740.

[alexfh]

clang-tidy/misc/UnusedParametersCheck.cpp

20 ↗

(On Diff #49951)

There's a matcher for this: isOverride.

[hokein]

Use internal isOverride method.

[alexfh]

clang-tidy/misc/UnusedParametersCheck.cpp

80 ↗

(On Diff #49954)

I meant, you can use it inside UsedByRef.

[hokein]

clang-tidy/misc/UnusedParametersCheck.cpp

80 ↗

(On Diff #49954)

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]

clang-tidy/misc/UnusedParametersCheck.cpp

67 ↗

(On Diff #49954)

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();

80 ↗

(On Diff #49954)

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.

[hokein]

Updated.

[hokein]

clang-tidy/misc/UnusedParametersCheck.cpp

74 ↗

(On Diff #49951)

Done. I also removed this lambda function.

[alexfh]

LG. Thank you!

[alexfh]

clang-tidy/misc/UnusedParametersCheck.cpp

80 ↗

(On Diff #52179)

nit: Please update formatting.

[hokein]

Code format.

clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp

 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
+namespace {
+bool isOverrideMethod(const FunctionDecl *Function) {
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(Function))
+    return MD->size_overridden_methods() > 0 || MD->hasAttr<OverrideAttr>();
+  return false;
+}
+} // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl().bind("function"), this);
 }
 
 template <typename T>
 static CharSourceRange removeNode(const MatchFinder::MatchResult &Result,
                                   const T *PrevNode, const T *Node,

 
 void UnusedParametersCheck::warnOnUnusedParameter(
     const MatchFinder::MatchResult &Result, const FunctionDecl *Function,
     unsigned ParamIndex) {
   const auto *Param = Function->getParamDecl(ParamIndex);
   auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused")
                 << Param->getName();
 
-  auto UsedByRef = [&] {
-    return !ast_matchers::match(
-                decl(hasDescendant(
-                    declRefExpr(to(equalsNode(Function)),
-                                unless(hasAncestor(
-                                    callExpr(callee(equalsNode(Function)))))))),
-                *Result.Context->getTranslationUnitDecl(), *Result.Context)
-                .empty();
-  };
+  auto DeclRefExpr =
+                  unless(hasAncestor(callExpr(callee(equalsNode(Function))))));
 
   // Comment out parameter name for non-local functions.
   if (Function->isExternallyVisible() ||
       !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-      UsedByRef()) {
+      !ast_matchers::match(DeclRefExpr, *Result.Context).empty() ||
+      isOverrideMethod(Function)) {
     SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
     // Note: We always add a space before the '/*' to not accidentally create a
     // '*/*' for pointer types, which doesn't start a comment. clang-format will
     // clean this up afterwards.
     MyDiag << FixItHint::CreateReplacement(
         RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
     return;
   }

clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp

void someMoreCallSites() {
   c.f(1);
 // CHECK-FIXES: c.f();
   c.g(1);
 // CHECK-FIXES: c.g();
 
   useFunction(&C::h);
 }
 
+class Base {
+  virtual void f(int i);
+};
+
+class Derived : public Base {
+  void f(int i) override {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void f(int  /*i*/) override {}
+};
+
 } // end namespace
 
 template <typename T> void someFunctionTemplate(T b, T e) { (void)b; (void)e; }
 
 template <typename T> void someFunctionTemplateOneUnusedParam(T b, T e) { (void)e; }
 // CHECK-MESSAGES: :[[@LINE-1]]:65: warning
 // CHECK-FIXES: {{^}}template <typename T> void someFunctionTemplateOneUnusedParam(T  /*b*/, T e) { (void)e; }