[libcxx-commits] [PATCH] D143071: [libc++] Add abi_tag on __exception_guard to avoid ODR violations

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

Thanks for looking into this. I agree with fixing this ASAP, however in the longer term we should simply encode the ABI-affecting properties of the library into the ABI tag. For example, whether the debug mode is enabled or not, whether exceptions are enabled, etc. Then that would apply library-wide. In order for that to work, we'd also need to drop `always_inline` and probably `exclude_from_explicit_instantiation` from those macros, which is a WIP.

To avoid making it look like this is the final solution, I would avoid introducing a macro in `__config` and I would simply use two differently-named classes.

LGTM w/ comments, also this needs to be cherry-picked onto `release/16.x`.



================
Comment at: libcxx/include/__utility/exception_guard.h:63
 template <class _Rollback>
 struct __exception_guard {
   __exception_guard() = delete;
----------------
Instead, let's define two classes, one is `__exception_guard_exceptions` and the other one is `__exception_guard_noexceptions`. Then we can have an alias to the right one.


================
Comment at: libcxx/test/libcxx/utilities/exception_guard.odr.sh.cpp:9
+
+// Make sure that __exception_guard is a different symbol when compiled with -fno-exceptions.
+
----------------



================
Comment at: libcxx/test/libcxx/utilities/exception_guard.odr.sh.cpp:11-12
+
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.except.o -g -Wno-private-header
+// RUN: %{cxx} %s %{flags} %{compile_flags} -c -o %t.noexcept.o -g -fno-exceptions -Wno-private-header
+// RUN: %{cxx} %{flags} %{link_flags} -o %t.exe %t.except.o %t.noexcept.o
----------------
Makes it easier to see what's going on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143071