[libcxx-commits] [PATCH] D145376: [libc++] add declval failure assertion for instantiation

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__utility/declval.h:30
+template <class _Tp, bool _False = false>
+_LIBCPP_HIDE_FROM_ABI decltype(std::__declval<_Tp>(0)) declval() _NOEXCEPT {
+  static_assert(_False, "Calling declval is ill-formed, see [declval]/2.");
----------------
fsb4000 wrote:
> fsb4000 wrote:
> > frederick-vs-ja wrote:
> > > The return type is... cursed for //cv// `void`.
> > > 
> > > It seems that `decltype(std::declval<const void>)` should be `const void()` (until C++17) / `const void() noexcept` (since C++17) (same for `volatile void` and `const volatile void`), but the cv-qualifiers are dropped in `decltype(std::__declval<_Tp>(0))`.
> > I found that Eric Niebler added the patch: https://bugs.llvm.org/show_bug.cgi?id=27798
> > 
> > It was `typename add_rvalue_reference<_Tp>::type` before.
> > 
> > 
> I've created: https://github.com/llvm/llvm-project/issues/61232
I think the performance improvement might not apply anymore, since we have a builtin now. Using `__add_rvalue_reference_t<_Tp>` should fix the bug and probably not result in any performance regressions. I'll try to make a small benchmark.


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

https://reviews.llvm.org/D145376