[clang-tidy] Don't suggest tuple_size_v in constraints in type-traits - #235257

diff --git a/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
index 49e21e8e57ce0f..e612f790e2dac7 100644
--- a/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "TypeTraitsCheck.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
@@ -159,6 +160,54 @@ static const llvm::StringSet<> TypeTraits = {
     "variant_alternative",
 };
 
+// Value traits whose primary template is not defined for every argument. For
+// these, a dependent `trait<T>::value` can be a substitution failure while
+// `trait_v<T>` is a hard error, so the two are not equivalent in constraints.
+static const llvm::StringSet<> NonSFINAEFriendlyValueTraits = {
+    "tuple_size",
+    "variant_size",
+};
+
+static bool isClause(const Expr *Clause, const Expr *E) {
+  return Clause && E && Clause->IgnoreParens() == E->IgnoreParens();
+}
+
+static bool isRequiresClause(const TemplateParameterList *TPL, const Expr *E) {
+  return TPL && isClause(TPL->getRequiresClause(), E);
+}
+
+// Returns true if E is a requires-clause of the declaration in Node.
+static bool isRequiresClauseOf(const DynTypedNode &Node, const Expr *E) {
+  if (const auto *DD = Node.get<DeclaratorDecl>();
+      DD && isClause(DD->getTrailingRequiresClause().ConstraintExpr, E))
+    return true;
+  if (const auto *TD = Node.get<TemplateDecl>())
+    return isRequiresClause(TD->getTemplateParameters(), E);
+  if (const auto *CTPSD = Node.get<ClassTemplatePartialSpecializationDecl>())
+    return isRequiresClause(CTPSD->getTemplateParameters(), E);
+  if (const auto *VTPSD = Node.get<VarTemplatePartialSpecializationDecl>())
+    return isRequiresClause(VTPSD->getTemplateParameters(), E);
+  return false;
+}
+
+// Returns true if E is part of a requires-expression, a concept definition or
+// a requires-clause.
+static bool isInConstraint(const Expr &E, ASTContext &Ctx) {
+  DynTypedNode Node = DynTypedNode::create(E);
+  while (true) {
+    const DynTypedNodeList Parents = Ctx.getParents(Node);
+    if (Parents.empty())
+      return false;
+    const DynTypedNode &Parent = Parents[0];
+    if (Parent.get<RequiresExpr>() || Parent.get<ConceptDecl>() ||
+        isRequiresClauseOf(Parent, Node.get<Expr>()))
+      return true;
+    if (Parent.get<Decl>())
+      return false;
+    Node = Parent;
+  }
+}
+
 static DeclarationName getName(const DependentScopeDeclRefExpr &D) {
   return D.getDeclName();
 }
