From 728fc710260d529a604bf3692b18c9131061e070 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin Date: Sun, 10 Aug 2025 12:25:26 -0700 Subject: [PATCH 1/5] [clang-tidy] Fix `modernize-use-constraints` crash on uses of nonstandard `enable_if`s --- .../modernize/UseConstraintsCheck.cpp | 10 ++++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 +++ .../checkers/modernize/use-constraints.cpp | 31 ++++++++++++++++++- 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp index e9b96c4016af6..ab25fb675552a 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp @@ -8,6 +8,7 @@ #include "UseConstraintsCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclTemplate.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" @@ -78,6 +79,15 @@ matchEnableIfSpecializationImplTypename(TypeLoc TheType) { if (!TD || TD->getName() != "enable_if") return std::nullopt; + const TemplateParameterList *Params = TD->getTemplateParameters(); + if (Params->size() != 2) + return std::nullopt; + + const auto *FirstParam = + dyn_cast(Params->getParam(0)); + if (!FirstParam || !FirstParam->getType()->isBooleanType()) + return std::nullopt; + int NumArgs = SpecializationLoc.getNumArgs(); if (NumArgs != 1 && NumArgs != 2) return std::nullopt; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2de2818172850..f6aedb299e456 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -136,6 +136,11 @@ Changes in existing checks - Improved :doc:`misc-header-include-cycle ` check performance. +- Fixed a :doc:`modernize-use-constraints + ` crash on uses of + nonstandard ``enable_if``s with a signature different from + ``std::enable_if`` (such as ``boost::enable_if``). + - Improved :doc:`modernize-use-designated-initializers ` check to suggest using designated initializers for aliased aggregate types. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp index 3bcd5cd74024e..d61073c1aac3f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy -std=c++20 %s modernize-use-constraints %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-constraints %t -- -- -fno-delayed-template-parsing // NOLINTBEGIN namespace std { @@ -756,3 +756,32 @@ abs(const number &v) { } } + +template +struct some_type_trait { + static constexpr bool value = true; +}; + +// Fix-its are offered even for a nonstandard enable_if. +namespace nonstd { + +template +struct enable_if : std::enable_if {}; + +} + +template +typename nonstd::enable_if::value, void>::type nonstd_enable_if() {} +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints] +// CHECK-FIXES: {{^}}void nonstd_enable_if() requires some_type_trait::value {}{{$}} + +// But only if the nonstandard enable_if has the same signature as the standard one. +namespace boost { + +template +struct enable_if : std::enable_if {}; + +} + +template +typename boost::enable_if, void>::type boost_enable_if() {} From a189367029572320d44ad3fb787571d1e4e984f1 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin Date: Sun, 10 Aug 2025 13:22:55 -0700 Subject: [PATCH 2/5] Adjust wording --- clang-tools-extra/docs/ReleaseNotes.rst | 6 +++--- .../test/clang-tidy/checkers/modernize/use-constraints.cpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f6aedb299e456..51deab4dfd234 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -136,9 +136,9 @@ Changes in existing checks - Improved :doc:`misc-header-include-cycle ` check performance. -- Fixed a :doc:`modernize-use-constraints - ` crash on uses of - nonstandard ``enable_if``s with a signature different from +- Improved :doc:`modernize-use-constraints + ` check by fixing a crash on + uses of non-standard ``enable_if`` with a signature different from ``std::enable_if`` (such as ``boost::enable_if``). - Improved :doc:`modernize-use-designated-initializers diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp index d61073c1aac3f..54e66882ff9fe 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp @@ -762,7 +762,7 @@ struct some_type_trait { static constexpr bool value = true; }; -// Fix-its are offered even for a nonstandard enable_if. +// Fix-its are offered even for a non-standard enable_if. namespace nonstd { template @@ -775,7 +775,7 @@ typename nonstd::enable_if::value, void>::type nonstd_enable_ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints] // CHECK-FIXES: {{^}}void nonstd_enable_if() requires some_type_trait::value {}{{$}} -// But only if the nonstandard enable_if has the same signature as the standard one. +// But only if the non-standard enable_if has the same signature as the standard one. namespace boost { template From 3911696efef576c43268e3582c9d4d8cc2e7e8c5 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin Date: Sun, 10 Aug 2025 15:08:10 -0700 Subject: [PATCH 3/5] Remove check for zero template arguments, handle `enable_if_t` --- .../clang-tidy/modernize/UseConstraintsCheck.cpp | 13 +++++++------ .../checkers/modernize/use-constraints.cpp | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp index ab25fb675552a..10f6eb5aa3271 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp @@ -79,12 +79,8 @@ matchEnableIfSpecializationImplTypename(TypeLoc TheType) { if (!TD || TD->getName() != "enable_if") return std::nullopt; - const TemplateParameterList *Params = TD->getTemplateParameters(); - if (Params->size() != 2) - return std::nullopt; - - const auto *FirstParam = - dyn_cast(Params->getParam(0)); + const auto *FirstParam = dyn_cast( + TD->getTemplateParameters()->getParam(0)); if (!FirstParam || !FirstParam->getType()->isBooleanType()) return std::nullopt; @@ -118,6 +114,11 @@ matchEnableIfSpecializationImplTrait(TypeLoc TheType) { if (!Specialization->isTypeAlias()) return std::nullopt; + const auto *FirstParam = dyn_cast( + TD->getTemplateParameters()->getParam(0)); + if (!FirstParam || !FirstParam->getType()->isBooleanType()) + return std::nullopt; + if (const auto *AliasedType = dyn_cast(Specialization->getAliasedType())) { ElaboratedTypeKeyword Keyword = AliasedType->getKeyword(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp index 54e66882ff9fe..7a8721061be2c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp @@ -768,6 +768,9 @@ namespace nonstd { template struct enable_if : std::enable_if {}; +template +using enable_if_t = typename enable_if::type; + } template @@ -775,13 +778,24 @@ typename nonstd::enable_if::value, void>::type nonstd_enable_ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints] // CHECK-FIXES: {{^}}void nonstd_enable_if() requires some_type_trait::value {}{{$}} +template +nonstd::enable_if_t::value, void> nonstd_enable_if_t() {} +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints] +// CHECK-FIXES: {{^}}void nonstd_enable_if_t() requires some_type_trait::value {}{{$}} + // But only if the non-standard enable_if has the same signature as the standard one. namespace boost { template struct enable_if : std::enable_if {}; +template +using enable_if_t = typename enable_if::type; + } template typename boost::enable_if, void>::type boost_enable_if() {} + +template +boost::enable_if_t, void> boost_enable_if_t() {} From 3d273ebd1712f120e93f91ff5d2bf96c978a4ff7 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin Date: Mon, 11 Aug 2025 20:51:43 -0700 Subject: [PATCH 4/5] Add asserts --- .../clang-tidy/modernize/UseConstraintsCheck.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp index 10f6eb5aa3271..4dc1a0a2c47a4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp @@ -79,6 +79,8 @@ matchEnableIfSpecializationImplTypename(TypeLoc TheType) { if (!TD || TD->getName() != "enable_if") return std::nullopt; + assert(!TD->getTemplateParameters()->empty() && + "found template with no template parameters?"); const auto *FirstParam = dyn_cast( TD->getTemplateParameters()->getParam(0)); if (!FirstParam || !FirstParam->getType()->isBooleanType()) @@ -114,6 +116,8 @@ matchEnableIfSpecializationImplTrait(TypeLoc TheType) { if (!Specialization->isTypeAlias()) return std::nullopt; + assert(!TD->getTemplateParameters()->empty() && + "found template with no template parameters?"); const auto *FirstParam = dyn_cast( TD->getTemplateParameters()->getParam(0)); if (!FirstParam || !FirstParam->getType()->isBooleanType()) From f01c0fc702fec03f874c3fa2f1bd3a4a04775c34 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin Date: Tue, 12 Aug 2025 13:46:22 -0700 Subject: [PATCH 5/5] Single argument form, specializations --- .../checkers/modernize/use-constraints.cpp | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp index 7a8721061be2c..90131c3d86920 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp @@ -783,7 +783,21 @@ nonstd::enable_if_t::value, void> nonstd_enable_if_t() {} // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints] // CHECK-FIXES: {{^}}void nonstd_enable_if_t() requires some_type_trait::value {}{{$}} -// But only if the non-standard enable_if has the same signature as the standard one. +template <> +nonstd::enable_if_t::value, void> nonstd_enable_if_t() {} +// FIXME - Support non-dependent enable_ifs. + +template +typename nonstd::enable_if::value>::type nonstd_enable_if_one_param() {} +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints] +// CHECK-FIXES: {{^}}void nonstd_enable_if_one_param() requires some_type_trait::value {}{{$}} + +template +nonstd::enable_if_t::value> nonstd_enable_if_t_one_param() {} +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use C++20 requires constraints instead of enable_if [modernize-use-constraints] +// CHECK-FIXES: {{^}}void nonstd_enable_if_t_one_param() requires some_type_trait::value {}{{$}} + +// No fix-its are offered for an enable_if with a different signature from the standard one. namespace boost { template @@ -799,3 +813,12 @@ typename boost::enable_if, void>::type boost_enable_if() {} template boost::enable_if_t, void> boost_enable_if_t() {} + +template <> +boost::enable_if_t, void> boost_enable_if_t() {} + +template +typename boost::enable_if>::type boost_enable_if_one_param() {} + +template +boost::enable_if_t> boost_enable_if_t_one_param() {}