[PATCH] D102631: Allow incomplete template types in unique_function arguments

[Daniil Suchkov via Phabricator via llvm-commits]

DaniilSuchkov added inline comments.


================
Comment at: llvm/include/llvm/ADT/FunctionExtras.h:93-97
+    using type = typename std::conditional<
+        llvm::is_trivially_copy_constructible<T>::value &&
+            llvm::is_trivially_move_constructible<T>::value &&
+            IsSizeLessThanThresholdT<T>::value,
+        T, T &>::type;
----------------
DaniilSuchkov wrote:
> std::is_reference is true not only for l-value references, i.e. if you try adding `static_assert(!std::is_reference<T>::value);` after this declaration, it'll fail. So it'd be probably better to keep that.
> Also please add `static_assert(!std::is_lvalue_reference<T>::value);` to make the intentions clear.
> So it'd be probably better to keep that.

Probably adding a specialization for `T &&` would be a better idea. In that case I would recommend adding `static_assert(!std::is_reference<T>::value);` to the original definition (instead of `static_assert(!std::is_lvalue_reference<T>::value);` recommended above). And a test case with `T &&` would be nice too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102631/new/

https://reviews.llvm.org/D102631