[libcxx-commits] [PATCH] D121213: [libc++] Enable modernize-use-equals-delete

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

In D121213#3369677 <https://reviews.llvm.org/D121213#3369677>, @philnik wrote:

> @EricWF And in what scenario would that change the ABI? If there are no copy- or move-constructors, then all are deleted by definition, and if there is one it isn't trivial.

@philnik: Eric is at least partly correct (and your original goalpost was wrong): this transformation can definitely change the `is_trivially_fooable` properties of the type. However, I haven't yet been able to get this to cause an actual calling-convention difference. https://godbolt.org/z/d17PM8dWc  It seems to me that the wording Eric quoted was probably designed precisely to //prevent// the breakage Eric is concerned about! It basically says, "Deleted SMFs don't affect triviality, //unless// you're using deleted SMFs to replicate the old C++98 trick of declare-but-not-defining //all// your SMFs. If //all// your SMFs are deleted, then we'll assume you still want to be non-trivial for the purposes of calls."
So I believe Eric is correct about the //possibility// of breakage here, and I think you should go look at the triviality-for-purposes-of-calls of all of the affected classes (i.e., don't just trust clang-tidy's mechanical change; go through it line by line). I predict you'll find that every affected class is either still non-trivial for other reasons (e.g. virtual dtor), or is a detail type like `__save_flags` where we don't care about its triviality. But yeah, Eric's convinced me that we shouldn't just //assume// this is safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121213