Skip to content

Conversation

kazutakahirata
Copy link
Contributor

This patch consolidates the two implementations of CallbacksHolder
with "if constexpr". The immediately evaluated lambda expression
works as a type selector as well as initializers. This way, we can
migrate away from the SFINAE trick.

This patch consolidates the two implementations of CallbacksHolder
with "if constexpr".  The immediately evaluated lambda expression
works as a type selector as well as initializers.  This way, we can
migrate away from the SFINAE trick.
@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch consolidates the two implementations of CallbacksHolder
with "if constexpr". The immediately evaluated lambda expression
works as a type selector as well as initializers. This way, we can
migrate away from the SFINAE trick.


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

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/FunctionExtras.h (+11-15)
diff --git a/llvm/include/llvm/ADT/FunctionExtras.h b/llvm/include/llvm/ADT/FunctionExtras.h
index 1311452a17bb3..12a2dc36e1af0 100644
--- a/llvm/include/llvm/ADT/FunctionExtras.h
+++ b/llvm/include/llvm/ADT/FunctionExtras.h
@@ -58,10 +58,6 @@ template <typename FunctionT> class unique_function;
 
 namespace detail {
 
-template <typename T>
-using EnableIfTrivial =
-    std::enable_if_t<std::is_trivially_move_constructible<T>::value &&
-                     std::is_trivially_destructible<T>::value>;
 template <typename CallableT, typename ThisT>
 using EnableUnlessSameType =
     std::enable_if_t<!std::is_same<remove_cvref_t<CallableT>, ThisT>::value>;
@@ -236,17 +232,17 @@ template <typename ReturnT, typename... ParamTs> class UniqueFunctionBase {
   // type erased behaviors needed. Create a static instance of the struct type
   // here and each instance will contain a pointer to it.
   // Wrap in a struct to avoid https://gcc.gnu.org/PR71954
-  template <typename CallableT, typename CalledAs, typename Enable = void>
-  struct CallbacksHolder {
-    inline static NonTrivialCallbacks Callbacks = {
-        &CallImpl<CalledAs>, &MoveImpl<CallableT>, &DestroyImpl<CallableT>};
-  };
-  // See if we can create a trivial callback. We need the callable to be
-  // trivially moved and trivially destroyed so that we don't have to store
-  // type erased callbacks for those operations.
-  template <typename CallableT, typename CalledAs>
-  struct CallbacksHolder<CallableT, CalledAs, EnableIfTrivial<CallableT>> {
-    inline static TrivialCallback Callbacks = {&CallImpl<CalledAs>};
+  template <typename CallableT, typename CalledAs> struct CallbacksHolder {
+    inline static auto Callbacks = []() constexpr {
+      // For trivial callables, we don't need to store move and destroy
+      // callbacks.
+      if constexpr (std::is_trivially_move_constructible_v<CallableT> &&
+                    std::is_trivially_destructible_v<CallableT>)
+        return TrivialCallback{&CallImpl<CalledAs>};
+      else
+        return NonTrivialCallbacks{&CallImpl<CalledAs>, &MoveImpl<CallableT>,
+                                   &DestroyImpl<CallableT>};
+    }();
   };
 
   // A simple tag type so the call-as type to be passed to the constructor.

else
return NonTrivialCallbacks{&CallImpl<CalledAs>, &MoveImpl<CallableT>,
&DestroyImpl<CallableT>};
}();
Copy link
Member

Choose a reason for hiding this comment

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

Neat, TIL

@kazutakahirata kazutakahirata merged commit 6995742 into llvm:main Sep 23, 2025
11 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250922_10TMP_ADT_FunctionExtras_CallbacksHolder_if_constexpr branch September 23, 2025 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants