search

chromium / subspace

A concept-centered standard library for C++20, enabling safer and more reliable products and a more modern feel for C++ code.; Also home of Subdoc the code-documentation generator.
https://suslib.cc
Apache License 2.0
89 stars 15 forks source link

Replace __has_extension usage #459

Open Quuxplusone opened 6 days ago

Quuxplusone commented 6 days ago

https://github.com/chromium/subspace/blob/f9c481a241961a7be827d31fadb01badac6ee86a/sus/mem/relocate.h#L60C7-L60C45

template <class T>
concept TriviallyRelocatable_impl =
    ~~~~
  #if __has_extension(trivially_relocatable)
         || __is_trivially_relocatable(std::remove_all_extents_t<T>)
  #endif

I was recently re-reminded that __has_extension isn't the best/right way to test for this (per @zygoloid's Dec 2021 comment in https://reviews.llvm.org/D114732#3165581 ). I suggest either removing this codepath entirely or replacing it with one of the P1144R9 feature-test macros:

(almost certainly the best option)

  #if defined(__cpp_lib_trivially_relocatable)
         || std::is_trivially_relocatable_v<std::remove_all_extents_t<T>>
  #endif

or (IMHO a worse option)

  #if defined(__clang__) && defined(__cpp_impl_trivially_relocatable)
         || __is_trivially_relocatable(std::remove_all_extents_t<T>)
  #endif

Note that Clang trunk's __is_trivially_relocatable(T) builtin is still not usable for your purposes (i.e., https://github.com/llvm/llvm-project/pull/84621 still isn't moving), so this codepath is still relevant only to the P1144 reference implementation on Godbolt.

attn: @danakj