This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Drop llvm::Optional clang-specific optmization for trivially copyable types
ClosedPublic

Authored by tstellar on Nov 14 2018, 1:48 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

tstellar created this revision.Nov 14 2018, 1:48 PM
bkramer accepted this revision.Nov 14 2018, 2:09 PM

Thanks. There's a bit of unused code that passes isPodLike through the layers, but this is the minimal fix. Looks good to me.

This revision is now accepted and ready to land.Nov 14 2018, 2:09 PM
hans accepted this revision.Nov 15 2018, 1:18 AM
hans removed a reviewer: hansw.
This revision was automatically updated to reflect the committed changes.

Hmm. This makes Optional<int> non-trivial, which is a serious semantic problem for certain uses of the type (e.g. putting it in a union); it's not just an optimization. Obviously those were unportable because of the #if, but we should really fix this. There's no way that GCC just generically miscompiles all partial specializations.

rjmccall added inline comments.Nov 16 2018, 10:53 AM
llvm/trunk/include/llvm/ADT/Optional.h
121

I wonder if GCC's problem is that we haven't formally created an object of type T at this address, and the assignment isn't good enough We could just placement-new here; you're allowed to just replace objects like that when the destructor is trivial.