[clang-tidy] Add an IgnoreMacros option to readability-avoid-const-params-in-decls

[thomasetter]

Summary

readability-avoid-const-params-in-decls.IgnoreMacros is enabled by default to be consistent with most other checks.

[Eugene.Zelenko]

Please mention changes in Release Notes.

[njames93]

Please can you upload your diff with full context(or use arcanist which does it for you).

[thomasetter]

In D130130#3666111, @njames93 wrote:

Please can you upload your diff with full context(or use arcanist which does it for you).

Sorry about that, should be fixed now

[thomasetter]

Added a description to the release notes

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

99

Please move it to Changes in existing checks section.

[thomasetter]

Is there anything else you would like me to change?

[thomasetter]

Friendly ping

[ymandel]

clang-tools-extra/docs/ReleaseNotes.rst

119–122

[Eugene.Zelenko]

Could you please rebase from main and upload patch here before commit?

[thomasetter]

rebased

[Eugene.Zelenko]

Thank you! Just wanted to ensure alphabetical order in Changes in existing checks :-) Fine to merge.

[thomasetter]

Applied review changes

[thomasetter]

Thanks for the review!

[ymandel]

Note: fixed typo in doc in followup commit 40f5c634bc991510427e274ba49d4c56f29ef593.

clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h

 namespace tidy {
 namespace readability {
 
 // Detect function declarations that have const value parameters and discourage
 // them.
 class AvoidConstParamsInDecls : public ClangTidyCheck {
 public:
   AvoidConstParamsInDecls(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
     return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_CONST_PARAMS_IN_DECLS_H

clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp

 
 SourceRange getTypeRange(const ParmVarDecl &Param) {
   return SourceRange(Param.getBeginLoc(),
                      Param.getLocation().getLocWithOffset(-1));
 }
 
 } // namespace
 
+void AvoidConstParamsInDecls::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void AvoidConstParamsInDecls::registerMatchers(MatchFinder *Finder) {
   const auto ConstParamDecl =
       parmVarDecl(hasType(qualType(isConstQualified()))).bind("param");
   Finder->addMatcher(
       functionDecl(unless(isDefinition()),
                    has(typeLoc(forEach(ConstParamDecl))))
           .bind("func"),
       this);
 }
 
 void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
   const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
   const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
 
   if (!Param->getType().isLocalConstQualified())
     return;
 
+  if (IgnoreMacros &&
+      (Param->getBeginLoc().isMacroID() || Param->getEndLoc().isMacroID())) {
+    // Suppress the check if macros are involved.
+    return;
+  }
+
   auto Diag = diag(Param->getBeginLoc(),
                    "parameter %0 is const-qualified in the function "
                    "declaration; const-qualification of parameters only has an "
                    "effect in function definitions");
   if (Param->getName().empty()) {
     for (unsigned int I = 0; I < Func->getNumParams(); ++I) {
       if (Param == Func->getParamDecl(I)) {
         Diag << (I + 1);

clang-tools-extra/docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 New checks
 ^^^^^^^^^^
 
 - New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
   <clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members>` check.
 
   Warns when a struct or class uses const or reference (lvalue or rvalue) data members.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-msc54-cpp
   <clang-tidy/checks/cert/msc54-cpp>` to
   :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone/signal-handler>` was added.
 
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Fixed a false positive in :doc:`bugprone-assignment-in-if-condition
   <clang-tidy/checks/bugprone/assignment-in-if-condition>` check when there
   was an assignement in a lambda found in the condition of an ``if``.
 
 - Improved :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone/signal-handler>` check. Partial
   support for C++14 signal handler rules was added. Bug report generation was
   improved.
 
 - Fixed a false positive in :doc:`cppcoreguidelines-pro-type-member-init
   <clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` when warnings
   would be emitted for uninitialized members of an anonymous union despite

 - Improved :doc:`modernize-use-equals-default <clang-tidy/checks/modernize/use-equals-default>`
   check.
 
   The check now skips unions/union-like classes since in this case a default constructor
   with empty body is not equivalent to the explicitly defaulted one. The check also skips
   copy assignment operators with nonstandard return types. The check is restricted to
   c++11-or-later.
 
+- Change the default behavior of :doc:`readability-avoid-const-params-in-decls
+  <clang-tidy/checks/readability-avoid-const-params-in-decls>` to not
+  warn about `const` value parameters of declarations inside macros.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst

 they should not be put there.
 
 Examples:
 
 .. code-block:: c++
 
   void f(const string);   // Bad: const is top level.
   void f(const string&);  // Good: const is not top level.
+
+Options
+-------
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.

clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp

+// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-avoid-const-params-in-decls.IgnoreMacros, value: false}]}"
+
+// Regression tests involving macros
+#define CONCAT(a, b) a##b
+void ConstNotVisible(CONCAT(cons, t) int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+    void member_function(const int i);
+);
+// CHECK-MESSAGES: :[[@LINE-2]]:26: warning: parameter 'i'
+// CHECK-FIXES: void member_function(int i);

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

 void NF(const int *i);
 void NF(alias_const_type i);
 void NF(const alias_type&);
 void NF(const int&);
 void NF(const int*);
 void NF(const int* const*);
 void NF(alias_const_type);
 
-// Regression test for when the 'const' token is not in the code.
+// Regression tests involving macros, which are ignored by default.
 #define CONCAT(a, b) a##b
 void ConstNotVisible(CONCAT(cons, t) int i);
-// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
-// We warn, but we can't give a fix
-// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+    void member_function(const int i);
+);
 
 // Regression test. We should not warn (or crash) on lambda expressions
 auto lambda_with_name = [](const int n) {};
 auto lambda_without_name = [](const int) {};