@@ -313,8 +362,13 @@ void TypeTraitsCheck::check(const MatchFinder::MatchResult &Result) {
 
   if (const auto *DSDRE =
           Result.Nodes.getNodeAs<DependentScopeDeclRefExpr>(Bind)) {
-    if (checkTemplatedDecl(DSDRE->getQualifier(), ValueTraits))
-      EmitValueWarning(DSDRE->getQualifierLoc(), DSDRE->getEndLoc());
+    if (!checkTemplatedDecl(DSDRE->getQualifier(), ValueTraits))
+      return;
+    if (checkTemplatedDecl(DSDRE->getQualifier(),
+                           NonSFINAEFriendlyValueTraits) &&
+        isInConstraint(*DSDRE, *Result.Context))
+      return;
+    EmitValueWarning(DSDRE->getQualifierLoc(), DSDRE->getEndLoc());
     return;
   }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 57ed93042aff2c..516cf3f1671cb4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -284,6 +284,12 @@ infrastructure are described first, followed by tool-specific sections.
   `std::initializer_list` constructor, as the braced form could select a
   different constructor.
 
+- Improved {doc}`modernize-type-traits
+  <clang-tidy/checks/modernize/type-traits>` check to no longer suggest
+  `std::tuple_size_v` and `std::variant_size_v` inside requires-expressions,
+  concept definitions and requires-clauses, where they are not equivalent to
+  the `::value` form because they are not SFINAE-friendly.
+
 - Fixed a crash in {doc}`modernize-use-designated-initializers
   <clang-tidy/checks/modernize/use-designated-initializers>` when analyzing
   malformed code with nested classes and ambiguous initializer.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
index 3871c6ae3934ba..782f36b3a27769 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
@@ -55,3 +55,9 @@ Limitations
 Does not currently diagnose uses of type traits with nested name
 specifiers (e.g. ``std::chrono::is_clock``,
 ``std::chrono::treat_as_floating_point``).
+
+Does not diagnose ``std::tuple_size<T>::value`` and
+``std::variant_size<T>::value`` inside requires-expressions, concept definitions
+and requires-clauses. There, a substitution failure in the ``::value`` form
+makes the constraint unsatisfied, while ``std::tuple_size_v<T>`` and
+``std::variant_size_v<T>`` would cause a hard error instead.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits-constraints.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits-constraints.cpp
new file mode 100644
index 00000000000000..4fe99334a70e0e
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits-constraints.cpp
@@ -0,0 +1,94 @@
+// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-type-traits %t
+
+namespace std {
+  typedef decltype(sizeof(0)) size_t;
+
+  template <typename T>
+  struct tuple_size;
+
+  template <typename T>
+  inline constexpr size_t tuple_size_v = tuple_size<T>::value; // NOLINT
+
+  template <typename T>
+  struct variant_size;
+
+  template <typename T>
+  inline constexpr size_t variant_size_v = variant_size<T>::value; // NOLINT
+
+  template <typename>
+  struct is_const {
+    static constexpr bool value = true;
+  };
+
+  template <typename T>
+  inline constexpr bool is_const_v = is_const<T>::value; // NOLINT
+} // namespace std
+
+// `std::tuple_size_v<T>` and `std::variant_size_v<T>` are hard errors when
+// `std::tuple_size<T>::value` would be a substitution failure, so they must
+// not be suggested inside constraints.
+
+template <typename T>
+constexpr bool has_tuple_size() {
+  if constexpr (requires { std::tuple_size<T>::value; })
+    return true;
+  return false;
+}
+
+template <typename T>
+concept is_pair = std::tuple_size<T>::value == 2;
+
+template <typename T>
+concept has_variant_size = requires {
+  requires std::variant_size<T>::value > 0;
+};
+
+template <typename T>
+  requires(std::tuple_size<T>::value == 2)
+void requiresClause();
+
+template <typename T>
+void trailingRequiresClause()
+  requires(std::tuple_size<T>::value == 2);
+
+template <typename T>
+struct S {
+  void method()
+    requires(std::tuple_size<T>::value == 2);
+};
+
+template <typename T>
+struct Partial;
+
+template <typename T>
+  requires(std::tuple_size<T>::value == 2)
+struct Partial<T *> {};
+
+template <typename T>
+void lambdas() {
+  auto L1 = []<typename U>(U)
+    requires(std::tuple_size<U>::value == 2)
+  {};
+  auto L2 = [](auto U)
+    requires(std::tuple_size<decltype(U)>::value == 2)
+  {};
+}
+
+// Other traits are always defined, so they are still diagnosed in constraints.
+template <typename T>
+concept is_const_type = std::is_const<T>::value;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use c++17 style variable templates
+// CHECK-FIXES: concept is_const_type = std::is_const_v<T>;
+
+// Outside of constraints, the replacement is still suggested.
+template <typename T>
+constexpr std::size_t tupleSize() {
+  return std::tuple_size<T>::value;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use c++17 style variable templates
+  // CHECK-FIXES: return std::tuple_size_v<T>;
+}
+
+template <typename T>
+constexpr std::size_t variantSize = std::variant_size<T>::value;
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use c++17 style variable templates
+// CHECK-FIXES: constexpr std::size_t variantSize = std::variant_size_v<T>;