[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGf466f0beda59: Disallow trivial_abi on a class if all copy and move constructors are deleted (authored by ahatanak).

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

ahatanak added a comment. Yes, I'm still interested in pursuing this. I'll rebase the patch and commit it if I don't get any more feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57626/new/ https://reviews.llvm.org/D57626

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

ahatanak updated this revision to Diff 269633. ahatanak added a comment. Rebase patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57626/new/ https://reviews.llvm.org/D57626 Files: clang/include/clang/Basic/AttrDocs.td

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

rsmith added a comment. Herald added a subscriber: ributzka. Are you still interested in pursuing this? This change looks good to me if so. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57626/new/ https://reviews.llvm.org/D57626

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

ahatanak added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953 + "have bases of non-trivial class types|have virtual bases|" + "have __weak fields under ARC|have fields of non-trivial class types}0">; Quuxplusone wrote: >

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

ahatanak updated this revision to Diff 186083. ahatanak marked 4 inline comments as done. ahatanak added a comment. Improve diagnostic messages. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57626/new/ https://reviews.llvm.org/D57626 Files:

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953 + "have bases of non-trivial class types|have virtual bases|" + "have __weak fields under ARC|have fields of non-trivial class types}0">; ahatanak wrote: >

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

ahatanak marked 2 inline comments as done. ahatanak added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953 + "have bases of non-trivial class types|have virtual bases|" + "have __weak fields under ARC|have fields of non-trivial class types}0">;

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

rsmith added a comment. In D57626#1382391 , @Quuxplusone wrote: > I admit that this `lock_guard` example is contrived and generally ill-advised > , > but its

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

Quuxplusone added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953 + "have bases of non-trivial class types|have virtual bases|" + "have __weak fields under ARC|have fields of non-trivial class types}0">; nit: "of non-trivial

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

ahatanak updated this revision to Diff 185157. ahatanak added a comment. Add a note diagnostic to inform the user of the reason for not allowing annotating a class with `trivial_abi`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57626/new/

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

rjmccall added a comment. Richard felt that this was an odd special case, and I was happy to use the old language-designer's dodge of banning something today so that we can decide to allow it tomorrow. This isn't SFINAE-able. ...of course, if it's just a *warning* that this isn't allowed,

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

Quuxplusone added a comment. Can you give more intuition on why classes with no copy/move operations should be forced non-trivial-abi? Let's take this specific example: struct [[clang::trivial_abi]] lock_guard { mutex *m; explicit lock_guard(mutex *m) : m(m) { m->lock(); }

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

rjmccall added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:7886 + if (!HasNonDeletedCopyOrMoveConstructor()) { +PrintDiagAndRemoveAttr(); +return; This is not a very useful diagnostic. We have several different reasons why we reject the

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

ahatanak created this revision. ahatanak added reviewers: rsmith, rjmccall, aaron.ballman. ahatanak added a project: clang. Herald added subscribers: dexonsmith, jkorous. Instead of forcing the class to be passed in registers, which was what r350920 did, issue a warning and inform the user that