[PATCH] D143891: [Clang] Adjust triviality computation in QualType::isTrivialType to C++20 cases.

Shafik Yaghmour via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 13 09:09:49 PST 2023


shafik added a comment.

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.

I think this makes sense.



================
Comment at: clang/lib/AST/Type.cpp:2495
+      // FIXME: We should merge this definition of triviality into
+      // CXXRecordDecl::isTrivial. Currently it computes the wrong thing.
+      return ClassDecl->hasTrivialDefaultConstructor() &&
----------------
Will you follow-up this change by updating `CXXRecordDecl::isTrivial` as well or is that a more difficult issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143891



More information about the cfe-commits mailing list