[PATCH] D6762: [libcxx] Add <experimental/any> v2.

Eric Fiselier eric at efcs.ca
Wed Jul 22 14:36:10 PDT 2015


EricWF marked 2 inline comments as done.
EricWF added a comment.

@mclow.lists: I addressed your comment about the exception namespace. I'll wait for your response before moving forward.


================
Comment at: src/any.cpp:18
@@ +17,3 @@
+}
+
+_LIBCPP_END_NAMESPACE_LFTS
----------------
mclow.lists wrote:
> Because of arcane packaging issues, we put the exception classes in an unversioned namespace and the destructors in the the dylib as well.   See <optional> implementation.
> 
>       _LIBCPP_BEGIN_NAMESPACE_EXPERIMENTAL
>       const char* bad_any_cast::what() const _NOEXCEPT
>       {
>           return "bad any cast";
>       }
> 
>       bad_any_cast::~ bad_any_cast() _NOEXCEPT {}
>       _LIBCPP_END_NAMESPACE_EXPERIMENTAL
> 
> This will require a change in the header file as well
I don't think this makes sense for things in the LFTS. Unlike `std::__1` which is an implementation detail, the LFTS explicitly puts the exceptions in the inline namespace and putting `bad_any_cast` in `std::experimental` is non-conforming.

AFAIK there are two reasons we generally put exceptions in namespace `std` directly and not `std::__1` 

1. ABI stability. When `std::__1` changes to `std::__2` the exception types are still compatible.
2. libstdc++ compatibility. libc++'s exceptions should be ABI compatible with GCC's when possible.

ABI stability doesn't really make sense when thinking about TS's because they are not meant to be stable.

Also If we put `bad_any_cast` outside of `fundamentals_v1` we lose exception compatibility with libstdc++.

@mclow.lists: Do you still want to make this change?


http://reviews.llvm.org/D6762







More information about the cfe-commits mailing list