[clang-tidy] readability-avoid-const-params-in-decls template instantiation bugfix

[malcolm.parsons]

Summary

Bugfix for 30398. Don't warn for template instantiations

[omtcyfz]

Probably it also makes sense to reflect both lambda and template instantiation parts in documentation, since I find current wording totally confusing at the moment.

clang-tidy/readability/AvoidConstParamsInDecls.cpp

41

ast_matchers:: is redundant here.

[malcolm.parsons]

clang-tidy/readability/AvoidConstParamsInDecls.cpp

41

You'd think so, but it didn't compile.

[omtcyfz]

clang-tidy/readability/AvoidConstParamsInDecls.cpp

41

Ah, I see.

Also, please provide more info next time:

it didn't compile

doesn't give anything.

For the others: the actual problem is that llvm/tools/clang/include/clang/Basic/Specifiers.h has inline bool isTemplateInstantiation(TemplateSpecializationKind Kind) function. It might make sense to change name of the AST Matcher to something else in order to prevent collision. Thoughts, suggestions?

However, this, of course, is not in scope of the current patch.

[alexfh]

clang-tidy/readability/AvoidConstParamsInDecls.cpp

39–44

Would be helpful to expand on why we're ignoring only member function of class instantiations (and not instantiations of member or free standing functions, for example).

[malcolm.parsons]

Expand code comment

[malcolm.parsons]

clang-tidy/readability/AvoidConstParamsInDecls.cpp

39–44

AFAICT, template instantiations of member and free standing functions are definitions when the template is a definition.

[alexfh]

LG

clang-tidy/readability/AvoidConstParamsInDecls.cpp

 
 void AvoidConstParamsInDecls::registerMatchers(MatchFinder *Finder) {
   const auto ConstParamDecl =
       parmVarDecl(hasType(qualType(isConstQualified()))).bind("param");
   Finder->addMatcher(
       functionDecl(unless(isDefinition()),
                    // Lambdas are always their own definition, but they
                    // generate a non-definition FunctionDecl too. Ignore those.
-                   unless(cxxMethodDecl(ofClass(cxxRecordDecl(isLambda())))),
+                   // Class template instantiations have a non-definition
+                   // CXXMethodDecl for methods that aren't used in this
+                   // translation unit. Ignore those, as the template will have
+                   // already been checked.
+                   unless(cxxMethodDecl(ofClass(cxxRecordDecl(anyOf(
+                       isLambda(), ast_matchers::isTemplateInstantiation()))))),
                    has(typeLoc(forEach(ConstParamDecl))))
           .bind("func"),
       this);
 }
 
 // Re-lex the tokens to get precise location of last 'const'
 static llvm::Optional<Token> ConstTok(CharSourceRange Range,
                                       const MatchFinder::MatchResult &Result) {

test/clang-tidy/readability-avoid-const-params-in-decls.cpp

 void F11(const unsigned int /*v*/);
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1
 // CHECK-FIXES: void F11(unsigned int /*v*/);
 
 void F12(const bool b = true);
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'b'
 // CHECK-FIXES: void F12(bool b = true);
 
+template<class T>
+int F13(const bool b = true);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'b'
+// CHECK-FIXES: int F13(bool b = true);
+int f13 = F13<int>();
+
 struct Foo {
   Foo(const int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: parameter 'i'
   // CHECK-FIXES: Foo(int i);
 
   void operator()(const int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
   // CHECK-FIXES: void operator()(int i);
 };
 
+template <class T>
+struct FooT {
+  FooT(const int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: parameter 'i'
+  // CHECK-FIXES: FooT(int i);
+
+  void operator()(const int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+  // CHECK-FIXES: void operator()(int i);
+};
+FooT<int> f(1);
+
 // Do not match on definitions
 void NF1(const int i) {}
 void NF2(const int *const i) {}
 void NF3(int const i) {}
 void NF4(alias_type const i) {}
 void NF5(const int) {}
 void NF6(const int *const) {}
 void NF7(int, const int) {}
 void NF8(const int, const int) {}
+template <class T>
+int NF9(const int, const int) { return 0; }
+int nf9 = NF9<int>(1, 2);
+
+// Do not match on inline member functions
+struct Bar {
+  Bar(const int i) {}
+
+  void operator()(const int i) {}
+};
+
+// Do not match on inline member functions of a templated class
+template <class T>
+struct BarT {
+  BarT(const int i) {}
+
+  void operator()(const int i) {}
+};
+BarT<int> b(1);
 
 // Do not match on other stuff
 void NF(const alias_type& i);
 void NF(const int &i);
 void NF(const int *i);
 void NF(alias_const_type i);
 void NF(const alias_type&);
 void NF(const int&);