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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik marked 2 inline comments as done.
philnik added inline comments.


================
Comment at: libcxx/include/__utility/exception_guard.h:126
 
-_LIBCPP_CTAD_SUPPORTED_FOR_TYPE(__exception_guard);
+_LIBCPP_CTAD_SUPPORTED_FOR_TYPE(__exception_guard_noexceptions);
+
----------------
dblaikie wrote:
> I guess the other changes in the patch to use `__make_exception_guard` are because CTAD can't be used through the alias? If that's true, maybe remove the CTAD support macros here - since it won't be used portably between exception/non-exception code anyway?
I expect us to revert part of this once Clang supports CTAD with aliases, so I'd just keep them since they will be added again later.


================
Comment at: libcxx/include/__utility/exception_guard.h:63
 template <class _Rollback>
 struct __exception_guard {
   __exception_guard() = delete;
----------------
dblaikie wrote:
> ldionne wrote:
> > 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.
> If they're going to have separate names with an alias - do they need to be/is it better that they not be preprocessor conditional, and only the definition of the alias would need the preprocessor conditionals? Or is it worth keeping the extra code out of the parsing/worth the extra preprocessor conditionality to do so?
I left them in the conditional to avoid accidentally depending on one of them directly. This should be caught in code review, but letting the compiler complain makes it more obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143071