aboutsummaryrefslogtreecommitdiff
path: root/clang-tools-extra
diff options
context:
space:
mode:
authorAMS21 <AMS21.github@gmail.com>2024-03-03 12:13:25 +0100
committerGitHub <noreply@github.com>2024-03-03 12:13:25 +0100
commiteb3b063995d6b4f8f3bc22eeecbf239ffaecc29f (patch)
treeda1212d18761a6c50a504e37c8e304668103a05b /clang-tools-extra
parent800de14fab136f8e17c04cc783e0f8f2b305333b (diff)
downloadllvm-eb3b063995d6b4f8f3bc22eeecbf239ffaecc29f.zip
llvm-eb3b063995d6b4f8f3bc22eeecbf239ffaecc29f.tar.gz
llvm-eb3b063995d6b4f8f3bc22eeecbf239ffaecc29f.tar.bz2
[clang-tidy] Improve `google-explicit-constructor` checks handling of `explicit(bool)` (#82689)
We now treat `explicit(false)` the same way we treat `noexcept(false)` in the noexcept checks, which is ignoring it. Also introduced a new warning message if a constructor has an `explicit` declaration which evaluates to false and no longer emit a faulty FixIt. Fixes #81121
Diffstat (limited to 'clang-tools-extra')
-rw-r--r--clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp33
-rw-r--r--clang-tools-extra/docs/ReleaseNotes.rst4
-rw-r--r--clang-tools-extra/test/clang-tidy/checkers/google/explicit-constructor-cxx20.cpp59
3 files changed, 88 insertions, 8 deletions
diff --git a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
index 34d49af..6f26de9 100644
--- a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
@@ -79,8 +79,10 @@ static bool isStdInitializerList(QualType Type) {
}
void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
- constexpr char WarningMessage[] =
+ constexpr char NoExpressionWarningMessage[] =
"%0 must be marked explicit to avoid unintentional implicit conversions";
+ constexpr char WithExpressionWarningMessage[] =
+ "%0 explicit expression evaluates to 'false'";
if (const auto *Conversion =
Result.Nodes.getNodeAs<CXXConversionDecl>("conversion")) {
@@ -91,7 +93,7 @@ void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
// gmock to define matchers).
if (Loc.isMacroID())
return;
- diag(Loc, WarningMessage)
+ diag(Loc, NoExpressionWarningMessage)
<< Conversion << FixItHint::CreateInsertion(Loc, "explicit ");
return;
}
@@ -101,9 +103,11 @@ void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
Ctor->getMinRequiredArguments() > 1)
return;
+ const ExplicitSpecifier ExplicitSpec = Ctor->getExplicitSpecifier();
+
bool TakesInitializerList = isStdInitializerList(
Ctor->getParamDecl(0)->getType().getNonReferenceType());
- if (Ctor->isExplicit() &&
+ if (ExplicitSpec.isExplicit() &&
(Ctor->isCopyOrMoveConstructor() || TakesInitializerList)) {
auto IsKwExplicit = [](const Token &Tok) {
return Tok.is(tok::raw_identifier) &&
@@ -130,18 +134,31 @@ void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
return;
}
- if (Ctor->isExplicit() || Ctor->isCopyOrMoveConstructor() ||
+ if (ExplicitSpec.isExplicit() || Ctor->isCopyOrMoveConstructor() ||
TakesInitializerList)
return;
- bool SingleArgument =
+ // Don't complain about explicit(false) or dependent expressions
+ const Expr *ExplicitExpr = ExplicitSpec.getExpr();
+ if (ExplicitExpr) {
+ ExplicitExpr = ExplicitExpr->IgnoreImplicit();
+ if (isa<CXXBoolLiteralExpr>(ExplicitExpr) ||
+ ExplicitExpr->isInstantiationDependent())
+ return;
+ }
+
+ const bool SingleArgument =
Ctor->getNumParams() == 1 && !Ctor->getParamDecl(0)->isParameterPack();
SourceLocation Loc = Ctor->getLocation();
- diag(Loc, WarningMessage)
+ auto Diag =
+ diag(Loc, ExplicitExpr ? WithExpressionWarningMessage
+ : NoExpressionWarningMessage)
<< (SingleArgument
? "single-argument constructors"
- : "constructors that are callable with a single argument")
- << FixItHint::CreateInsertion(Loc, "explicit ");
+ : "constructors that are callable with a single argument");
+
+ if (!ExplicitExpr)
+ Diag << FixItHint::CreateInsertion(Loc, "explicit ");
}
} // namespace clang::tidy::google
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5bae530..0d24672 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -157,6 +157,10 @@ Changes in existing checks
<clang-tidy/checks/google/build-namespaces>` check by replacing the local
option `HeaderFileExtensions` by the global option of the same name.
+- Improved :doc:`google-explicit-constructor
+ <clang-tidy/checks/google/explicit-constructor>` check to better handle
+ ``C++-20`` `explicit(bool)`.
+
- Improved :doc:`google-global-names-in-headers
<clang-tidy/checks/google/global-names-in-headers>` check by replacing the local
option `HeaderFileExtensions` by the global option of the same name.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/explicit-constructor-cxx20.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/explicit-constructor-cxx20.cpp
new file mode 100644
index 0000000..95206f1
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/explicit-constructor-cxx20.cpp
@@ -0,0 +1,59 @@
+// RUN: %check_clang_tidy %s google-explicit-constructor %t -std=c++20-or-later
+
+namespace issue_81121
+{
+
+static constexpr bool ConstFalse = false;
+static constexpr bool ConstTrue = true;
+
+struct A {
+ explicit(true) A(int);
+};
+
+struct B {
+ explicit(false) B(int);
+};
+
+struct C {
+ explicit(ConstTrue) C(int);
+};
+
+struct D {
+ explicit(ConstFalse) D(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: single-argument constructors explicit expression evaluates to 'false' [google-explicit-constructor]
+};
+
+template <typename>
+struct E {
+ explicit(true) E(int);
+};
+
+template <typename>
+struct F {
+ explicit(false) F(int);
+};
+
+template <typename>
+struct G {
+ explicit(ConstTrue) G(int);
+};
+
+template <typename>
+struct H {
+ explicit(ConstFalse) H(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: single-argument constructors explicit expression evaluates to 'false' [google-explicit-constructor]
+};
+
+template <int Val>
+struct I {
+ explicit(Val > 0) I(int);
+};
+
+template <int Val>
+struct J {
+ explicit(Val > 0) J(int);
+};
+
+void useJ(J<0>, J<100>);
+
+} // namespace issue_81121