Implementing CWG1734-

Clang Frontend
#1

Hi all-

I’m attempting to implement CWG1734[0], which changes the definition of TriviallyCopyable[1]. My motivating examples that need to pass are[2]:

struct A{};

struct C : A {

const A a;

};

struct A{};

struct D : A {

const A a;

D& operator=(D&&) = default;

};

struct E {

E &operator=(E&&)=delete;

};

static_assert(__is_trivially_copyable(C),“”);

static_assert(__is_trivially_copyable(D),“”);

static_assert(__is_trivially_copyable(E),“”);

Currently the 1st and 2nd static asserts fail. However, because of CWG1734 all 3 should pass (based on my reading). ‘C’ has its move assignment deleted, as does D, with E explicitly deleted.

However, we don’t seem to have a good way of validating each of these. I identified has(Non)TrivialMoveAssignment and data().DefaultedMoveAssignmentIsDeleted as the possible things, however there isn’t really a bit of logic that works for that.

C:

hasTrivialMoveAssignment : false

hasNonTrivialMoveAssignment : true

data().DefaultedMoveAssignmentIsDeleted : true

D:

hasTrivialMoveAssignment : false

hasNonTrivialMoveAssignment : true

data().DefaultedMoveAssignmentIsDeleted : false

E:

hasTrivialMoveAssignment : true

hasNonTrivialMoveAssignment : false

data().DefaultedMoveAssignmentIsDeleted : false

There seems to be a piece of information that I don’t have yet, or a piece of logic that I need to figure out, but I’m not sure what it could be yet. Can anyone give me a hint as to the logic? Do we need to capture the =delete in a similar flag in the DefinitionData struct? It seems that we need a hasDeleted(Copy/Move)(Constructor/Assignment) that includes the test for defaulted deleted AND trivially marked delete.

-Erich

[0] Issue 1734: Nontrivial deleted copy functions - WG21 CWG Issues

[1] https://en.cppreference.com/w/cpp/named_req/TriviallyCopyable

[2] https://godbolt.org/z/PYGb-r

#2

Could this be false for D? The move assignment is declared but trivial.

#3

Oh boy, this is old :smiley:

I tried this for a while, and @AaronBallman spent a while working on this as well, and the real problem here is that you get caught up in ABI issues. I also doubt much of my analysis is up to date, @royjacobson spent a while messing with all of this, so this question/thread is perhaps not useful/relevant anymore.

#4

I accessed here from royjacobson’s commit about P0848, and the commit indicates that the handling of deleted members is not compliant. The two asserts still fail for the trunk version of Clang. I debugged the case of D, and the return value of hasNonTrivialMoveAssignment is still true. Could you please provide the context about ABI issues?

#5

Changing the definition of triviality will cause every program that falls afoul of the rule change to change which overloads get called, etc. This results in an ABI break. We can usually be fine with that by updating the ClangABIVer thing, and adding a new entry.

Additionally, I believe Aaron and I both ran into some pretty nasty road blocks while trying to implement,but I don’t recall right now. The calculation of all of these levels of triviality are likely messed up/more information is needed somewhere. We both decided at the time it wasn’t particularly valuable of a use of our time.

BUT, assuming you add a clang-abi-version flag change, feel free to give it a shot. My memory is short as to what the difficulties were beyond what I wrote above.

#6

The issue that I recall running into had to do with things like us tracking whether a class has a default ctor as a boolean result but due to templated constructors, the class can have multiple default constructors. But it’s been a while since I dug into it and the landscape may have changed since then.

#7

So, dumping my current state on things:

  1. GCC haven’t implemented this DR. MSVC have.
  2. We run custom code to determine if types can be passed in registers, see SemaDeclCXX.cpp/canPassInRegisters. This code does not rely on the ‘proper’ type traits, so I think we can just change the type traits without changing the calling convention.
  3. ABI breaks due to is_trivial[ly_copyable] mismatch are way less scary. They could affect things like libraries optimizing constructors into memcpy etc or templates mangling. But as long as the calling convention is the same I think the space of possible bugs is way smaller. We’re currently NOT compatible with MSVC and we don’t really see complaints about that, so that suggests that this is indeed not a very bug deal.
  4. Regarding implementation - the maybe ‘correct’ way to do it is to change HasTrivialSpecialMembers into HasTrivialNonDeletedSpecialMembers and fix the places in the code that might break. This discards information but this is information that is recoverable in Sema if needed. If we want to do more gradual changes we could add 6 more bits to CXXRecordDeclDefinitionBits and keep both.
  5. Worth noting that DR1496 is also very similar and maybe they can be fixed together.
#8

I strongly agree with the point about calling conventions:

  • Feel free to change type traits. Surely this will break some code somewhere, but this can be managed with the normal tools of any transition: flags, announcements, ABI levels, etc.
  • Do not, under any circumstances, change the ABI rules used to pass types by value. That will break the world.

I was initially terrified after re-reading this text: “CWG1734[0], which changes the definition of TriviallyCopyable…” thinking that this would feed into changing calling conventions, but if you watch out for that, anything else is possible.

#9

Also, if I recall correctly, there is a series of changes to the definitions that this changes, so JUST fixing CWG1734 is probably not acceptable/is more difficult, and we should just ‘update’ the definition to whatever is the current core-iteration of it.