mgartmann updated this revision to Diff 346640.
mgartmann edited the summary of this revision.
mgartmann added a comment.
Added `ConstructorWhitelist` option to the `google-explicit-constructor` check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102779/new/
https://reviews.llvm.org/D102779
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp
===================================================================
--- /dev/null
+++ 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 <typename... T>
+ 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 <typename... T>
+ 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 <typename... T>
+ 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);
+};
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -407,6 +407,7 @@
`cppcoreguidelines-avoid-c-arrays <cppcoreguidelines-avoid-c-arrays.html>`_, `modernize-avoid-c-arrays <modernize-avoid-c-arrays.html>`_,
`cppcoreguidelines-avoid-magic-numbers <cppcoreguidelines-avoid-magic-numbers.html>`_, `readability-magic-numbers <readability-magic-numbers.html>`_,
`cppcoreguidelines-c-copy-assignment-signature <cppcoreguidelines-c-copy-assignment-signature.html>`_, `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_,
+ `cppcoreguidelines-explicit-constructor-and-conversion <cppcoreguidelines-explicit-constructor-and-conversion.html>`_, `google-explicit-constructor <google-explicit-constructor.html>`_, "Yes"
`cppcoreguidelines-explicit-virtual-functions <cppcoreguidelines-explicit-virtual-functions.html>`_, `modernize-use-override <modernize-use-override.html>`_, "Yes"
`cppcoreguidelines-non-private-member-variables-in-classes <cppcoreguidelines-non-private-member-variables-in-classes.html>`_, `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
`fuchsia-header-anon-namespaces <fuchsia-header-anon-namespaces.html>`_, `google-build-namespaces <google-build-namespaces.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
+++ 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 <cppcoreguidelines-explicit-constructor-and-conversion.html>`_
+ alias to comply with the CppCoreGuidelines. The default list is empty.
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
===================================================================
--- /dev/null
+++ 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 <google-explicit-constructor.html>`_
+for more information.
+
+This check implements `C.46 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c46-by-default-declare-single-argument-constructors-explicit>`_
+and `C.164 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c164-avoid-implicit-conversion-operators>`_
+from the CppCoreGuidelines.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,6 +127,11 @@
:doc:`concurrency-thread-canceltype-asynchronous
<clang-tidy/checks/concurrency-thread-canceltype-asynchronous>` was added.
+- New alias :doc:`cppcoreguidelines-explicit-constructor-and-conversion
+ <clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion>` to
+ :doc:`google-explicit-constructor
+ <clang-tidy/checks/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
+ <clang-tidy/checks/google-explicit-constructor>` check.
+
+ Added an option to disable warnings for constructors contained in a
+ whitelist.
+
Removed checks
^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
+++ 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
Index: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
+++ 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<std::string> NameList =
+ utils::options::parseStringList(Names);
+ return hasAnyName(std::vector<StringRef>(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<CXXConversionDecl>("conversion")) {
+ Result.Nodes.getNodeAs<CXXConversionDecl>("conversion")) {
if (Conversion->isOutOfLine())
return;
SourceLocation Loc = Conversion->getLocation();
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ 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<AvoidNonConstGlobalVariablesCheck>(
"cppcoreguidelines-avoid-non-const-global-variables");
+ CheckFactories.registerCheck<google::ExplicitConstructorCheck>(
+ "cppcoreguidelines-explicit-constructor-and-conversion");
CheckFactories.registerCheck<modernize::UseOverrideCheck>(
"cppcoreguidelines-explicit-virtual-functions");
CheckFactories.registerCheck<InitVariablesCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
