cjdb added a subscriber: jwakely. cjdb added inline comments.
================ Comment at: clang/include/clang/Basic/TokenKinds.def:528 +TYPE_TRAIT_1(__is_nothrow_copy_constructible, IsNothrowCopyConstructible, KEYCXX) +TYPE_TRAIT_1(__is_trivially_copy_constructible, IsTriviallyCopyConstructible, KEYCXX) TYPE_TRAIT_2(__reference_binds_to_temporary, ReferenceBindsToTemporary, KEYCXX) ---------------- ldionne wrote: > cjdb wrote: > > erichkeane wrote: > > > royjacobson wrote: > > > > erichkeane wrote: > > > > > cjdb wrote: > > > > > > erichkeane wrote: > > > > > > > So this one is a whole 'thing'. The Clang definition of > > > > > > > 'trivially copy constructible' is a few DRs behind. We should > > > > > > > probably discuss this with libcxx to make sure use of this > > > > > > > wouldn't be broken. > > > > > > I'd prefer to get those DRs in before finalising D135238 and > > > > > > subsequent ones. Do you know the DR numbers I should be looking at, > > > > > > or should I just poke npaperbot? > > > > > Not off the top of my head, Aaron and I both poked at it at one point > > > > > trying to get trivially constructible right at one point, but I think > > > > > we both gave up due to the ABI/versioning concerns. > > > > Maybe DR1734? Although it's about the trivially copyable trait, not > > > > trivially copy constructible. > > > > > > > Yeah, I think that was the DR, that number sounds familiar. > > The `__is_trivially_*` traits were, in part, what inspired the Great Split > > of D116208. I could remove them for now and revisit once I rip my hair out > > over these DRs, if that would substantially improve the chances of these > > commits landing (other commentary notwithstanding). > I am not sure I see a problem with the "triviality" part of this -- we > already use a compiler builtin for `std::is_trivially_constructible`, so I > would expect either this patch is fine, or we already have a latent bug in > libc++. > > I think I can echo @philnik's comment about this not necessarily providing > the biggest benefit since our implementation of > `std::is_trivially_copy_constructible` is a fairly trivial wrapper on top of > `__is_trivially_constructible`, but I wouldn't object to the patch on that > basis. I think it would probably be possible to instead provide a set of > basis builtin operations that we can then build all of the library type > traits on top of -- that would provide the highest bang-for-our-buck ratio. > > At the same time, there's something kind of enticing in the consistency of > defining every single type trait as a builtin, without exception. If that's > the end goal, I think that would also be neat and we'd likely regroup all of > our type traits under a single header, since each of them would literally be > a one liner. > > There's also the question of whether GCC provides these builtins -- if they > don't and if they don't have plans to, then we'd actually need to add > complexity in libc++ to support both, which we would be unlikely to do given > that there's probably not a huge compile-time performance benefit. > > TLDR, I think the two questions that can help gauge how much interest there > will be from libc++ to use this are: > > 1. Is the plan to provide *all* the type traits as builtins? > 2. Will GCC implement them? > > That being said, libc++ might not be the only potential user of these > builtins, so I wouldn't necessarily make it a hard requirement to satisfy us. > > I think I can echo @philnik's comment about this not necessarily providing > the biggest benefit since our implementation of > `std::is_trivially_copy_constructible` is a fairly trivial wrapper on top of > `__is_trivially_constructible`, but I wouldn't object to the patch on that > basis. I haven't had time to do anything properly in the way of benchmarking, but after looking at @philnik's quoted code, I see that I'd naively addressed `__is_constructible(T, T const&)`, forgetting that `__add_lvalue_reference` would've fixed that issue. > 1. Is the plan to provide *all* the type traits as builtins? Yes, with the possible exception of `enable_if` and `add_const` etc. (see D116203 for why the qualifier ones aren't already in). The hardest ones will probably be `common_type`, `common_reference`, `*invocable*`, and `*swappable*`. The former two depend on technology that doesn't exist in Clang yet, and the latter two are likely hard due there not being prior art. > 2. Will GCC implement them? @jwakely do you know if there can be cross-compiler synergy here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135238/new/ https://reviews.llvm.org/D135238 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
