diff mbox series

[committed] libstdc++: Do not include <system_error> in concurrency headers

Message ID 20230113001546.944147-1-jwakely@redhat.com
State Repeat Merge
Headers
Series [committed] libstdc++: Do not include <system_error> in concurrency headers |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jonathan Wakely Jan. 13, 2023, 12:15 a.m. UTC 
  Tested x86_64-linux and powerpc64le-linux. Pushed to trunk.

-- >8 --

The <condition_variable>, <mutex>, and <shared_mutex> headers use
std::errc constants, but don't use std::system_error itself. They only
use the __throw_system_error(int) function, which is defined in
<bits/functexcept.h>.

By including the header for the errc constants instead of the whole of
<system_error> we avoid depending on the whole std::string definition.

libstdc++-v3/ChangeLog:

	* include/bits/std_mutex.h: Remove <system_error> include.
	* include/std/condition_variable: Add <bits/error_constants.h>
	include.
	* include/std/mutex: Likewise.
	* include/std/shared_mutex: Likewise.
---
 libstdc++-v3/include/bits/std_mutex.h       | 1 -
 libstdc++-v3/include/std/condition_variable | 3 ++-
 libstdc++-v3/include/std/mutex              | 2 +-
 libstdc++-v3/include/std/shared_mutex       | 1 +
 4 files changed, 4 insertions(+), 3 deletions(-)

Comments

Rainer Orth Jan. 13, 2023, 3:08 p.m. UTC | #1 
Hi Jonathan,

> The <condition_variable>, <mutex>, and <shared_mutex> headers use
> std::errc constants, but don't use std::system_error itself. They only
> use the __throw_system_error(int) function, which is defined in
> <bits/functexcept.h>.
>
> By including the header for the errc constants instead of the whole of
> <system_error> we avoid depending on the whole std::string definition.

it seems this patch broke many tests on Solaris, e.g.

FAIL: 29_atomics/atomic/requirements/types_neg.cc (test for excess errors)
Excess errors:
/var/gcc/regression/master/11.4-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/bits/std_mutex.h:157: error: 'EBUSY' was not declared in this scope

	Rainer
Jonathan Wakely Jan. 13, 2023, 4:39 p.m. UTC | #2 
On Fri, 13 Jan 2023 at 15:08, Rainer Orth wrote:
>
> Hi Jonathan,
>
> > The <condition_variable>, <mutex>, and <shared_mutex> headers use
> > std::errc constants, but don't use std::system_error itself. They only
> > use the __throw_system_error(int) function, which is defined in
> > <bits/functexcept.h>.
> >
> > By including the header for the errc constants instead of the whole of
> > <system_error> we avoid depending on the whole std::string definition.
>
> it seems this patch broke many tests on Solaris, e.g.
>
> FAIL: 29_atomics/atomic/requirements/types_neg.cc (test for excess errors)
> Excess errors:
> /var/gcc/regression/master/11.4-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/bits/std_mutex.h:157: error: 'EBUSY' was not declared in this scope
>

Oops, testing this patch now.
commit 58e6fe334e55f56eeb0211c10697be4d4a8c52b6
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jan 13 16:37:57 2023

    libstdc++: Add <errno.h> to <bits/std_mutex.h>
    
    This needs to be included explicitly now that we don't include all of
    <system_error> here.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/std_mutex.h: Include <errno.h>.

diff --git a/libstdc++-v3/include/bits/std_mutex.h b/libstdc++-v3/include/bits/std_mutex.h
index bc515358d23..f74ddc4123a 100644
--- a/libstdc++-v3/include/bits/std_mutex.h
+++ b/libstdc++-v3/include/bits/std_mutex.h
@@ -36,6 +36,7 @@
 # include <bits/c++0x_warning.h>
 #else
 
+#include <errno.h> // EBUSY
 #include <bits/functexcept.h>
 #include <bits/gthr.h>
