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

[Bob Wilson via llvm-commits]

OK, Tom was right. This is hard to follow. I had the mistaken impression that we could revert more of these changes and get back to something that worked for Swift. I see now that the current state of things has essentially already done that, and the issues for Swift were exposed by more recent checks in Swift that rely on some Optional types being trivial. Since no one seems to have a solution, I plan to just keep the problematic code in the swift-llvm repo. I hate to diverge from trunk but it seems like the best way forward for now. Maybe sometime in the future we can switch to std::optional

> On Dec 5, 2018, at 1:00 PM, Bob Wilson <bob.wilson at apple.com> wrote:
> 
> You mention “all of the reverts” and it being hard to follow this. Here is a history of the recent changes:
> 
> r317019: Benjamin Kramer introduced this code
> r317077: NAKAMURA Takumi reverted it because it didn’t work with GCC
> r322838: Benjamin Kramer reapplied r317079, thinking that the GCC problems had gone away
> r322862: Benjamin Kramer gave up on fixing this code for GCC and put it inside an #if
> r342637: Benjamin Kramer removed the #if
> r342643: Benjamin Kramer attempted to fix it for GCC 5.4
> r342723: Benjamin Kramer attempted to fix it for GCC 5.4
> r342966: Hans Wennborg reverted r342637, r342643, and r342723
> r346985: Tom Stellard dropped the Clang-specific code for trivially copyable types
> r346990: Tom Stellard reverted r346985
> r347004: Tom Stellard reapplied r346985
> 
> Ben, can you respond? From what I can tell, we’ve never had a version of this code that works consistently with both GCC and Clang. If we can’t find a way to make that work, maybe we should just unwind this whole thing. The original comment in r317019 described the change as: "This makes uses of Optional more transparent to the compiler (and clang-tidy) and generates slightly smaller code.” That seems like something we could live without if necessary.
> 
>> On Nov 30, 2018, at 9:05 PM, Tom Stellard <tstellar at redhat.com> wrote:
>> 
>> On 11/30/2018 05:30 PM, Bob Wilson wrote:
>>> We need to figure out what to do about this. It breaks building Swift and I had to revert it from the swift-clang repo as a temporary workaround. Perhaps there is some way to adjust the code to avoid whatever problem GCC has with it? Otherwise, could we conditionally include the optimization for contexts where compatibility between GCC and Clang doesn’t matter? I don’t know of a good way to identify those contexts, but I suppose some kind of build configuration option could work.
>>> 
>> 
>> With all the reverts, this has been hard to follow.  Would someone be able to
>> submit the desired Optional implementation (even if gcc can't compile it),
>> so we have something to start a discussion with?
>> 
>> -Tom
>> 
>>>> On Nov 16, 2018, at 10:34 AM, John McCall via Phabricator via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>> 
>>>> rjmccall added a comment.
>>>> 
>>>> 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.
>>>> 
>>>> 
>>>> Repository:
>>>> rL LLVM
>>>> 
>>>> https://reviews.llvm.org/D54540
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> 
>> 
>