diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "../google/ExplicitConstructorCheck.h" #include "../misc/NonPrivateMemberVariablesInClassesCheck.h" #include "../misc/UnconventionalAssignOperatorCheck.h" #include "../modernize/AvoidCArraysCheck.h" @@ -52,6 +53,8 @@ "cppcoreguidelines-avoid-magic-numbers"); CheckFactories.registerCheck( "cppcoreguidelines-avoid-non-const-global-variables"); + CheckFactories.registerCheck( + "cppcoreguidelines-explicit-constructor-and-conversion"); CheckFactories.registerCheck( "cppcoreguidelines-explicit-virtual-functions"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h --- a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h +++ b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h @@ -24,12 +24,17 @@ class ExplicitConstructorCheck : public ClangTidyCheck { public: ExplicitConstructorCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), + ConstructorWhitelist(Options.get("ConstructorWhitelist", "")) {} + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const std::string ConstructorWhitelist; }; } // namespace google diff --git a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp --- a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "ExplicitConstructorCheck.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -17,11 +18,24 @@ namespace clang { namespace tidy { namespace google { +namespace { +auto hasAnyWhitelistedName(const std::string &Names) { + const std::vector NameList = + utils::options::parseStringList(Names); + return hasAnyName(std::vector(NameList.begin(), NameList.end())); +} +} // namespace + +void ExplicitConstructorCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "ConstructorWhitelist", ConstructorWhitelist); +} void ExplicitConstructorCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - cxxConstructorDecl(unless(anyOf(isImplicit(), // Compiler-generated. - isDeleted(), isInstantiated()))) + cxxConstructorDecl( + unless(anyOf(isImplicit(), // Compiler-generated. + isDeleted(), isInstantiated(), + hasAnyWhitelistedName(ConstructorWhitelist)))) .bind("ctor"), this); Finder->addMatcher( @@ -85,7 +99,7 @@ "%0 must be marked explicit to avoid unintentional implicit conversions"; if (const auto *Conversion = - Result.Nodes.getNodeAs("conversion")) { + Result.Nodes.getNodeAs("conversion")) { if (Conversion->isOutOfLine()) return; SourceLocation Loc = Conversion->getLocation(); 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 @@ -127,6 +127,11 @@ :doc:`concurrency-thread-canceltype-asynchronous ` was added. +- New alias :doc:`cppcoreguidelines-explicit-constructor-and-conversion + ` to + :doc:`google-explicit-constructor + ` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -142,6 +147,12 @@ function or assignment to ``nullptr``. Added support for pointers to ``std::unique_ptr``. +- Improved :doc:`google-explicit-constructor + ` check. + + Added an option to disable warnings for constructors contained in a + whitelist. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst @@ -0,0 +1,14 @@ +.. title:: clang-tidy - cppcoreguidelines-explicit-constructor-and-conversion +.. meta:: + :http-equiv=refresh: 5;URL=google-explicit-constructor.html + +cppcoreguidelines-explicit-constructor +====================================== + +The cppcoreguidelines-explicit-constructor-and-conversion check is an alias, +please see `google-explicit-constructor `_ +for more information. + +This check implements `C.46 `_ +and `C.164 `_ +from the CppCoreGuidelines. diff --git a/clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst b/clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst --- a/clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst @@ -54,3 +54,13 @@ See https://google.github.io/styleguide/cppguide.html#Explicit_Constructors + +Options +------- + +.. option:: ConstructorWhitelist + + Non-explicit single-argument constructors in this semicolon-separated list + will be ignored and will not trigger a warning. This option is used by this + check's `cppcoreguidelines-explicit-constructor-and-conversion `_ + alias to comply with the CppCoreGuidelines. The default list is empty. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -407,6 +407,7 @@ `cppcoreguidelines-avoid-c-arrays `_, `modernize-avoid-c-arrays `_, `cppcoreguidelines-avoid-magic-numbers `_, `readability-magic-numbers `_, `cppcoreguidelines-c-copy-assignment-signature `_, `misc-unconventional-assign-operator `_, + `cppcoreguidelines-explicit-constructor-and-conversion `_, `google-explicit-constructor `_, "Yes" `cppcoreguidelines-explicit-virtual-functions `_, `modernize-use-override `_, "Yes" `cppcoreguidelines-non-private-member-variables-in-classes `_, `misc-non-private-member-variables-in-classes `_, `fuchsia-header-anon-namespaces `_, `google-build-namespaces `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp @@ -0,0 +1,99 @@ +// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \ +// RUN: google-explicit-constructor %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: ]}' + +// RUN: %check_clang_tidy -check-suffix=WHITELISTED %s \ +// RUN: google-explicit-constructor %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: google-explicit-constructor.ConstructorWhitelist, value: "A;B"} \ +// RUN: ]}' + +struct A { + A() {} + A(int x, int y) {} + + explicit A(void *x) {} + explicit A(void *x, void *y) {} + + explicit A(const A &a) {} + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}A(const A &a) {} + // WHITELISTED: Warning disabled with ConstructorWhitelist=A + + A(int x1); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}explicit A(int x1); + // WHITELISTED: Warning disabled with ConstructorWhitelist=A + + A(double x2, double y = 3.14) {} + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}explicit A(double x2, double y = 3.14) {} + // WHITELISTED: Warning disabled with ConstructorWhitelist=A + + template + A(T &&...args); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument + // CHECK-FIXES-DEFAULT: {{^ }}explicit A(T &&...args); + // WHITELISTED: Warning disabled with ConstructorWhitelist=A +}; + +inline A::A(int x1) {} + +struct B { + B() {} + B(int x, int y) {} + + explicit B(void *x) {} + explicit B(void *x, void *y) {} + + explicit B(const B &b) {} + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}B(const B &b) {} + // WHITELISTED: Warning disabled with ConstructorWhitelist=B + + B(int x1); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}explicit B(int x1); + // WHITELISTED: Warning disabled with ConstructorWhitelist=B + + B(double x2, double y = 3.14) {} + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}explicit B(double x2, double y = 3.14) {} + // WHITELISTED: Warning disabled with ConstructorWhitelist=B + + template + B(T &&...args); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument + // CHECK-FIXES-DEFAULT: {{^ }}explicit B(T &&...args); + // WHITELISTED: Warning disabled with ConstructorWhitelist=B +}; + +struct C { + C() {} + C(int x, int y) {} + + explicit C(void *x) {} + explicit C(void *x, void *y) {} + + explicit C(const C &c) {} + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor] + // CHECK-MESSAGES-WHITELISTED: :[[@LINE-2]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor] + // CHECK-FIXES: {{^ }}C(const C &c) {} + + C(int x1); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-WHITELISTED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit C(int x1); + + C(double x2, double y = 3.14) {} + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-WHITELISTED: :[[@LINE-2]]:3: warning: constructors that are callable with a single argument must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit C(double x2, double y = 3.14) {} + + template + C(T &&...args); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument + // CHECK-MESSAGES-WHITELISTED: :[[@LINE-2]]:3: warning: constructors that are callable with a single argument + // CHECK-FIXES: {{^ }}explicit C(T &&...args); +};