erichkeane added a comment.

In D143891#4125954 <https://reviews.llvm.org/D143891#4125954>, @erichkeane 
wrote:

> In D143891#4122731 <https://reviews.llvm.org/D143891#4122731>, @aaron.ballman 
> wrote:
>
>> In D143891#4122668 <https://reviews.llvm.org/D143891#4122668>, @royjacobson 
>> wrote:
>>
>>> In D143891#4122660 <https://reviews.llvm.org/D143891#4122660>, 
>>> @aaron.ballman wrote:
>>>
>>>> This is an ABI breaking change, isn't it? (The type trait now returns 
>>>> something different than it did before, which could change instantiations 
>>>> or object layout.)
>>>
>>> Technically it is, but it only affects code that relies on constrained 
>>> default constructors, which we're only going to support in Clang 16. So if 
>>> we backport this to 16 it's not very problematic.
>>
>> Hmmm, if it's true that this only changes the behavior of a type with a 
>> constrained default constructor, then I think it's fine despite being an ABI 
>> break (we never claimed full support for concepts, so anyone relying on a 
>> particular behavior was mistaken to do that). I can't quite convince myself 
>> this doesn't impact other cases though -- the logic for computing triviality 
>> is nontrivial itself (pun intended), so I'm really only concerned that 
>> `__is_trivial` and friends return a different value for a non-constrained 
>> type. However, I also can't come up with a test case whose behavior changes 
>> either.
>
> Avoiding hte rest of the discussion, because I cannot even:
> ABI Breaks like this are typically OK, BUT we need to update the 
> ClangABIVersion stuff, which includes only implementing this in the newest 
> version.

Aaron points out offline that this is Concepts specific, and can only be run 
into with a concept.  I didn't realize on first read the 'ineligible' was 
'disabled with a constraint'.  If so, we don't really make any guarantees about 
ABI compat with Concepts yet, so the ClangABIVersion stuff is likely 
unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143891

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to