diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -36,6 +36,12 @@ "llvm-qualified-auto"); CheckFactories.registerCheck("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. diff --git a/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h b/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h --- a/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h +++ b/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h @@ -24,9 +24,14 @@ 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 diff --git a/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp b/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp @@ -102,6 +102,10 @@ } // 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 @@ hasAnyTemplateArgument(IsBoundToType))))), "auto"), this); + if (!AddConstToQualified) + return; Finder->addMatcher(ExplicitSingleVarDecl( hasType(pointerType(pointee(autoType()))), "auto_ptr"), this); @@ -177,11 +183,9 @@ 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 @@ << (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 @@ return; } - CharSourceRange FixItRange; if (llvm::Optional 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("auto_ref")) { @@ -272,20 +270,15 @@ // Const isnt wrapped in the auto type, so must be declared explicitly. return; - CharSourceRange FixItRange; if (llvm::Optional 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; } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -104,6 +104,11 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`readability-qualified-auto + ` check now supports a + `AddConstToQualified` to enable adding ``const`` qualifiers to variables + typed with ``auto *`` and ``auto &``. + - Improved :doc:`readability-redundant-string-init ` check now supports a `StringNames` option enabling its application to custom string classes. The diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst --- a/clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst +++ b/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 `_ 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 `_ +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,12 +24,6 @@ .. code-block:: c++ - for (auto &Data : MutatableContainer) { - change(Data); - } - for (const auto &Data : ConstantContainer) { - observe(Data); - } for (auto *Data : MutatablePtrContainer) { change(*Data); } @@ -44,21 +31,54 @@ 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(Baz1); - const auto Bar = cast(Baz2); - volatile auto FooBar = cast(Baz3); + const auto Foo = cast(Baz1); + const auto Bar = cast(Baz2); + volatile auto FooBar = cast(Baz3); + auto BarFoo = cast(Baz4); Would be transformed into: .. code-block:: c++ - auto *const Foo = cast(Baz1); - const auto *const Bar = cast(Baz2); - auto *volatile FooBar = cast(Baz3); + auto *const Foo = cast(Baz1); + const auto *const Bar = cast(Baz2); + auto *volatile FooBar = cast(Baz3); + auto *BarFoo = cast(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(Bar1); + auto *Foo2 = cast(Bar2); + auto &Foo3 = cast(Bar3); + + If AddConstToQualified is set to `0`, it will be transformed into: + +.. code-block:: c++ + + const auto *Foo1 = cast(Bar1); + auto *Foo2 = cast(Bar2); + auto &Foo3 = cast(Bar3); + + Otherwise it will be transformed into: + +.. code-block:: c++ + + const auto *Foo1 = cast(Bar1); + const auto *Foo2 = cast(Bar2); + const auto &Foo3 = cast(Bar3); -This check helps to enforce this `LLVM Coding Standards recommendation -`_. + Note in the LLVM alias, the default value is `0`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp new file mode 100644 --- /dev/null +++ b/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(); +}