https://github.com/yashaggarwal85d updated https://github.com/llvm/llvm-project/pull/235257
>From 833a3d1b94566ec779368d55facec9d924219c6d Mon Sep 17 00:00:00 2001 From: Yash Aggarwal <[email protected]> Date: Sat, 24 Oct 2026 06:55:31 +0000 Subject: [PATCH] [clang-tidy] Don't suggest tuple_size_v in constraints in type-traits --- .../clang-tidy/modernize/TypeTraitsCheck.cpp | 58 +++++++++++- clang-tools-extra/docs/ReleaseNotes.md | 6 ++ .../checks/modernize/type-traits.rst | 6 ++ .../modernize/type-traits-constraints.cpp | 94 +++++++++++++++++++ 4 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits-constraints.cpp 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>; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
