Skip to content

Conversation

localspook
Copy link
Contributor

@localspook localspook commented Aug 10, 2025

Fixes #152868. See that issue for details.
Fixes #123726.

@llvmbot
Copy link
Member

llvmbot commented Aug 10, 2025

@llvm/pr-subscribers-clang-tools-extra

Author: Victor Chernyakin (localspook)

Changes

Fixes #152868. See that issue for details.


Full diff: https://github.com/llvm/llvm-project/pull/152938.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp (+10)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp (+30-1)
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<NonTypeTemplateParmDecl>(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
   <clang-tidy/checks/misc/header-include-cycle>` check performance.
 
+- Fixed a :doc:`modernize-use-constraints
+  <clang-tidy/checks/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
   <clang-tidy/checks/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<T, ExpressionTemplates> &v) {
 }
 
 }
+
+template <typename T>
+struct some_type_trait {
+  static constexpr bool value = true;
+};
+
+// Fix-its are offered even for a nonstandard enable_if.
+namespace nonstd {
+
+template <bool Condition, typename T = void>
+struct enable_if : std::enable_if<Condition, T> {};
+
+}
+
+template <typename T>
+typename nonstd::enable_if<some_type_trait<T>::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<T>::value {}{{$}}
+
+// But only if the nonstandard enable_if has the same signature as the standard one.
+namespace boost {
+
+template <typename Condition, typename T = void>
+struct enable_if : std::enable_if<Condition::value, T> {};
+
+}
+
+template <typename T>
+typename boost::enable_if<some_type_trait<T>, void>::type boost_enable_if() {}

@llvmbot
Copy link
Member

llvmbot commented Aug 10, 2025

@llvm/pr-subscribers-clang-tidy

Author: Victor Chernyakin (localspook)

Changes

Fixes #152868. See that issue for details.


Full diff: https://github.com/llvm/llvm-project/pull/152938.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp (+10)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-constraints.cpp (+30-1)
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<NonTypeTemplateParmDecl>(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
   <clang-tidy/checks/misc/header-include-cycle>` check performance.
 
+- Fixed a :doc:`modernize-use-constraints
+  <clang-tidy/checks/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
   <clang-tidy/checks/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<T, ExpressionTemplates> &v) {
 }
 
 }
+
+template <typename T>
+struct some_type_trait {
+  static constexpr bool value = true;
+};
+
+// Fix-its are offered even for a nonstandard enable_if.
+namespace nonstd {
+
+template <bool Condition, typename T = void>
+struct enable_if : std::enable_if<Condition, T> {};
+
+}
+
+template <typename T>
+typename nonstd::enable_if<some_type_trait<T>::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<T>::value {}{{$}}
+
+// But only if the nonstandard enable_if has the same signature as the standard one.
+namespace boost {
+
+template <typename Condition, typename T = void>
+struct enable_if : std::enable_if<Condition::value, T> {};
+
+}
+
+template <typename T>
+typename boost::enable_if<some_type_trait<T>, void>::type boost_enable_if() {}

@localspook
Copy link
Contributor Author

Latest commit fixes enable_if_t, it also causes this crash

Copy link
Contributor

@vbvictor vbvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, as a final sanity check, could we add tests with 1 param in enable_if and specialized function like template<> enable_if<true, int>::value foo()

@localspook
Copy link
Contributor Author

The check right now skips non-dependent enable_if. Would you like the template<> enable_if<true, int>::value foo() test anyway?

@vbvictor
Copy link
Contributor

vbvictor commented Aug 12, 2025

The check right now skips non-dependent enable_if. Would you like the template<> enable_if<true, int>::value foo() test anyway?

If there isn't such a test right now, I'd say yes.
Generally, I think that every meaningful part in matchers should be tested, otherwise it should be removed as a patch "[NFC] simplify xxx check".

@localspook
Copy link
Contributor Author

Looks like we have tests for non-dependent enable_if, but not for specializations, so I've gone and added them.

template <typename U>
std::enable_if_t<true, Obj> no_dependent_type(U) {
return Obj{};
}
// FIXME - Support non-dependent enable_ifs. Low priority though...

@localspook localspook merged commit 5b5a8cd into llvm:main Aug 23, 2025
10 checks passed
@localspook localspook deleted the enable-if-crash branch August 23, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants