[PATCH] D92509: ADT: Remove redundant `alignas` from IntervalMap, NFC

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In D92509#2456446 <https://reviews.llvm.org/D92509#2456446>, @dexonsmith wrote:

> I missed this email yesterday, but your solution SGTM.
>
> It looks like you did not revert 65c5c9f92ec514ae41c8ea407d1c885737d699ec <https://reviews.llvm.org/rG65c5c9f92ec514ae41c8ea407d1c885737d699ec> (ADT: Rely on std::aligned_union_t for math in AlignedCharArrayUnion, NFC), which changed it to rely on the math in `std::aligned_union_t`:
>
>   template <typename T, typename... Ts> struct AlignedCharArrayUnion {
>     using AlignedUnion = std::aligned_union_t<1, T, Ts...>;
>     alignas(alignof(AlignedUnion)) char buffer[sizeof(AlignedUnion)];
>   };
>
> Should that be reverted too, or if not, why does `alignof(std::aligned_union_t<...>)` give us the right answer?

Basically, it's an MSVC compiler bug: it won't align local variables unless they have explicit an `alignas` attribute. `std::aligned_union_t<...>` produces a type with the right size and alignment, but then the compiler doesn't follow through on the alignment for local variables. LLVM's implementation uses alignas, which makes it work.

This should be documented in comments, I'll send that out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92509