Jonathan Wakely Jan. 13, 2023, 5:38 p.m. UTC | #3 
On Fri, 13 Jan 2023 at 16:39, Jonathan Wakely wrote:
>
> On Fri, 13 Jan 2023 at 15:08, Rainer Orth wrote:
> >
> > Hi Jonathan,
> >
> > > The <condition_variable>, <mutex>, and <shared_mutex> headers use
> > > std::errc constants, but don't use std::system_error itself. They only
> > > use the __throw_system_error(int) function, which is defined in
> > > <bits/functexcept.h>.
> > >
> > > By including the header for the errc constants instead of the whole of
> > > <system_error> we avoid depending on the whole std::string definition.
> >
> > it seems this patch broke many tests on Solaris, e.g.
> >
> > FAIL: 29_atomics/atomic/requirements/types_neg.cc (test for excess errors)
> > Excess errors:
> > /var/gcc/regression/master/11.4-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/bits/std_mutex.h:157: error: 'EBUSY' was not declared in this scope
> >
>
> Oops, testing this patch now.

Pushed to trunk - thanks for the report!
Rainer Orth Jan. 16, 2023, 1:21 p.m. UTC | #4 
Hi Jonathan,

> On Fri, 13 Jan 2023 at 16:39, Jonathan Wakely wrote:
>>
>> On Fri, 13 Jan 2023 at 15:08, Rainer Orth wrote:
>> >
>> > Hi Jonathan,
>> >
>> > > The <condition_variable>, <mutex>, and <shared_mutex> headers use
>> > > std::errc constants, but don't use std::system_error itself. They only
>> > > use the __throw_system_error(int) function, which is defined in
>> > > <bits/functexcept.h>.
>> > >
>> > > By including the header for the errc constants instead of the whole of
>> > > <system_error> we avoid depending on the whole std::string definition.
>> >
>> > it seems this patch broke many tests on Solaris, e.g.
>> >
>> > FAIL: 29_atomics/atomic/requirements/types_neg.cc (test for excess errors)
>> > Excess errors:
>> > /var/gcc/regression/master/11.4-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/bits/std_mutex.h:157:
>> > error: 'EBUSY' was not declared in this scope
>> >
>>
>> Oops, testing this patch now.
>
> Pushed to trunk - thanks for the report!

great, thanks for the quick fix.

	Rainer
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/std_mutex.h b/libstdc++-v3/include/bits/std_mutex.h
index 68f5fb9ed65..bc515358d23 100644
--- a/libstdc++-v3/include/bits/std_mutex.h
+++ b/libstdc++-v3/include/bits/std_mutex.h
@@ -36,7 +36,6 @@ 
 # include <bits/c++0x_warning.h>
 #else
 
-#include <system_error>
 #include <bits/functexcept.h>
 #include <bits/gthr.h>
 
diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
index b885e1baa1b..f671fe4afe1 100644
--- a/libstdc++-v3/include/std/condition_variable
+++ b/libstdc++-v3/include/std/condition_variable
@@ -38,6 +38,7 @@ 
 #else
 
 #include <bits/chrono.h>
+#include <bits/error_constants.h>
 #include <bits/std_mutex.h>
 #include <bits/unique_lock.h>
 #include <bits/alloc_traits.h>
@@ -372,7 +373,7 @@  _GLIBCXX_BEGIN_INLINE_ABI_NAMESPACE(_V2)
         {
           return __p();
         }
- 
+
       std::stop_callback __cb(__stoken, [this] { notify_all(); });
       shared_ptr<mutex> __mutex = _M_mutex;
       while (!__p())
diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index aca5f91e03c..4eedbe5038c 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -40,8 +40,8 @@ 
 #include <tuple>
 #include <exception>
 #include <type_traits>
-#include <system_error>
 #include <bits/chrono.h>
+#include <bits/error_constants.h>
 #include <bits/std_mutex.h>
 #include <bits/unique_lock.h>
 #if ! _GTHREAD_USE_MUTEX_TIMEDLOCK
diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
index 7b70697f178..57c3cc54d81 100644
--- a/libstdc++-v3/include/std/shared_mutex
+++ b/libstdc++-v3/include/std/shared_mutex
@@ -36,6 +36,7 @@ 
 #if __cplusplus >= 201402L
 
 #include <bits/chrono.h>
+#include <bits/error_constants.h>
 #include <bits/functexcept.h>
 #include <bits/move.h>        // move, __exchange
 #include <bits/std_mutex.h>   // defer_lock_t