[libcxx-commits] [PATCH] D65667: [libcxx] Avoid destructor call for error_category singletons

[Hubert Tong via Phabricator via libcxx-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: libcxx/src/system_error.cpp:232-238
 const error_category&
 system_category() _NOEXCEPT
 {
-    static __system_error_category s;
-    return s;
+    _LIBCPP_SAFE_STATIC_AFTER_CXX11
+    static SystemErrorHelper s;
+    return s.system_error_category.new_system_error_category;
 }
----------------
Quuxplusone wrote:
> cebowleratibm wrote:
> > Quuxplusone wrote:
> > > After rebasing, I would expect this would end up looking like
> > > ```
> > > const error_category&
> > > system_category() _NOEXCEPT
> > > {
> > >     union U {
> > >         __system_error_category s;
> > >         _LIBCPP_CONSTEXPR explicit U() : s() {}
> > >         ~U() {}  // Too bad this can't be trivial... or can it?
> > >     };
> > >     static _LIBCPP_CONSTINIT U u;
> > >     return u.s;
> > > }
> > > ```
> > > i.e. a very small diff against the left-hand side.
> > I think the constexpr version will hit this error:
> > 
> > error: constexpr variable cannot have non-literal type 'const U'
> There's no constexpr variable here, but you're right, the constexpr //constructor// hits
> ```
> src/system_error.cpp:212:37: error: constexpr constructor never produces a constant expression [-Winvalid-constexpr]
>          _LIBCPP_CONSTEXPR explicit U() : s() {}
>                                     ^
> ```
> That's also a problem with the current PR. (Maybe that Clang diagnostic didn't exist when the PR was originally made; but it does exist now; I just tried it locally.)
We probably want to apply `[[no_destroy]]` in conjunction with the above. That is, guarantee the functionality with the above and suppress the registration of the destructor for compilers that support it.


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

https://reviews.llvm.org/D65667