Skip to content

[clang-tidy] Fix modernize-use-constraints crash on uses of nonstandard enable_ifs #152938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants