[libcxx-commits] [PATCH] D63744: In the libc++ unstable ABI, use [[no_unique_address]] instead of __compressed_pair when available.

[James Y Knight via Phabricator via libcxx-commits]

jyknight added a comment.

IMO, this is a good idea, and should be committed evendespite the addition of ifdefs in the implementation.

We'd definitely want to do this in a brand new ABI, and there's not really any better way to accomplish it now.



================
Comment at: libcxx/include/__config:122
+#if defined(_LIBCPP_ABI_UNSTABLE)
+#  if __has_cpp_attribute(no_unique_address)
+#    define _LIBCPP_ABI_USE_NO_UNIQUE_ADDRESS 1
----------------
IMO, the ABI shouldn't change based on a compiler feature-test, even in unstable-abi mode.

But I don't see why it'd be a problem to just enable it by default with _LIBCPP_ABI_UNSTABLE, especially if, per the request above, there's a way to explicitly disable it again.

Then, if _LIBCPP_ABI_USE_NO_UNIQUE_ADDRESS is enabled, it should just emit an #error when the compiler fails to support the attribute.


================
Comment at: libcxx/include/memory:2611
+    return *__ptr_
+#ifndef _LIBCPP_ABI_USE_NO_UNIQUE_ADDRESS
+        .first()
----------------
ifdef in the middle of an expression isn't worth it. Just repeat the rest of the statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63744