diff mbox series

[committed] libstdc++: Make std::vector<bool>::reference constructor private [PR115098]

Message ID 20240823121906.959071-1-jwakely@redhat.com
State New
Headers show
Series [committed] libstdc++: Make std::vector<bool>::reference constructor private [PR115098] | expand

Commit Message

Jonathan Wakely Aug. 23, 2024, 12:18 p.m. UTC 
Tested x86_64-linux. Pushed to trunk.

-- >8 --

The standard says this constructor should be private.  LWG 4141 proposes
to remove it entirely. We still need it, but it doesn't need to be
public.

For std::bitset the default constructor is already private (and never
even defined) but there's a non-standard constructor that's public, but
doesn't need to be.

libstdc++-v3/ChangeLog:

	PR libstdc++/115098
	* include/bits/stl_bvector.h (_Bit_reference): Make default
	constructor private. Declare vector and bit iterators as
	friends.
	* include/std/bitset (bitset::reference): Make constructor and
	data members private.
	* testsuite/20_util/bitset/115098.cc: New test.
	* testsuite/23_containers/vector/bool/115098.cc: New test.
---
 libstdc++-v3/include/bits/stl_bvector.h              | 12 +++++++++---
 libstdc++-v3/include/std/bitset                      |  5 +----
 libstdc++-v3/testsuite/20_util/bitset/115098.cc      | 11 +++++++++++
 .../testsuite/23_containers/vector/bool/115098.cc    |  8 ++++++++
 4 files changed, 29 insertions(+), 7 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/20_util/bitset/115098.cc
 create mode 100644 libstdc++-v3/testsuite/23_containers/vector/bool/115098.cc

Comments

Andrew Pinski Aug. 25, 2024, 11:08 p.m. UTC | #1 
On Fri, Aug 23, 2024 at 5:20 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> Tested x86_64-linux. Pushed to trunk.
>
> -- >8 --
>
> The standard says this constructor should be private.  LWG 4141 proposes
> to remove it entirely. We still need it, but it doesn't need to be
> public.
>
> For std::bitset the default constructor is already private (and never
> even defined) but there's a non-standard constructor that's public, but
> doesn't need to be.

This looks like broke the pretty-printers testcase:
```
/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc:
In function 'int main()':
/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc:156:
error: 'std::_Bit_reference::_Bit_reference()' is private within this
context
In file included from
/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/vector:67,
                 from
/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc:31:
/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_bvector.h:90:
note: declared private here
compiler exited with status 1

...
spawn -ignore SIGHUP
/home/apinski/src/upstream-gcc-isel/gcc/objdir/./gcc/xg++
-shared-libgcc -B/home/apinski/src/upstream-gcc-isel/gcc/objdir/./gcc
-nostdinc++ -L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/src
-L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs
-L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs
-B/home/apinski/upstream-gcc-isel/x86_64-pc-linux-gnu/bin/
-B/home/apinski/upstream-gcc-isel/x86_64-pc-linux-gnu/lib/ -isystem
/home/apinski/upstream-gcc-isel/x86_64-pc-linux-gnu/include -isystem
/home/apinski/upstream-gcc-isel/x86_64-pc-linux-gnu/sys-include
-fchecking=1 -B/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/./libstdc++-v3/src/.libs
-fmessage-length=0 -fno-show-column -ffunction-sections
-fdata-sections -fcf-protection -mshstk -g -O2 -D_GNU_SOURCE
-DLOCALEDIR="." -nostdinc++
-I/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu
-I/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include
-I/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/libsupc++
-I/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/include/backward
-I/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/util
/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc
-g -O0 -fdiagnostics-plain-output ./libtestc++.a -Wl,--gc-sections
-L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/src/filesystem/.libs
-L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/src/experimental/.libs
-lm -o ./simple11.exe
/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc:
In function 'int main()':
/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc:149:
error: 'std::_Bit_reference::_Bit_reference()' is private within this
context
In file included from
/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/vector:67,
                 from
/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc:31:
/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_bvector.h:90:
note: declared private here
compiler exited with status 1
```

Noticed because of the new UNRESOLVED .

Thanks,
Andrew Pinski




