[clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto

Summary: Adds an option called `AddConstToQualified` to readability-qualified-auto to toggle adding const to the auto typed pointers and references. By default its enabled but in the LLVM module its disabled.

Reviewers: aaron.ballman, alexfh, JonasToth, hokein, sammccall

Reviewed By: aaron.ballman

Subscribers: Quuxplusone, merge_guards_bot, lebedev.ri, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D73548

Author: njames93

clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

@@ -36,6 +36,12 @@ class LLVMModule : public ClangTidyModule {
         "llvm-qualified-auto");
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
   }
+
+  ClangTidyOptions getModuleOptions() override {
+    ClangTidyOptions Options;
+    Options.CheckOptions["llvm-qualified-auto.AddConstToQualified"] = "0";
+    return Options;
+  }
 };
 
 // Register the LLVMTidyModule using this statically initialized variable.

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

@@ -102,6 +102,10 @@ bool isAutoPointerConst(QualType QType) {
 
 } // namespace
 
+void QualifiedAutoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AddConstToQualified", AddConstToQualified);
+}
+
 void QualifiedAutoCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
     return; // Auto deduction not used in 'C or C++03 and earlier', so don't
@@ -142,6 +146,8 @@ void QualifiedAutoCheck::registerMatchers(MatchFinder *Finder) {
                           hasAnyTemplateArgument(IsBoundToType))))),
           "auto"),
       this);
+  if (!AddConstToQualified)
+    return;
   Finder->addMatcher(ExplicitSingleVarDecl(
                          hasType(pointerType(pointee(autoType()))), "auto_ptr"),
                      this);
@@ -177,11 +183,9 @@ void QualifiedAutoCheck::check(const MatchFinder::MatchResult &Result) {
     bool IsLocalVolatile = Var->getType().isLocalVolatileQualified();
     bool IsLocalRestrict = Var->getType().isLocalRestrictQualified();
 
-    if (CheckQualifier(IsLocalConst, Qualifier::Const))
-      return;
-    if (CheckQualifier(IsLocalVolatile, Qualifier::Volatile))
-      return;
-    if (CheckQualifier(IsLocalRestrict, Qualifier::Restrict))
+    if (CheckQualifier(IsLocalConst, Qualifier::Const) ||
+        CheckQualifier(IsLocalVolatile, Qualifier::Volatile) ||
+        CheckQualifier(IsLocalRestrict, Qualifier::Restrict))
       return;
 
     // Check for bridging the gap between the asterisk and name.
@@ -214,8 +218,7 @@ void QualifiedAutoCheck::check(const MatchFinder::MatchResult &Result) {
         << (IsLocalRestrict ? "__restrict " : "") << Var->getName() << ReplStr;
 
     for (SourceRange &Range : RemoveQualifiersRange) {
-      Diag << FixItHint::CreateRemoval(
-          CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()));
+      Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(Range));
     }
 
     Diag << FixItHint::CreateReplacement(FixItRange, ReplStr);
@@ -247,22 +250,17 @@ void QualifiedAutoCheck::check(const MatchFinder::MatchResult &Result) {
         return;
     }
 
-    CharSourceRange FixItRange;
     if (llvm::Optional<SourceRange> TypeSpec =
             getTypeSpecifierLocation(Var, Result)) {
-      FixItRange = CharSourceRange::getCharRange(*TypeSpec);
-      if (FixItRange.isInvalid())
+      if (TypeSpec->isInvalid() || TypeSpec->getBegin().isMacroID() ||
+          TypeSpec->getEnd().isMacroID())
         return;
-    } else
-      return;
-
-    DiagnosticBuilder Diag =
-        diag(FixItRange.getBegin(),
-             "'auto *%0%1%2' can be declared as 'const auto *%0%1%2'")
-        << (Var->getType().isLocalConstQualified() ? "const " : "")
-        << (Var->getType().isLocalVolatileQualified() ? "volatile " : "")
-        << Var->getName();
-    Diag << FixItHint::CreateReplacement(FixItRange, "const auto *");
+      SourceLocation InsertPos = TypeSpec->getBegin();
+      diag(InsertPos, "'auto *%0%1%2' can be declared as 'const auto *%0%1%2'")
+          << (Var->getType().isLocalConstQualified() ? "const " : "")
+          << (Var->getType().isLocalVolatileQualified() ? "volatile " : "")
+          << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const ");
+    }
     return;
   }
   if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("auto_ref")) {
@@ -272,20 +270,15 @@ void QualifiedAutoCheck::check(const MatchFinder::MatchResult &Result) {
       // Const isnt wrapped in the auto type, so must be declared explicitly.
       return;
 
-    CharSourceRange FixItRange;
     if (llvm::Optional<SourceRange> TypeSpec =
             getTypeSpecifierLocation(Var, Result)) {
-      FixItRange = CharSourceRange::getCharRange(*TypeSpec);
-      if (FixItRange.isInvalid())
+      if (TypeSpec->isInvalid() || TypeSpec->getBegin().isMacroID() ||
+          TypeSpec->getEnd().isMacroID())
         return;
-    } else
-      return;
-
-    DiagnosticBuilder Diag =
-        diag(FixItRange.getBegin(),
-             "'auto &%0' can be declared as 'const auto &%0'")
-        << Var->getName();
-    Diag << FixItHint::CreateReplacement(FixItRange, "const auto &");
+      SourceLocation InsertPos = TypeSpec->getBegin();
+      diag(InsertPos, "'auto &%0' can be declared as 'const auto &%0'")
+          << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const ");
+    }
     return;
   }
 }

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

