[PATCH] D92500: ADT: Replace guts of AlignedCharArrayUnion with std::aligned_union_t, NFC

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 2 15:00:56 PST 2020


dexonsmith added a comment.

In D92500#2429545 <https://reviews.llvm.org/D92500#2429545>, @dblaikie wrote:

> In D92500#2429186 <https://reviews.llvm.org/D92500#2429186>, @dexonsmith wrote:
>
>> @dblaikie / others, any thoughts on whether it's worth keeping our custom template parameters, or if `AlignedCharArrayUnion` should just be deleted?
>
> Eh, don't have strong feelings - the first length parameter of std::aligned_union_t seems a bit unnecessary, and if most users want to cast this to char* then it's nice to have the char[] member be named/accessible, unlike std::aligned_union_t. But I guess most users just take the address, implicitly convert to void* to pass to placement new? So then it's just that extra 1 parameter we have to pass everywhere? Yeah, I'd probably be OK with that on balance of "standard names will be more recognizable/less in-house quirkiness".

I only found one place in LLVM that required a `char` buffer and it was just doing an unnecessary `const_cast<char *>`. See the updated `bit_cast` call near the top of https://reviews.llvm.org/D92512#change-qICtDonOx8i6. (Looks like I might have missed some in clang, will update that revision in a bit...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92500



More information about the llvm-commits mailing list