clang-tidy: add IgnoreMacros option to readability-inconsistent-declaration-parameter-name

[vmiklos]

Summary

And also enable it by default to be consistent with e.g. modernize-use-using.

This helps e.g. when running this check on client code where the macro is
provided by the system, so there is no easy way to modify it.

[lebedev.ri]

And also enable it by default to be consistent with e.g. modernize-use-using.

That changes the defaults though. I thought clang-tidy *tried* to produce the same results
on different clang-tidy versions with the same .clang-tidy config? Or is there no such guarantees?

[vmiklos]

In D41716#967237, @lebedev.ri wrote:

That changes the defaults though. I thought clang-tidy *tried* to produce the same results
on different clang-tidy versions with the same .clang-tidy config? Or is there no such guarantees?

Hmm, I see this as a trade-off between backwards compatibility and consistency. Here I went for consistency, so that all of readability-inconsistent-declaration-parameter-name, modernize-use-default-member-init and modernize-use-using default to IgnoreMacros=1. But I don't have a too strong opinion on that, can change the default to 0 if really wanted,

[alexfh]

In D41716#967247, @vmiklos wrote:

In D41716#967237, @lebedev.ri wrote:

That changes the defaults though. I thought clang-tidy *tried* to produce the same results
on different clang-tidy versions with the same .clang-tidy config? Or is there no such guarantees?

Hmm, I see this as a trade-off between backwards compatibility and consistency. Here I went for consistency, so that all of readability-inconsistent-declaration-parameter-name, modernize-use-default-member-init and modernize-use-using default to IgnoreMacros=1. But I don't have a too strong opinion on that, can change the default to 0 if really wanted,

I'd personally prefer consistency in this case, especially because the option can be specified globally for all checks.

[alexfh]

Thus, looks good. Thanks!

clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h

 ///
 /// For detailed documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.html
 ///
 class InconsistentDeclarationParameterNameCheck : public ClangTidyCheck {
 public:
   InconsistentDeclarationParameterNameCheck(StringRef Name,
                                             ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   void markRedeclarationsAsVisited(const FunctionDecl *FunctionDeclaration);
 
   llvm::DenseSet<const FunctionDecl *> VisitedDeclarations;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_INCONSISTENT_DECLARATION_PARAMETER_NAME_H

clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp

for (const InconsistentDeclarationInfo &InconsistentDeclaration :
     formatDifferingParamsDiagnostic(
         Check, InconsistentDeclaration.DeclarationLocation,
         ParameterSourceDescription, InconsistentDeclaration.DifferingParams);
   }
 }
 
 } // anonymous namespace
 
+void InconsistentDeclarationParameterNameCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void InconsistentDeclarationParameterNameCheck::registerMatchers(
     MatchFinder *Finder) {
   Finder->addMatcher(functionDecl(unless(isImplicit()), hasOtherDeclarations())
                          .bind("functionDecl"),
                      this);
 }
 
 void InconsistentDeclarationParameterNameCheck::check(
InconsistentDeclarationsContainer InconsistentDeclarations =
                                   ParameterSourceDeclaration,
                                   *Result.SourceManager);
   if (InconsistentDeclarations.empty()) {
     // Avoid unnecessary further visits.
     markRedeclarationsAsVisited(OriginalDeclaration);
     return;
   }
 
+  SourceLocation StartLoc = OriginalDeclaration->getLocStart();
+  if (StartLoc.isMacroID() && IgnoreMacros) {
+    markRedeclarationsAsVisited(OriginalDeclaration);
+    return;
+  }
+
   if (OriginalDeclaration->getTemplatedKind() ==
       FunctionDecl::TK_FunctionTemplateSpecialization) {
     formatDiagnostics(this, ParameterSourceDeclaration, OriginalDeclaration,
                       InconsistentDeclarations,
                       "function template specialization",
                       "primary template declaration");
   } else if (ParameterSourceDeclaration->isThisDeclarationADefinition()) {
     formatDiagnostics(this, ParameterSourceDeclaration, OriginalDeclaration,

docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst

 .. code-block:: c++
 
   void foo(int a); // warning and fix-it hint (replace "a" to "b")
   int foo(int b) { return b + 2; } // definition with use of "b"
 
 In the case of multiple redeclarations or function template specializations,
 a warning is issued for every redeclaration or specialization inconsistent with
 the definition or the first declaration seen in a translation unit.
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about names declared inside macros.

test/clang-tidy/readability-inconsistent-declaration-parameter-name-macros.cpp

+// RUN: %check_clang_tidy %s readability-inconsistent-declaration-parameter-name %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-inconsistent-declaration-parameter-name.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+#define MACRO() \
+  void f(int x);
+
+struct S {
+  MACRO();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'S::f' has a definition with different parameter names
+};
+
+void S::f(int y) {
+}
+
+//////////////////////////////////////////////////////
+
+#define DECLARE_FUNCTION_WITH_PARAM_NAME(function_name, param_name) \
+  void function_name(int param_name)
+
+// CHECK-MESSAGES: :[[@LINE+1]]:34: warning: function 'macroFunction' has 1 other declaration with different parameter names [readability-inconsistent-declaration-parameter-name]
+DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, a);
+// CHECK-MESSAGES: :[[@LINE+2]]:34: note: the 1st inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE+1]]:34: note: differing parameters are named here: ('b'), in the other declaration: ('a')
+DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, b);

test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp

 template<>
 // CHECK-MESSAGES: :[[@LINE+3]]:13: warning: function template specialization 'Class::memberFunctionTemplateWithSpecializations<float>' has a primary template
 // CHECK-MESSAGES: :[[@LINE-18]]:8: note: the primary template declaration seen here
 // CHECK-MESSAGES: :[[@LINE+1]]:13: note: differing parameters are named here: ('c'), in primary template declaration: ('a')
 void Class::memberFunctionTemplateWithSpecializations(float c) { c; }
 
 //////////////////////////////////////////////////////
 
-#define DECLARE_FUNCTION_WITH_PARAM_NAME(function_name, param_name) \
-  void function_name(int param_name)
+// This resulted in a warning by default.
+#define MACRO() \
+  void f(int x);
 
-// CHECK-MESSAGES: :[[@LINE+1]]:34: warning: function 'macroFunction' has 1 other declaration with different parameter names [readability-inconsistent-declaration-parameter-name]
-DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, a);
-// CHECK-MESSAGES: :[[@LINE+2]]:34: note: the 1st inconsistent declaration seen here
-// CHECK-MESSAGES: :[[@LINE+1]]:34: note: differing parameters are named here: ('b'), in the other declaration: ('a')
-DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, b);
+struct S {
+  MACRO();
+};
+
+void S::f(int y)
+{
+}