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 @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_EXPLICITCONSTRUCTORCHECK_H #include "../ClangTidyCheck.h" +#include "../utils/OptionsUtils.h" namespace clang { namespace tidy { @@ -24,12 +25,21 @@ class ExplicitConstructorCheck : public ClangTidyCheck { public: ExplicitConstructorCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), + IgnoredConstructors(utils::options::parseStringList( + Options.get("IgnoredConstructors", ""))), + IgnoredConversionOperators(utils::options::parseStringList( + Options.get("IgnoredConversionOperators", ""))) {} + 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::vector IgnoredConstructors; + const std::vector IgnoredConversionOperators; }; } // 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,17 +18,45 @@ namespace clang { namespace tidy { namespace google { +namespace { +auto isIgnoredCtor(const std::vector &Names) { + return hasAnyName(std::vector(Names.begin(), Names.end())); +} + +AST_MATCHER_P(CXXConversionDecl, isIgnoredConversionOperator, + std::vector, IgnoredConversionOps) { + std::string FullOperatorName = + Node.getParent()->getNameAsString().append("::").append( + Node.getNameAsString()); + + return llvm::any_of(IgnoredConversionOps, + [FullOperatorName](std::string NameInOptions) { + return NameInOptions == FullOperatorName; + }); +} +} // namespace + +void ExplicitConstructorCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoredConstructors", + utils::options::serializeStringList(IgnoredConstructors)); + Options.store( + Opts, "IgnoredConversionOperators", + utils::options::serializeStringList(IgnoredConversionOperators)); +} void ExplicitConstructorCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( cxxConstructorDecl(unless(anyOf(isImplicit(), // Compiler-generated. - isDeleted(), isInstantiated()))) + isDeleted(), isInstantiated(), + isIgnoredCtor(IgnoredConstructors)))) .bind("ctor"), this); Finder->addMatcher( cxxConversionDecl(unless(anyOf(isExplicit(), // Already marked explicit. isImplicit(), // Compiler-generated. - isDeleted(), isInstantiated()))) + isDeleted(), isInstantiated(), + isIgnoredConversionOperator( + IgnoredConversionOperators)))) .bind("conversion"), this); @@ -85,7 +114,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 options to disable warnings for constructors and conversion operators + contained in a ignore-list. + 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,22 @@ See https://google.github.io/styleguide/cppguide.html#Explicit_Constructors + +Options +------- + +.. option:: IgnoredConstructors + + Non-explicit single-argument constructors in this semicolon-separated list + will be ignored and will not trigger a warning. To ignore constructors of a + class ``A``, the list would look as follows: ``"A"``. The default list is + empty. This option is used by this check's + `cppcoreguidelines-explicit-constructor-and-conversion `_ + alias to comply with the CppCoreGuidelines. + +.. option:: IgnoredConversionOperators + + Non-explicit conversion operators in this semicolon-separated list will be + ignored and will not trigger a warning. The list to ignore conversion + operator `operator B()` in class ``A`` would look as follows: + ``"A::operator B"``. 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-ignoredconstructors-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconstructors-option.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconstructors-option.cpp @@ -0,0 +1,129 @@ +// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \ +// RUN: google-explicit-constructor %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: ]}' + +// RUN: %check_clang_tidy -check-suffix=IGNORED %s \ +// RUN: google-explicit-constructor %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: google-explicit-constructor.IgnoredConstructors, 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) {} + // IGNORED: 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); + // IGNORED: 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) {} + // IGNORED: 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); + // IGNORED: Warning disabled with ConstructorWhitelist=A + + operator bool() const { return true; } + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator bool() const { return true; } + + operator double() const; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator double() const; +}; + +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) {} + // IGNORED: 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); + // IGNORED: 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) {} + // IGNORED: 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); + // IGNORED: Warning disabled with ConstructorWhitelist=B + + operator bool() const { return true; } + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator bool() const { return true; } + + operator double() const; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator double() const; +}; + +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-IGNORED: :[[@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-IGNORED: :[[@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-IGNORED: :[[@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-IGNORED: :[[@LINE-2]]:3: warning: constructors that are callable with a single argument + // CHECK-FIXES: {{^ }}explicit C(T &&...args); + + operator bool() const { return true; } + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator bool() const { return true; } + + operator double() const; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator double() const; +}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp @@ -0,0 +1,88 @@ +// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \ +// RUN: google-explicit-constructor %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: ]}' + +// RUN: %check_clang_tidy -check-suffix=IGNORED %s \ +// RUN: google-explicit-constructor %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: google-explicit-constructor.IgnoredConversionOperators, value: "A::operator bool;B::operator double;B::operator A"} \ +// RUN: ]}' + +struct A { + A() {} + A(int x, int y) {} + + explicit A(void *x) {} + explicit A(void *x, void *y) {} + + 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-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit A(int x1); + + operator bool() const { return true; } + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}explicit operator bool() const { return true; } + + operator double() const; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator double() const; +}; + +inline A::A(int x1) {} + +struct B { + B() {} + B(int x, int y) {} + + explicit B(void *x) {} + explicit B(void *x, void *y) {} + + 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-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit B(int x1); + + operator bool() const { return true; } + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator bool() const { return true; } + + operator double() const; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}explicit operator double() const; + + operator A() const; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator A' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES-DEFAULT: {{^ }}explicit operator A() const; +}; + +struct C { + C() {} + C(int x, int y) {} + + explicit C(void *x) {} + explicit C(void *x, void *y) {} + + 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-IGNORED: :[[@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); + + operator bool() const { return true; } + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator bool() const { return true; } + + operator double() const; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator double() const; + + operator A() const; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator A' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator A' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor] + // CHECK-FIXES: {{^ }}explicit operator A() const; +};