Skip to content

Commit 1b56480

Browse files
ldionnememfrob
authored andcommitted
[libc++] Refactor how basic_string and vector hoist exception-throwing functions
In basic_string and vector, we've been encapsulating all exception throwing code paths in helper functions of a base class, which are defined in the compiled library. For example, __vector_base_common defines two methods, __throw_length_error() and __throw_out_of_range(), and the class is externally instantiated in the library. This was done a long time ago, but after investigating, I believe the goal of the current design was to: 1. Encapsulate the code to throw an exception (which is non-trivial) in an externally-defined function so that the important code paths that call it (e.g. vector::at) are free from that code. Basically, the intent is for the "hot" code path to contain a single conditional jump (based on checking the error condition) to an externally-defined function, which handles all the exception-throwing business. 2. Avoid defining this exception-throwing function once per instantiation of the class template. In other words, we want a single copy of __throw_length_error even if we have vector<int>, vector<char>, etc. 3. Encapsulate the passing of the container-specific string (i.e. "vector" and "basic_string") to the underlying exception-throwing function so that object files don't contain those duplicated string literals. For example, we'd like to have a single "vector" string literal for passing to `std::__throw_length_error` in the library, instead of having one per translation unit. However, the way this is achieved right now has two problems: - Using a base class and exporting it is really weird - I've been confused about this ever since I first saw it. It's just a really unusual way of achieving the above goals. Also, it's made even worse by the fact that the definitions of __throw_length_error and __throw_out_of_range appear in the headers despite always being intended to be defined in the compiled library (via the extern template instantiation). - We end up exporting those functions as weak symbols, which isn't great for load times. Instead, it would be better to export those as strong symbols from the library. This patch fixes those issues while retaining ABI compatibility (e.g. we still export the exact same symbols as before). Note that we need to keep the base classes as-is to avoid breaking the ABI of someone who might inherit from std::basic_string or std::vector. Differential Revision: https://reviews.llvm.org/D111173
1 parent b17a74b commit 1b56480

File tree

4 files changed

+30
-47
lines changed

4 files changed

+30
-47
lines changed

libcxx/include/string

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -613,28 +613,14 @@ operator+(const basic_string<_CharT, _Traits, _Allocator>& __x, _CharT __y);
613613
_LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS string operator+<char, char_traits<char>, allocator<char> >(char const*, string const&))
614614

615615
template <bool>
616-
class _LIBCPP_TEMPLATE_VIS __basic_string_common
617-
{
618-
protected:
619-
_LIBCPP_NORETURN void __throw_length_error() const;
620-
_LIBCPP_NORETURN void __throw_out_of_range() const;
621-
};
622-
623-
template <bool __b>
624-
void
625-
__basic_string_common<__b>::__throw_length_error() const
626-
{
627-
_VSTD::__throw_length_error("basic_string");
628-
}
616+
struct __basic_string_common;
629617

630-
template <bool __b>
631-
void
632-
__basic_string_common<__b>::__throw_out_of_range() const
633-
{
634-
_VSTD::__throw_out_of_range("basic_string");
635-
}
636-
637-
_LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __basic_string_common<true>)
618+
template <>
619+
struct __basic_string_common<true> {
620+
// Both are defined in string.cpp
621+
_LIBCPP_NORETURN _LIBCPP_EXPORTED_FROM_ABI void __throw_length_error() const;
622+
_LIBCPP_NORETURN _LIBCPP_EXPORTED_FROM_ABI void __throw_out_of_range() const;
623+
};
638624

639625
template <class _Iter>
640626
struct __string_is_trivial_iterator : public false_type {};
@@ -688,7 +674,7 @@ class
688674
_LIBCPP_PREFERRED_NAME(u32string)
689675
#endif
690676
basic_string
691-
: private __basic_string_common<true>
677+
: private __basic_string_common<true> // This base class is historical, but it needs to remain for ABI compatibility
692678
{
693679
public:
694680
typedef basic_string __self;

libcxx/include/vector

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -302,33 +302,18 @@ _LIBCPP_PUSH_MACROS
302302
_LIBCPP_BEGIN_NAMESPACE_STD
303303

304304
template <bool>
305-
class _LIBCPP_TEMPLATE_VIS __vector_base_common
306-
{
307-
protected:
308-
_LIBCPP_INLINE_VISIBILITY __vector_base_common() {}
309-
_LIBCPP_NORETURN void __throw_length_error() const;
310-
_LIBCPP_NORETURN void __throw_out_of_range() const;
311-
};
312-
313-
template <bool __b>
314-
void
315-
__vector_base_common<__b>::__throw_length_error() const
316-
{
317-
_VSTD::__throw_length_error("vector");
318-
}
319-
320-
template <bool __b>
321-
void
322-
__vector_base_common<__b>::__throw_out_of_range() const
323-
{
324-
_VSTD::__throw_out_of_range("vector");
325-
}
305+
struct __vector_base_common;
326306

327-
_LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS __vector_base_common<true>)
307+
template <>
308+
struct __vector_base_common<true> {
309+
// Both are defined in vector.cpp
310+
_LIBCPP_NORETURN _LIBCPP_EXPORTED_FROM_ABI void __throw_length_error() const;
311+
_LIBCPP_NORETURN _LIBCPP_EXPORTED_FROM_ABI void __throw_out_of_range() const;
312+
};
328313

329314
template <class _Tp, class _Allocator>
330315
class __vector_base
331-
: protected __vector_base_common<true>
316+
: protected __vector_base_common<true> // This base class is historical, but it needs to remain for ABI compatibility
332317
{
333318
public:
334319
typedef _Allocator allocator_type;

libcxx/src/string.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,13 @@
1818

1919
_LIBCPP_BEGIN_NAMESPACE_STD
2020

21-
template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __basic_string_common<true>;
21+
void __basic_string_common<true>::__throw_length_error() const {
22+
_VSTD::__throw_length_error("basic_string");
23+
}
24+
25+
void __basic_string_common<true>::__throw_out_of_range() const {
26+
_VSTD::__throw_out_of_range("basic_string");
27+
}
2228

2329
#define _LIBCPP_EXTERN_TEMPLATE_DEFINE(...) template __VA_ARGS__;
2430
#ifdef _LIBCPP_ABI_STRING_OPTIMIZED_EXTERNAL_INSTANTIATION

libcxx/src/vector.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@
1010

1111
_LIBCPP_BEGIN_NAMESPACE_STD
1212

13-
template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __vector_base_common<true>;
13+
void __vector_base_common<true>::__throw_length_error() const {
14+
_VSTD::__throw_length_error("vector");
15+
}
16+
17+
void __vector_base_common<true>::__throw_out_of_range() const {
18+
_VSTD::__throw_out_of_range("vector");
19+
}
1420

1521
_LIBCPP_END_NAMESPACE_STD

0 commit comments

Comments
 (0)