@@ -24,9 +24,14 @@ namespace readability {
 class QualifiedAutoCheck : public ClangTidyCheck {
 public:
   QualifiedAutoCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        AddConstToQualified(Options.get("AddConstToQualified", true)) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool AddConstToQualified;
 };
 
 } // namespace readability

clang-tools-extra/docs/ReleaseNotes.rst

@@ -104,6 +104,11 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Improved :doc:`readability-qualified-auto
+  <clang-tidy/checks/readability-qualified-about>` check now supports a 
+  `AddConstToQualified` to enable adding ``const`` qualifiers to variables
+  typed with ``auto *`` and ``auto &``.
+
 - Improved :doc:`readability-redundant-string-init
   <clang-tidy/checks/readability-redundant-string-init>` check now supports a
   `StringNames` option enabling its application to custom string classes. The 

clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst

@@ -3,23 +3,16 @@
 readability-qualified-auto
 ==========================
 
-Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
-to pointers and ``const`` pointers.
+Adds pointer qualifications to ``auto``-typed variables that are deduced to 
+pointers.
 
-`LLVM Coding Standards <https://llvm.org/docs/CodingStandards.html>`_ advises to
-make it obvious if a ``auto`` typed variable is a pointer, constant pointer or 
-constant reference. This check will transform ``auto`` to ``auto *`` when the 
-type is deduced to be a pointer, as well as adding ``const`` when applicable to
-``auto`` pointers or references
+`LLVM Coding Standards <https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto>`_
+advises to make it obvious if a ``auto`` typed variable is a pointer. This 
+check will transform ``auto`` to ``auto *`` when the type is deduced to be a
+pointer.
 
 .. code-block:: c++
 
-  for (auto &Data : MutatableContainer) {
-    change(Data);
-  }
-  for (auto &Data : ConstantContainer) {
-    observe(Data);
-  }
   for (auto Data : MutatablePtrContainer) {
     change(*Data);
   }
@@ -31,34 +24,61 @@ Would be transformed into:
 
 .. code-block:: c++
 
-  for (auto &Data : MutatableContainer) {
-    change(Data);
-  }
-  for (const auto &Data : ConstantContainer) {
-    observe(Data);
-  }
   for (auto *Data : MutatablePtrContainer) {
     change(*Data);
   }
   for (const auto *Data : ConstantPtrContainer) {
     observe(*Data);
   }
 
-Note const volatile qualified types will retain their const and volatile qualifiers.
+Note ``const`` ``volatile`` qualified types will retain their ``const`` and 
+``volatile`` qualifiers. Pointers to pointers will not be fully qualified.
 
 .. code-block:: c++
 
-  const auto Foo = cast<int *>(Baz1);
-  const auto Bar = cast<const int *>(Baz2);
-  volatile auto FooBar = cast<int*>(Baz3);
+   const auto Foo = cast<int *>(Baz1);
+   const auto Bar = cast<const int *>(Baz2);
+   volatile auto FooBar = cast<int *>(Baz3);
+   auto BarFoo = cast<int **>(Baz4);
 
 Would be transformed into:
 
 .. code-block:: c++
 
-  auto *const Foo = cast<int *>(Baz1);
-  const auto *const Bar = cast<const int *>(Baz2);
-  auto *volatile FooBar = cast<int*>(Baz3);
+   auto *const Foo = cast<int *>(Baz1);
+   const auto *const Bar = cast<const int *>(Baz2);
+   auto *volatile FooBar = cast<int *>(Baz3);
+   auto *BarFoo = cast<int **>(Baz4);
+
+Options
+-------
+
+.. option:: AddConstToQualified
+   
+   When set to `1` the check will add const qualifiers variables defined as
+   ``auto *`` or ``auto &`` when applicable.
+   Default value is '1'.
+
+.. code-block:: c++
+
+   auto Foo1 = cast<const int *>(Bar1);
+   auto *Foo2 = cast<const int *>(Bar2);
+   auto &Foo3 = cast<const int &>(Bar3);
+
+   If AddConstToQualified is set to `0`,  it will be transformed into:
+
+.. code-block:: c++
+
+   const auto *Foo1 = cast<const int *>(Bar1);
+   auto *Foo2 = cast<const int *>(Bar2);
+   auto &Foo3 = cast<const int &>(Bar3);
+
+   Otherwise it will be transformed into:
+
+.. code-block:: c++
+
+   const auto *Foo1 = cast<const int *>(Bar1);
+   const auto *Foo2 = cast<const int *>(Bar2);
+   const auto &Foo3 = cast<const int &>(Bar3);
 
-This check helps to enforce this `LLVM Coding Standards recommendation
-<https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto>`_.
+   Note in the LLVM alias, the default value is `0`.

clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp

@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s llvm-qualified-auto %t
+
+// This check just ensures by default the llvm alias doesn't add const
+// qualifiers to decls, so no need to copy the entire test file from
+// readability-qualified-auto.
+
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+  auto NakedPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+  // CHECK-FIXES: {{^}}  auto *NakedPtr = getIntPtr();
+  auto NakedConstPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedConstPtr' can be declared as 'const auto *NakedConstPtr'
+  // CHECK-FIXES: {{^}}  const auto *NakedConstPtr = getCIntPtr();
+  auto *Ptr = getIntPtr();
+  auto *ConstPtr = getCIntPtr();
+  auto &NakedRef = *getIntPtr();
+  auto &NakedConstRef = *getCIntPtr();
+}