[clang-tools-extra] eb3b063 - [clang-tidy] Improve `google-explicit-constructor` checks handling of `explicit(bool)` (#82689)

[via cfe-commits]

Author: AMS21
Date: 2024-03-03T12:13:25+01:00
New Revision: eb3b063995d6b4f8f3bc22eeecbf239ffaecc29f

URL: https://github.com/llvm/llvm-project/commit/eb3b063995d6b4f8f3bc22eeecbf239ffaecc29f
DIFF: https://github.com/llvm/llvm-project/commit/eb3b063995d6b4f8f3bc22eeecbf239ffaecc29f.diff

LOG: [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

Added: 
    clang-tools-extra/test/clang-tidy/checkers/google/explicit-constructor-cxx20.cpp

Modified: 
    clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
index 34d49af9f81e23..6f26de9881357f 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 5bae530e942384..0d2467210fc664 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 00000000000000..95206f1ef420c3
--- /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