[libcxx-commits] [PATCH] D124346: [libc++] Complete the implementation of N4190

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D124346#3571465 <https://reviews.llvm.org/D124346#3571465>, @EricWF wrote:

> I'm pretty sure this change results in cases where, under ABI v2, C++14 and C++17 produce different ABI's. Am I mistaken?

Oh, I missed that you said "Under ABI V2". Yes, I think that's correct, actually. It's tricky because we generally don't think about ABI compatibility at all when we start enabling these ABI macros, however I do agree that if we are to ever consider ABI v2 as stable, we need to. It's doubly tricky because I don't know what's the state of other ABI macros w.r.t. standard versions -- have we done other things that would produce a different ABI between C++14 and C++17 in ABI v2? That's possible, we'd need to check.

Concretely for this patch, I think it only means that we need to drop the `_LIBCPP_ABI_NO_BINDER_BASES` macro. Furthermore, I think it gives us a new criterion for creating these ABI macros in the first place: An ABI macro must not result in a different ABI for different Standard versions, period. I'll document that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124346