>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/115098
>         * include/bits/stl_bvector.h (_Bit_reference): Make default
>         constructor private. Declare vector and bit iterators as
>         friends.
>         * include/std/bitset (bitset::reference): Make constructor and
>         data members private.
>         * testsuite/20_util/bitset/115098.cc: New test.
>         * testsuite/23_containers/vector/bool/115098.cc: New test.
> ---
>  libstdc++-v3/include/bits/stl_bvector.h              | 12 +++++++++---
>  libstdc++-v3/include/std/bitset                      |  5 +----
>  libstdc++-v3/testsuite/20_util/bitset/115098.cc      | 11 +++++++++++
>  .../testsuite/23_containers/vector/bool/115098.cc    |  8 ++++++++
>  4 files changed, 29 insertions(+), 7 deletions(-)
>  create mode 100644 libstdc++-v3/testsuite/20_util/bitset/115098.cc
>  create mode 100644 libstdc++-v3/testsuite/23_containers/vector/bool/115098.cc
>
> diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
> index c45b7ff3320..42261ac5915 100644
> --- a/libstdc++-v3/include/bits/stl_bvector.h
> +++ b/libstdc++-v3/include/bits/stl_bvector.h
> @@ -81,6 +81,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>
>    struct _Bit_reference
>    {
> +  private:
> +    template<typename, typename> friend class vector;
> +    friend struct _Bit_iterator;
> +    friend struct _Bit_const_iterator;
> +
> +    _GLIBCXX20_CONSTEXPR
> +    _Bit_reference() _GLIBCXX_NOEXCEPT : _M_p(0), _M_mask(0) { }
> +
>      _Bit_type * _M_p;
>      _Bit_type _M_mask;
>
> @@ -88,9 +96,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>      _Bit_reference(_Bit_type * __x, _Bit_type __y)
>      : _M_p(__x), _M_mask(__y) { }
>
> -    _GLIBCXX20_CONSTEXPR
> -    _Bit_reference() _GLIBCXX_NOEXCEPT : _M_p(0), _M_mask(0) { }
> -
> +  public:
>  #if __cplusplus >= 201103L
>      _Bit_reference(const _Bit_reference&) = default;
>  #endif
> diff --git a/libstdc++-v3/include/std/bitset b/libstdc++-v3/include/std/bitset
> index e5d677ff059..2e82a0e289d 100644
> --- a/libstdc++-v3/include/std/bitset
> +++ b/libstdc++-v3/include/std/bitset
> @@ -870,10 +870,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>         _WordT* _M_wp;
>         size_t  _M_bpos;
>
> -       // left undefined
> -       reference();
> -
> -      public:
>         _GLIBCXX23_CONSTEXPR
>         reference(bitset& __b, size_t __pos) _GLIBCXX_NOEXCEPT
>         {
> @@ -881,6 +877,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>           _M_bpos = _Base::_S_whichbit(__pos);
>         }
>
> +      public:
>  #if __cplusplus >= 201103L
>         reference(const reference&) = default;
>  #endif
> diff --git a/libstdc++-v3/testsuite/20_util/bitset/115098.cc b/libstdc++-v3/testsuite/20_util/bitset/115098.cc
> new file mode 100644
> index 00000000000..52d6a0ec378
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/bitset/115098.cc
> @@ -0,0 +1,11 @@
> +// { dg-do compile { target c++11 } }
> +
> +#include <bitset>
> +
> +using namespace std;
> +
> +static_assert( ! is_default_constructible<bitset<10>::reference>::value,
> +    "std::bitset<N>::reference is not default constructible");
> +
> +static_assert( ! is_constructible<bitset<10>::reference, bitset<10>&, size_t>::value,
> +    "std::bitset<N>::reference is not default constructible");
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/115098.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/115098.cc
> new file mode 100644
> index 00000000000..3df8b801795
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/23_containers/vector/bool/115098.cc
> @@ -0,0 +1,8 @@
> +// { dg-do compile { target c++11 } }
> +
> +#include <vector>
> +
> +static_assert(
> +    !std::is_default_constructible<std::vector<bool>::reference>::value,
> +    "std::vector<bool>::reference is not default constructible"
> +    );
> --
> 2.46.0
>
Jonathan Wakely Aug. 27, 2024, 1:24 p.m. UTC | #2 
On Mon, 26 Aug 2024 at 00:08, Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Fri, Aug 23, 2024 at 5:20 AM Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > Tested x86_64-linux. Pushed to trunk.
> >
> > -- >8 --
> >
> > The standard says this constructor should be private.  LWG 4141 proposes
> > to remove it entirely. We still need it, but it doesn't need to be
> > public.
> >
> > For std::bitset the default constructor is already private (and never
> > even defined) but there's a non-standard constructor that's public, but
> > doesn't need to be.
>
> This looks like broke the pretty-printers testcase:
> ```
> /home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc:
> In function 'int main()':
> /home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc:156:
> error: 'std::_Bit_reference::_Bit_reference()' is private within this
> context
> In file included from
> /home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/vector:67,
>                  from
> /home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc:31:
> /home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_bvector.h:90:
> note: declared private here
> compiler exited with status 1
>
> ...
> spawn -ignore SIGHUP
> /home/apinski/src/upstream-gcc-isel/gcc/objdir/./gcc/xg++
> -shared-libgcc -B/home/apinski/src/upstream-gcc-isel/gcc/objdir/./gcc
> -nostdinc++ -L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/src
> -L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/src/.libs
> -L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs
> -B/home/apinski/upstream-gcc-isel/x86_64-pc-linux-gnu/bin/
> -B/home/apinski/upstream-gcc-isel/x86_64-pc-linux-gnu/lib/ -isystem
> /home/apinski/upstream-gcc-isel/x86_64-pc-linux-gnu/include -isystem
> /home/apinski/upstream-gcc-isel/x86_64-pc-linux-gnu/sys-include
> -fchecking=1 -B/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/./libstdc++-v3/src/.libs
> -fmessage-length=0 -fno-show-column -ffunction-sections
> -fdata-sections -fcf-protection -mshstk -g -O2 -D_GNU_SOURCE
> -DLOCALEDIR="." -nostdinc++
> -I/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu
> -I/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include
> -I/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/libsupc++
> -I/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/include/backward
> -I/home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/util
> /home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc
> -g -O0 -fdiagnostics-plain-output ./libtestc++.a -Wl,--gc-sections
> -L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/src/filesystem/.libs
> -L/home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/src/experimental/.libs
> -lm -o ./simple11.exe
> /home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc:
> In function 'int main()':
> /home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc:149:
> error: 'std::_Bit_reference::_Bit_reference()' is private within this
> context
> In file included from
> /home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/vector:67,
>                  from
> /home/apinski/src/upstream-gcc-isel/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc:31:
> /home/apinski/src/upstream-gcc-isel/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_bvector.h:90:
> note: declared private here
> compiler exited with status 1
> ```
>
> Noticed because of the new UNRESOLVED .

Oops, thanks for noticing. I didn't see it because it didn't add a FAIL.

I'll push a fix to the tests.
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index c45b7ff3320..42261ac5915 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -81,6 +81,14 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 
   struct _Bit_reference
   {
+  private:
+    template<typename, typename> friend class vector;
+    friend struct _Bit_iterator;
+    friend struct _Bit_const_iterator;
+
+    _GLIBCXX20_CONSTEXPR
+    _Bit_reference() _GLIBCXX_NOEXCEPT : _M_p(0), _M_mask(0) { }
+
     _Bit_type * _M_p;
     _Bit_type _M_mask;
 
@@ -88,9 +96,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
     _Bit_reference(_Bit_type * __x, _Bit_type __y)
     : _M_p(__x), _M_mask(__y) { }
 
-    _GLIBCXX20_CONSTEXPR
-    _Bit_reference() _GLIBCXX_NOEXCEPT : _M_p(0), _M_mask(0) { }
-
+  public:
 #if __cplusplus >= 201103L
     _Bit_reference(const _Bit_reference&) = default;
 #endif
diff --git a/libstdc++-v3/include/std/bitset b/libstdc++-v3/include/std/bitset
index e5d677ff059..2e82a0e289d 100644
--- a/libstdc++-v3/include/std/bitset
+++ b/libstdc++-v3/include/std/bitset
@@ -870,10 +870,6 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	_WordT*	_M_wp;
 	size_t 	_M_bpos;
 
-	// left undefined
-	reference();
-
-      public:
 	_GLIBCXX23_CONSTEXPR
 	reference(bitset& __b, size_t __pos) _GLIBCXX_NOEXCEPT
 	{
@@ -881,6 +877,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  _M_bpos = _Base::_S_whichbit(__pos);
 	}
 
+      public:
 #if __cplusplus >= 201103L
 	reference(const reference&) = default;
 #endif
diff --git a/libstdc++-v3/testsuite/20_util/bitset/115098.cc b/libstdc++-v3/testsuite/20_util/bitset/115098.cc
new file mode 100644
index 00000000000..52d6a0ec378
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/bitset/115098.cc
@@ -0,0 +1,11 @@ 
+// { dg-do compile { target c++11 } }
+
+#include <bitset>
+
+using namespace std;
+
+static_assert( ! is_default_constructible<bitset<10>::reference>::value,
+    "std::bitset<N>::reference is not default constructible");
+
+static_assert( ! is_constructible<bitset<10>::reference, bitset<10>&, size_t>::value,
+    "std::bitset<N>::reference is not default constructible");
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/115098.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/115098.cc
new file mode 100644
index 00000000000..3df8b801795
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/115098.cc
@@ -0,0 +1,8 @@ 
+// { dg-do compile { target c++11 } }
+
+#include <vector>
+
+static_assert(
+    !std::is_default_constructible<std::vector<bool>::reference>::value,
+    "std::vector<bool>::reference is not default constructible"
+    );