Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

Without this patch, we are doing a roundtrip on types. Specifically,
if decltype(...) is well formed, std::is_same_v evaluates to a boolean
value. We then pass the boolean value to std::enable_if_t, go through
the sizeof(char)/sizeof(double) trick, and then come back to a boolean
value.

This patch simplifies all this by having test() return
std::is_same<...>. The "caller" attaches ::value, so effectively we
are using std::is_same<...>::value when decltype(...) is well formed,
bypassing std::enable_if_t and the sizeof(char)/sizeof(double) trick.

If we did not care about the return type of the shift operator, we
could use llvm::is_detected, but the return type check doesn't allow
us to simplify things that far.

Without this patch, we are doing a roundtrip on types.  Specifically,
if decltype(...) is well formed, std::is_same_v evaluates to a boolean
value.  We then pass the boolean value to std::enable_if_t, go through
the sizeof(char)/sizeof(double) trick, and then come back to a boolean
value.

This patch simplifies all this by having test() return
std::is_same<...>.  The "caller" attaches ::value, so effectively we
are using std::is_same<...>::value when decltype(...) is well formed,
bypassing std::enable_if_t and the sizeof(char)/sizeof(double) trick.

If we did not care about the return type of the shift operator, we
could use llvm::is_detected, but the return type check doesn't allow
us to simplify things that far.
@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2025

@llvm/pr-subscribers-llvm-support

Author: Kazu Hirata (kazutakahirata)

Changes

Without this patch, we are doing a roundtrip on types. Specifically,
if decltype(...) is well formed, std::is_same_v evaluates to a boolean
value. We then pass the boolean value to std::enable_if_t, go through
the sizeof(char)/sizeof(double) trick, and then come back to a boolean
value.

This patch simplifies all this by having test() return
std::is_same<...>. The "caller" attaches ::value, so effectively we
are using std::is_same<...>::value when decltype(...) is well formed,
bypassing std::enable_if_t and the sizeof(char)/sizeof(double) trick.

If we did not care about the return type of the shift operator, we
could use llvm::is_detected, but the return type check doesn't allow
us to simplify things that far.


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

1 Files Affected:

  • (modified) llvm/include/llvm/Support/FormatVariadicDetails.h (+6-7)
diff --git a/llvm/include/llvm/Support/FormatVariadicDetails.h b/llvm/include/llvm/Support/FormatVariadicDetails.h
index fa11d56fc1ada..4002caf76675c 100644
--- a/llvm/include/llvm/Support/FormatVariadicDetails.h
+++ b/llvm/include/llvm/Support/FormatVariadicDetails.h
@@ -78,15 +78,14 @@ template <class T> class has_StreamOperator {
   using ConstRefT = const std::decay_t<T> &;
 
   template <typename U>
-  static char test(std::enable_if_t<
-                   std::is_same_v<decltype(std::declval<llvm::raw_ostream &>()
-                                           << std::declval<U>()),
-                                  llvm::raw_ostream &>,
-                   int *>);
+  static auto test(int)
+      -> std::is_same<decltype(std::declval<llvm::raw_ostream &>()
+                               << std::declval<U>()),
+                      llvm::raw_ostream &>;
 
-  template <typename U> static double test(...);
+  template <typename U> static auto test(...) -> std::false_type;
 
-  static bool const value = (sizeof(test<ConstRefT>(nullptr)) == 1);
+  static constexpr bool value = decltype(test<ConstRefT>(0))::value;
 };
 
 // Simple template that decides whether a type T should use the member-function

@kazutakahirata kazutakahirata merged commit 5cb7bf6 into llvm:main Sep 17, 2025
11 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250916_Support_has_StreamOperator branch September 17, 2025 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants