[libc++] Remove _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS

[philnik777]

This macro isn't required if we define all the functions inline. In fact, quite a few of the marked functions have already been inlined.

This patch basically only moves code around and adds _LIBCPP_HIDE_FROM_ABI to the places where it's been missing so far.

This also removes inlining hints, since it dropps inline in some places, but that shouldn't make much of a difference. The functions tend to be either really small, so should be inlined anyways, or are big enough that they shouldn't be inlined even with an inlinehint.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This macro isn't required if we define all the functions inline. In fact, quite a few of the marked functions have already been inlined.

This patch basically only moves code around and adds _LIBCPP_HIDE_FROM_ABI to the places where it's been missing so far.

---

Patch is 84.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/111964.diff

12 Files Affected:

• (modified) libcxx/.clang-format (-1)
• (modified) libcxx/docs/DesignDocs/VisibilityMacros.rst (-29)
• (modified) libcxx/include/__condition_variable/condition_variable.h (+85-97)
• (modified) libcxx/include/__config (-8)
• (modified) libcxx/include/__locale (+14-18)
• (modified) libcxx/include/__thread/thread.h (+61-66)
• (modified) libcxx/include/condition_variable (+30-36)
• (modified) libcxx/include/future (+21-27)
• (modified) libcxx/include/locale (+263-284)
• (modified) libcxx/include/mutex (+36-40)
• (modified) libcxx/include/shared_mutex (+48-52)
• (modified) libcxx/include/string (+114-191)

diff --git a/libcxx/.clang-format b/libcxx/.clang-format
index f548119652c19..0d9b2806987d9 100644
--- a/libcxx/.clang-format
+++ b/libcxx/.clang-format
@@ -38,7 +38,6 @@ AttributeMacros: [
                   '_LIBCPP_HIDDEN',
                   '_LIBCPP_HIDE_FROM_ABI_AFTER_V1',
                   '_LIBCPP_HIDE_FROM_ABI',
-                  '_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS',
                   '_LIBCPP_NO_SANITIZE',
                   '_LIBCPP_NO_UNIQUE_ADDRESS',
                   '_LIBCPP_NOALIAS',
diff --git a/libcxx/docs/DesignDocs/VisibilityMacros.rst b/libcxx/docs/DesignDocs/VisibilityMacros.rst
index 83a9a62942bc9..e37e712014c08 100644
--- a/libcxx/docs/DesignDocs/VisibilityMacros.rst
+++ b/libcxx/docs/DesignDocs/VisibilityMacros.rst
@@ -105,35 +105,6 @@ Visibility Macros
   the extern template declaration) as exported on Windows, as discussed above.
   On all other platforms, this macro has an empty definition.
 
-**_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS**
-  Mark a symbol as hidden so it will not be exported from shared libraries. This
-  is intended specifically for method templates of either classes marked with
-  `_LIBCPP_TYPE_VIS` or classes with an extern template instantiation
-  declaration marked with `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS`.
-
-  When building libc++ with hidden visibility, we want explicit template
-  instantiations to export members, which is consistent with existing Windows
-  behavior. We also want classes annotated with `_LIBCPP_TYPE_VIS` to export
-  their members, which is again consistent with existing Windows behavior.
-  Both these changes are necessary for clients to be able to link against a
-  libc++ DSO built with hidden visibility without encountering missing symbols.
-
-  An unfortunate side effect, however, is that method templates of classes
-  either marked `_LIBCPP_TYPE_VIS` or with extern template instantiation
-  declarations marked with `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` also get default
-  visibility when instantiated. These methods are often implicitly instantiated
-  inside other libraries which use the libc++ headers, and will therefore end up
-  being exported from those libraries, since those implicit instantiations will
-  receive default visibility. This is not acceptable for libraries that wish to
-  control their visibility, and led to PR30642.
-
-  Consequently, all such problematic method templates are explicitly marked
-  either hidden (via this macro) or inline, so that they don't leak into client
-  libraries. The problematic methods were found by running
-  `bad-visibility-finder <https://github.com/smeenai/bad-visibility-finder>`_
-  against the libc++ headers after making `_LIBCPP_TYPE_VIS` and
-  `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` expand to default visibility.
-
 Links
 =====
 
diff --git a/libcxx/include/__condition_variable/condition_variable.h b/libcxx/include/__condition_variable/condition_variable.h
index 82ecb804669e6..1e8edd5dcb009 100644
--- a/libcxx/include/__condition_variable/condition_variable.h
+++ b/libcxx/include/__condition_variable/condition_variable.h
@@ -39,60 +39,6 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 _LIBCPP_DECLARE_STRONG_ENUM(cv_status){no_timeout, timeout};
 _LIBCPP_DECLARE_STRONG_ENUM_EPILOG(cv_status)
 
-class _LIBCPP_EXPORTED_FROM_ABI condition_variable {
-  __libcpp_condvar_t __cv_ = _LIBCPP_CONDVAR_INITIALIZER;
-
-public:
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR condition_variable() _NOEXCEPT = default;
-
-#  if _LIBCPP_HAS_TRIVIAL_CONDVAR_DESTRUCTION
-  ~condition_variable() = default;
-#  else
-  ~condition_variable();
-#  endif
-
-  condition_variable(const condition_variable&)            = delete;
-  condition_variable& operator=(const condition_variable&) = delete;
-
-  void notify_one() _NOEXCEPT;
-  void notify_all() _NOEXCEPT;
-
-  void wait(unique_lock<mutex>& __lk) _NOEXCEPT;
-  template <class _Predicate>
-  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS void wait(unique_lock<mutex>& __lk, _Predicate __pred);
-
-  template <class _Clock, class _Duration>
-  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS cv_status
-  wait_until(unique_lock<mutex>& __lk, const chrono::time_point<_Clock, _Duration>& __t);
-
-  template <class _Clock, class _Duration, class _Predicate>
-  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS bool
-  wait_until(unique_lock<mutex>& __lk, const chrono::time_point<_Clock, _Duration>& __t, _Predicate __pred);
-
-  template <class _Rep, class _Period>
-  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS cv_status
-  wait_for(unique_lock<mutex>& __lk, const chrono::duration<_Rep, _Period>& __d);
-
-  template <class _Rep, class _Period, class _Predicate>
-  bool _LIBCPP_HIDE_FROM_ABI
-  wait_for(unique_lock<mutex>& __lk, const chrono::duration<_Rep, _Period>& __d, _Predicate __pred);
-
-  typedef __libcpp_condvar_t* native_handle_type;
-  _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__cv_; }
-
-private:
-  void
-  __do_timed_wait(unique_lock<mutex>& __lk, chrono::time_point<chrono::system_clock, chrono::nanoseconds>) _NOEXCEPT;
-#  if _LIBCPP_HAS_COND_CLOCKWAIT
-  _LIBCPP_HIDE_FROM_ABI void
-  __do_timed_wait(unique_lock<mutex>& __lk, chrono::time_point<chrono::steady_clock, chrono::nanoseconds>) _NOEXCEPT;
-#  endif
-  template <class _Clock>
-  _LIBCPP_HIDE_FROM_ABI void
-  __do_timed_wait(unique_lock<mutex>& __lk, chrono::time_point<_Clock, chrono::nanoseconds>) _NOEXCEPT;
-};
-#endif // _LIBCPP_HAS_THREADS
-
 template <class _Rep, class _Period, __enable_if_t<is_floating_point<_Rep>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI chrono::nanoseconds __safe_nanosecond_cast(chrono::duration<_Rep, _Period> __d) {
   using namespace chrono;
@@ -140,64 +86,106 @@ inline _LIBCPP_HIDE_FROM_ABI chrono::nanoseconds __safe_nanosecond_cast(chrono::
   return nanoseconds(__result);
 }
 
-#if _LIBCPP_HAS_THREADS
-template <class _Predicate>
-void condition_variable::wait(unique_lock<mutex>& __lk, _Predicate __pred) {
-  while (!__pred())
-    wait(__lk);
-}
+class _LIBCPP_EXPORTED_FROM_ABI condition_variable {
+  __libcpp_condvar_t __cv_ = _LIBCPP_CONDVAR_INITIALIZER;
 
-template <class _Clock, class _Duration>
-cv_status condition_variable::wait_until(unique_lock<mutex>& __lk, const chrono::time_point<_Clock, _Duration>& __t) {
-  using namespace chrono;
-  using __clock_tp_ns = time_point<_Clock, nanoseconds>;
+public:
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR condition_variable() _NOEXCEPT = default;
 
-  typename _Clock::time_point __now = _Clock::now();
-  if (__t <= __now)
-    return cv_status::timeout;
+#  if _LIBCPP_HAS_TRIVIAL_CONDVAR_DESTRUCTION
+  ~condition_variable() = default;
+#  else
+  ~condition_variable();
+#  endif
 
-  __clock_tp_ns __t_ns = __clock_tp_ns(std::__safe_nanosecond_cast(__t.time_since_epoch()));
+  condition_variable(const condition_variable&)            = delete;
+  condition_variable& operator=(const condition_variable&) = delete;
 
-  __do_timed_wait(__lk, __t_ns);
-  return _Clock::now() < __t ? cv_status::no_timeout : cv_status::timeout;
-}
+  void notify_one() _NOEXCEPT;
+  void notify_all() _NOEXCEPT;
+
+  void wait(unique_lock<mutex>& __lk) _NOEXCEPT;
 
-template <class _Clock, class _Duration, class _Predicate>
-bool condition_variable::wait_until(
-    unique_lock<mutex>& __lk, const chrono::time_point<_Clock, _Duration>& __t, _Predicate __pred) {
-  while (!__pred()) {
-    if (wait_until(__lk, __t) == cv_status::timeout)
-      return __pred();
+  template <class _Predicate>
+  _LIBCPP_HIDE_FROM_ABI void wait(unique_lock<mutex>& __lk, _Predicate __pred) {
+    while (!__pred())
+      wait(__lk);
   }
-  return true;
-}
 
-template <class _Rep, class _Period>
-cv_status condition_variable::wait_for(unique_lock<mutex>& __lk, const chrono::duration<_Rep, _Period>& __d) {
-  using namespace chrono;
-  if (__d <= __d.zero())
-    return cv_status::timeout;
-  using __ns_rep                   = nanoseconds::rep;
-  steady_clock::time_point __c_now = steady_clock::now();
+  template <class _Clock, class _Duration>
+  _LIBCPP_HIDE_FROM_ABI cv_status
+  wait_until(unique_lock<mutex>& __lk, const chrono::time_point<_Clock, _Duration>& __t) {
+    using namespace chrono;
+    using __clock_tp_ns = time_point<_Clock, nanoseconds>;
+
+    typename _Clock::time_point __now = _Clock::now();
+    if (__t <= __now)
+      return cv_status::timeout;
+
+    __clock_tp_ns __t_ns = __clock_tp_ns(std::__safe_nanosecond_cast(__t.time_since_epoch()));
+
+    __do_timed_wait(__lk, __t_ns);
+    return _Clock::now() < __t ? cv_status::no_timeout : cv_status::timeout;
+  }
+
+  template <class _Clock, class _Duration, class _Predicate>
+  _LIBCPP_HIDE_FROM_ABI bool
+  wait_until(unique_lock<mutex>& __lk, const chrono::time_point<_Clock, _Duration>& __t, _Predicate __pred) {
+    while (!__pred()) {
+      if (wait_until(__lk, __t) == cv_status::timeout)
+        return __pred();
+    }
+    return true;
+  }
+
+  template <class _Rep, class _Period>
+  _LIBCPP_HIDE_FROM_ABI cv_status wait_for(unique_lock<mutex>& __lk, const chrono::duration<_Rep, _Period>& __d) {
+    using namespace chrono;
+    if (__d <= __d.zero())
+      return cv_status::timeout;
+    using __ns_rep                   = nanoseconds::rep;
+    steady_clock::time_point __c_now = steady_clock::now();
 
 #  if _LIBCPP_HAS_COND_CLOCKWAIT
-  using __clock_tp_ns     = time_point<steady_clock, nanoseconds>;
-  __ns_rep __now_count_ns = std::__safe_nanosecond_cast(__c_now.time_since_epoch()).count();
+    using __clock_tp_ns     = time_point<steady_clock, nanoseconds>;
+    __ns_rep __now_count_ns = std::__safe_nanosecond_cast(__c_now.time_since_epoch()).count();
 #  else
-  using __clock_tp_ns     = time_point<system_clock, nanoseconds>;
-  __ns_rep __now_count_ns = std::__safe_nanosecond_cast(system_clock::now().time_since_epoch()).count();
+    using __clock_tp_ns     = time_point<system_clock, nanoseconds>;
+    __ns_rep __now_count_ns = std::__safe_nanosecond_cast(system_clock::now().time_since_epoch()).count();
 #  endif
 
-  __ns_rep __d_ns_count = std::__safe_nanosecond_cast(__d).count();
+    __ns_rep __d_ns_count = std::__safe_nanosecond_cast(__d).count();
 
-  if (__now_count_ns > numeric_limits<__ns_rep>::max() - __d_ns_count) {
-    __do_timed_wait(__lk, __clock_tp_ns::max());
-  } else {
-    __do_timed_wait(__lk, __clock_tp_ns(nanoseconds(__now_count_ns + __d_ns_count)));
+    if (__now_count_ns > numeric_limits<__ns_rep>::max() - __d_ns_count) {
+      __do_timed_wait(__lk, __clock_tp_ns::max());
+    } else {
+      __do_timed_wait(__lk, __clock_tp_ns(nanoseconds(__now_count_ns + __d_ns_count)));
+    }
+
+    return steady_clock::now() - __c_now < __d ? cv_status::no_timeout : cv_status::timeout;
   }
 
-  return steady_clock::now() - __c_now < __d ? cv_status::no_timeout : cv_status::timeout;
-}
+  template <class _Rep, class _Period, class _Predicate>
+  bool _LIBCPP_HIDE_FROM_ABI
+  wait_for(unique_lock<mutex>& __lk, const chrono::duration<_Rep, _Period>& __d, _Predicate __pred);
+
+  typedef __libcpp_condvar_t* native_handle_type;
+  _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__cv_; }
+
+private:
+  void
+  __do_timed_wait(unique_lock<mutex>& __lk, chrono::time_point<chrono::system_clock, chrono::nanoseconds>) _NOEXCEPT;
+#  if _LIBCPP_HAS_COND_CLOCKWAIT
+  _LIBCPP_HIDE_FROM_ABI void
+  __do_timed_wait(unique_lock<mutex>& __lk, chrono::time_point<chrono::steady_clock, chrono::nanoseconds>) _NOEXCEPT;
+#  endif
+  template <class _Clock>
+  _LIBCPP_HIDE_FROM_ABI void
+  __do_timed_wait(unique_lock<mutex>& __lk, chrono::time_point<_Clock, chrono::nanoseconds>) _NOEXCEPT;
+};
+#endif // _LIBCPP_HAS_THREADS
+
+#if _LIBCPP_HAS_THREADS
 
 template <class _Rep, class _Period, class _Predicate>
 inline bool
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 070298301b0d3..c7bd9d0b6130c 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -388,7 +388,6 @@ typedef __char32_t char32_t;
 #    endif
 
 #    define _LIBCPP_HIDDEN
-#    define _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
 #    define _LIBCPP_TEMPLATE_VIS
 #    define _LIBCPP_TEMPLATE_DATA_VIS
 #    define _LIBCPP_TYPE_VISIBILITY_DEFAULT
@@ -412,13 +411,6 @@ typedef __char32_t char32_t;
 #      define _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_VISIBILITY("default")
 #    endif
 
-#    if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
-// The inline should be removed once PR32114 is resolved
-#      define _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS inline _LIBCPP_HIDDEN
-#    else
-#      define _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
-#    endif
-
 // GCC doesn't support the type_visibility attribute, so we have to keep the visibility attribute on templates
 #    if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) && !__has_attribute(__type_visibility__)
 #      define _LIBCPP_TEMPLATE_VIS __attribute__((__visibility__("default")))
diff --git a/libcxx/include/__locale b/libcxx/include/__locale
index 5ae3228989749..473641bb3e1a9 100644
--- a/libcxx/include/__locale
+++ b/libcxx/include/__locale
@@ -42,6 +42,9 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 class _LIBCPP_EXPORTED_FROM_ABI locale;
 
+template <class _CharT>
+class collate;
+
 template <class _Facet>
 _LIBCPP_HIDE_FROM_ABI bool has_facet(const locale&) _NOEXCEPT;
 
@@ -81,7 +84,12 @@ public:
   const locale& operator=(const locale&) _NOEXCEPT;
 
   template <class _Facet>
-  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS locale combine(const locale&) const;
+  _LIBCPP_HIDE_FROM_ABI locale combine(const locale& __other) const {
+    if (!std::has_facet<_Facet>(__other))
+      __throw_runtime_error("locale::combine: locale missing facet");
+
+    return locale(*this, std::addressof(const_cast<_Facet&>(std::use_facet<_Facet>(__other))));
+  }
 
   // locale operations:
   string name() const;
@@ -90,8 +98,11 @@ public:
   _LIBCPP_HIDE_FROM_ABI bool operator!=(const locale& __y) const { return !(*this == __y); }
 #endif
   template <class _CharT, class _Traits, class _Allocator>
-  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS bool
-  operator()(const basic_string<_CharT, _Traits, _Allocator>&, const basic_string<_CharT, _Traits, _Allocator>&) const;
+  _LIBCPP_HIDE_FROM_ABI bool operator()(const basic_string<_CharT, _Traits, _Allocator>& __x,
+                                        const basic_string<_CharT, _Traits, _Allocator>& __y) const {
+    return std::use_facet<std::collate<_CharT> >(*this).compare(
+               __x.data(), __x.data() + __x.size(), __y.data(), __y.data() + __y.size()) < 0;
+  }
 
   // global locale objects:
   static locale global(const locale&);
@@ -152,14 +163,6 @@ inline _LIBCPP_HIDE_FROM_ABI locale::locale(const locale& __other, _Facet* __f)
   __install_ctor(__other, __f, __f ? __f->id.__get() : 0);
 }
 
-template <class _Facet>
-locale locale::combine(const locale& __other) const {
-  if (!std::has_facet<_Facet>(__other))
-    std::__throw_runtime_error("locale::combine: locale missing facet");
-
-  return locale(*this, std::addressof(const_cast<_Facet&>(std::use_facet<_Facet>(__other))));
-}
-
 template <class _Facet>
 inline _LIBCPP_HIDE_FROM_ABI bool has_facet(const locale& __l) _NOEXCEPT {
   return __l.has_facet(_Facet::id);
@@ -286,13 +289,6 @@ protected:
 };
 #endif
 
-template <class _CharT, class _Traits, class _Allocator>
-bool locale::operator()(const basic_string<_CharT, _Traits, _Allocator>& __x,
-                        const basic_string<_CharT, _Traits, _Allocator>& __y) const {
-  return std::use_facet<std::collate<_CharT> >(*this).compare(
-             __x.data(), __x.data() + __x.size(), __y.data(), __y.data() + __y.size()) < 0;
-}
-
 // template <class charT> class ctype
 
 class _LIBCPP_EXPORTED_FROM_ABI ctype_base {
diff --git a/libcxx/include/__thread/thread.h b/libcxx/include/__thread/thread.h
index 5e22013dec49f..5aae50710bbfc 100644
--- a/libcxx/include/__thread/thread.h
+++ b/libcxx/include/__thread/thread.h
@@ -152,47 +152,6 @@ operator<<(basic_ostream<_CharT, _Traits>& __os, __thread_id __id) {
 }
 #  endif // _LIBCPP_HAS_LOCALIZATION
 
-class _LIBCPP_EXPORTED_FROM_ABI thread {
-  __libcpp_thread_t __t_;
-
-  thread(const thread&);
-  thread& operator=(const thread&);
-
-public:
-  typedef __thread_id id;
-  typedef __libcpp_thread_t native_handle_type;
-
-  _LIBCPP_HIDE_FROM_ABI thread() _NOEXCEPT : __t_(_LIBCPP_NULL_THREAD) {}
-#  ifndef _LIBCPP_CXX03_LANG
-  template <class _Fp, class... _Args, __enable_if_t<!is_same<__remove_cvref_t<_Fp>, thread>::value, int> = 0>
-  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS explicit thread(_Fp&& __f, _Args&&... __args);
-#  else // _LIBCPP_CXX03_LANG
-  template <class _Fp>
-  _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS explicit thread(_Fp __f);
-#  endif
-  ~thread();
-
-  _LIBCPP_HIDE_FROM_ABI thread(thread&& __t) _NOEXCEPT : __t_(__t.__t_) { __t.__t_ = _LIBCPP_NULL_THREAD; }
-
-  _LIBCPP_HIDE_FROM_ABI thread& operator=(thread&& __t) _NOEXCEPT {
-    if (!__libcpp_thread_isnull(&__t_))
-      terminate();
-    __t_     = __t.__t_;
-    __t.__t_ = _LIBCPP_NULL_THREAD;
-    return *this;
-  }
-
-  _LIBCPP_HIDE_FROM_ABI void swap(thread& __t) _NOEXCEPT { std::swap(__t_, __t.__t_); }
-
-  _LIBCPP_HIDE_FROM_ABI bool joinable() const _NOEXCEPT { return !__libcpp_thread_isnull(&__t_); }
-  void join();
-  void detach();
-  _LIBCPP_HIDE_FROM_ABI id get_id() const _NOEXCEPT { return __libcpp_thread_get_id(&__t_); }
-  _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() _NOEXCEPT { return __t_; }
-
-  static unsigned hardware_concurrency() _NOEXCEPT;
-};
-
 #  ifndef _LIBCPP_CXX03_LANG
 
 template <class _TSp, class _Fp, class... _Args, size_t... _Indices>
@@ -210,19 +169,6 @@ _LIBCPP_HIDE_FROM_ABI void* __thread_proxy(void* __vp) {
   return nullptr;
 }
 
-template <class _Fp, class... _Args, __enable_if_t<!is_same<__remove_cvref_t<_Fp>, thread>::value, int> >
-thread::thread(_Fp&& __f, _Args&&... __args) {
-  typedef unique_ptr<__thread_struct> _TSPtr;
-  _TSPtr __tsp(new __thread_struct);
-  typedef tuple<_TSPtr, __decay_t<_Fp>, __decay_t<_Args>...> _Gp;
-  unique_ptr<_Gp> __p(new _Gp(std::move(__tsp), std::forward<_Fp>(__f), std::forward<_Args>(__args)...));
-  int __ec = std::__libcpp_thread_create(&__t_, std::addressof(__thread_proxy<_Gp>), __p.get());
-  if (__ec == 0)
-    __p.release();
-  else
-    std::__throw_system_error(__ec, "thread constructor failed");
-}
-
 #  else // _LIBCPP_CXX03_LANG
 
 template <class _Fp>
@@ -243,20 +189,69 @@ _LIBCPP_HIDE_FROM_ABI void* __thread_proxy_cxx03(void* __vp) {
   return nullptr;
 }
 
-template <class _Fp>
-thread::thread(_Fp __f) {
-  typedef __thread_invoke_pair<_Fp> _InvokePair;
-  typedef unique_ptr<_InvokePair> _PairPtr;
-  _PairPtr __pp(new _InvokePair(__f));
-  int __ec = std::__libcpp_thread_create(&__t_, &__thread_proxy_cxx03<_InvokePair>, __pp.get());
-  if (__ec == 0)
-    __pp.release();
-  else
-    std::__throw_system_error(__ec, "thread constructor failed");
-}
-
 #  endif // _LIBCPP_CXX03_LANG
 
+class _LIBCPP_EXPORTED_FROM_ABI thread {
+  __libcpp_thread_t __t_;
+
+  thread(const thread&);
+  thread& operator=(const thread&);
+
+public:
+  typedef __thread_id id;
+  typedef __libcpp_thread_t native_handle_type;
+
+  _LIBCPP_HIDE_FROM_ABI thread() _NOEXCEPT : __t_(_LIBCPP_NULL_THREAD) {}
+
+#  ifndef _LIBCPP_CXX03_LANG
+  template <class _Fp, class... _Args, __enable_if_t<!is_same<__remove_cvref_t<_Fp>, thread>::value, int> = 0>
+  _LIBCPP_HIDE_FROM_ABI explicit thread(_Fp&& __f, _Args&&... __args) {
+    typedef unique_ptr<__thread_struct> _TSPtr;
+    _TSPtr __tsp(new __thread_struct);
+    typedef tuple<_TSPtr, __decay_t<_Fp>, __decay_t<_Args>...> _Gp;
+    unique_ptr<_Gp> __p(new _Gp(std::move(__tsp), std::forward<_Fp>(__f), std::forward<_Args>(__args)...));
+    int __ec = std::__libcpp_thread_create(&__t_, std::addressof(__thread_proxy<_Gp>), __p.get());
+    if (__ec == 0)
+      __p.release();
+    else
+      __throw_system_error(__ec, "thread constructor failed");
+  }
+#  else // _LIBCPP_CXX03_LANG
+  template <class _Fp>
+  _LIBCPP_HIDE_FROM_ABI explicit thread(_Fp __f) {
+    typedef __thread_invoke_pair<_Fp> _InvokePair;
+    typedef unique_ptr<_InvokePair> _PairPtr;
+    _PairPtr __pp(new _InvokePair(__f));
+    int __ec = std::__libcpp_thread_create(&__t_, &__thread_proxy_cxx03<_InvokePair>, __pp.get());
+    if (__ec...
[truncated]

[ldionne]

This technically removes the inline keyword from the affected functions. Like we looked into a while back, it turns out that inline is used as an actual inlining hint by the compiler (contrary to what we've been taught in recent years by e.g. Chandler).

I am not too concerned about that, we looked through the functions and they are either very small (e.g. string functions) and should be inlined no matter what, or they are pretty large (e.g. locale) and shouldn't get inlined either way. But please call out this change explicitly in the commit message to make it easier to bisect in case someone needs to investigate a performance regression.

Also, just for due diligence, please run our string microbenchmarks before and after. I really don't think there will be any difference, but let's just make sure that we don't notice anything odd.