[PATCH] D54472: Disable invalid isPodLike<> specialization

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 15 13:08:19 PST 2018


dblaikie added inline comments.


================
Comment at: include/llvm/Support/type_traits.h:40-44
+#define LLVM_VERIFY_ISPODLIKE_CONSISTENCY(T)                                   \
+  static_assert(llvm::isPodLike<T>::value ==                                   \
+                    std::is_trivially_copyable<T>::value,                      \
+                "pod-like implies trivially copyable")
+
----------------
serge-sans-paille wrote:
> dblaikie wrote:
> > Does the presence of this macro mean "no one should ever specialize isPodLike"? & if they don't, then its certain that isPodLike == is_trivially_copyable, by definition?
> > 
> > If so, then I'm not sure of the importance of the macro - we should just say "don't specialize isPodLike, ever & leave it at that, maybe?
> Almost. It means « if you specialize ``isPodLike``, then make sure that for modern compiler, it does not conflict with std::is_trivially_copyable ».
> 
> Basically for modern compiler, we should have
> 
> ``template<class T> using isPodLike = std::is_trivially_copiable<T>;``
> 
> However, some compilers supported by LLVM don't implement that feature. In my opinion, it is still valid to speicalize ``isPodLike`` to ensure decent performance in these case. But not at the cost of UB.
OK, fair enough. 

Though does the performance matter on platforms that don't support the real trait? Which compilers/versions is that compared to the set of supported compilers/versions for building Clang/LLVM. (also, in part, the performance of Clang/LLVM is most important on a self-host build - correctness more broadly, but those interest in performance should likely be bootstrapping)

In any case, /if/ we're going to keep that performance feature (whereby LLVM developers can annotate types they know would pass is_trivially_copyable if it were available), probably validating its usage should be kept at the same place as the trait specializations, rather than in the usage?

Any chance of improving/changing Optional/SuccIterator to be trivially copyable when their parameters are trivially copyable?

(& is libc++'s std::pair trivially copyable when T1 and T2 are trivially copyable?)


Repository:
  rL LLVM

https://reviews.llvm.org/D54472





More information about the llvm-commits mailing list