diff --git a/Lib/Coproduct.hpp b/Lib/Coproduct.hpp index 8bb29c162..e97c8db2d 100644 --- a/Lib/Coproduct.hpp +++ b/Lib/Coproduct.hpp @@ -201,7 +201,7 @@ namespace CoproductImpl { { this->switchN([&](auto N) { using A = TL::Get; - this->template cast().~A(); + this->template castAt().~A(); }); } }; @@ -228,8 +228,8 @@ namespace CoproductImpl { this->assignTag(other.tag()); \ this->switchN([&](auto N) { \ using A = TL::Get; \ - ::new(&this->template cast()) \ - A(MOVE(other.template cast())); \ + ::new(&this->template castAt()) \ + A(MOVE(other.template castAt())); \ }); \ } \ \ @@ -239,7 +239,7 @@ namespace CoproductImpl { if (this == &other) return *this; \ this->switchN([&](auto N) { \ using A = TL::Get; \ - this->template cast().~A(); \ + this->template castAt().~A(); \ }); \ ::new(this) DefaultImpl(MOVE(other)); \ return *this; \ @@ -255,27 +255,53 @@ namespace CoproductImpl { } - template - struct MaxSize; + /** + * Storage holding exactly one of As..., as a union rather than a byte buffer: a union + * tells the compiler that these types share the memory, so reading the live alternative + * is an ordinary member access at the right type and alias analysis stays correct. + * Do not go back to a `char[]` read through a cast -- that is undefined, and GCC + * miscompiled it here under -flto. + * + * Nothing is constructed or destroyed in the storage: Coproduct placement-news the live + * alternative and destroys it explicitly (see TrivialOperations). The destructor must + * still be *trivial* whenever every alternative is trivially destructible, or Coproduct + * loses its own triviality -- hence the two specialisations. + */ + template + union VariantStorage; - template<> - struct MaxSize<> - { static constexpr unsigned value = 0; }; + template<> union VariantStorage {}; + template<> union VariantStorage {}; + + template + union VariantStorage { + A head; + VariantStorage tail; + VariantStorage() {} // leaves every alternative dead + }; - template - struct MaxSize - { static constexpr unsigned value = std::max(sizeof(T), MaxSize::value); }; + template + union VariantStorage { + A head; + VariantStorage tail; + VariantStorage() {} // leaves every alternative dead + ~VariantStorage() {} + }; - template - struct MaxAlign; + /** Reference to the idx-th alternative of a VariantStorage; constness follows the + * storage. Coproduct constructs through this same path, so the alternative read is + * the one that was constructed. */ + template + struct StorageAt { + template + static auto& get(S& self) { return StorageAt::get(self.tail); } + }; template<> - struct MaxAlign<> - { static constexpr unsigned value = 0; }; - - template - struct MaxAlign - { static constexpr unsigned value = std::max(alignof(T), MaxAlign::value); }; + struct StorageAt<0> { + template + static auto& get(S& self) { return self.head; } + }; template class RawCoproduct { @@ -301,9 +327,13 @@ namespace CoproductImpl { static_assert(nTags == 0 || nTags - 1 == ((nTags - 1) & bitMask), "bug in function neededBits"); - using Bytes = char [MaxSize::value]; + /** trivial destructibility has to be inherited by the storage; see VariantStorage */ + static constexpr bool allTriviallyDestructible + = TL::All::val; + + using Storage = VariantStorage; unsigned _tag: neededBits(nTags); - alignas(MaxAlign::value) Bytes _content; + Storage _content; TrivialOperations::DisableIfNeeded _copyCons; @@ -317,8 +347,9 @@ namespace CoproductImpl { : _tag(size) { #if __COPRODUCT_CONTENT_INIT - for (unsigned i = 0; i < sizeof(Bytes); i++) { - _content[i] = 0xFF; + auto* bytes = reinterpret_cast(&_content); + for (unsigned i = 0; i < sizeof(Storage); i++) { + bytes[i] = 0xFF; } #endif // __COPRODUCT_CONTENT_INIT } @@ -333,11 +364,11 @@ namespace CoproductImpl { ) #define CONST_POLYMORPIHIC(CONST) \ - template \ - B CONST& cast() CONST \ + template \ + TL::Get CONST& castAt() CONST \ { \ - static_assert(TL::Contains>::val, "invalid cast"); \ - return *(B CONST*)_content; \ + static_assert(idx < size, "variant index out of bounds"); \ + return StorageAt::get(_content); \ } \ \ @@ -582,7 +613,7 @@ class Coproduct inline TL::Get REF unwrap() REF { \ static_assert(idx < size, "out of bounds"); \ ASS_EQ(idx, tag()); \ - return MOVE(_inner.template cast>()); \ + return MOVE(_inner.template castAt()); \ } \ \ /** \ @@ -633,7 +664,8 @@ class Coproduct static_assert(std::is_same>::value, "illegal index for variant"); _inner.template assignTag(); - ::new(&_inner._content) B(move_if_value(value._self)); + // through castAt(), so the alternative constructed is the one read back + ::new(&_inner.template castAt()) B(move_if_value(value._self)); } /**