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

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Mar 1 07:07:43 PST 2022


Quuxplusone 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;
 }
----------------
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.)


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

https://reviews.llvm.org/D65667



More information about the libcxx-commits mailing list