[libc++][sstream] Explicitly delete special member functions #80254
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
H-G-Hristov
commented
Feb 1, 2024
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
H-G-Hristov
force-pushed
the
hgh/libcxx/stringstream-Add-deleted-special-member-functions
branch
from
efd6cae to
7c88fd8
Compare
The standard declares the copy constructors and copy assign operators as deleted. References: - https://eel.is/c++draft/string.streams
H-G-Hristov
force-pushed
the
hgh/libcxx/stringstream-Add-deleted-special-member-functions
branch
from
7c88fd8 to
ee057b6
Compare
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Hristo Hristov (H-G-Hristov) ChangesThe standard declares the copy constructors and copy assign operators as deleted. References: Full diff: https://github.com/llvm/llvm-project/pull/80254.diff 9 Files Affected:
diff --git a/libcxx/include/sstream b/libcxx/include/sstream
index 6c354cf0b3973..2a009b05b2ba6 100644
--- a/libcxx/include/sstream
+++ b/libcxx/include/sstream
@@ -48,10 +48,12 @@ public:
template <class SAlloc>
explicit basic_stringbuf(const basic_string<char_type, traits_type, SAlloc>& s,
ios_base::openmode which = ios_base::in | ios_base::out); // C++20
+ basic_istringstream(const basic_istringstream&) = delete;
basic_stringbuf(basic_stringbuf&& rhs);
basic_stringbuf(basic_stringbuf&& rhs, const allocator_type& a); // C++20
// [stringbuf.assign] Assign and swap:
+ basic_istringstream& operator=(const basic_istringstream&) = delete;
basic_stringbuf& operator=(basic_stringbuf&& rhs);
void swap(basic_stringbuf& rhs) noexcept(see below); // conditionally noexcept since C++20
@@ -119,9 +121,11 @@ public:
template <class SAlloc>
explicit basic_istringstream(const basic_string<char_type, traits_type, SAlloc>& s,
ios_base::openmode which = ios_base::in); // C++20
+ basic_istringstream(const basic_istringstream&) = delete;
basic_istringstream(basic_istringstream&& rhs);
// [istringstream.assign] Assign and swap:
+ basic_istringstream& operator=(const basic_istringstream&) = delete;
basic_istringstream& operator=(basic_istringstream&& rhs);
void swap(basic_istringstream& rhs);
@@ -178,9 +182,11 @@ public:
template <class SAlloc>
explicit basic_ostringstream(const basic_string<char_type, traits_type, SAlloc>& s,
ios_base::openmode which = ios_base::out); // C++20
+ basic_ostringstream(const basic_ostringstream&) = delete;
basic_ostringstream(basic_ostringstream&& rhs);
// [ostringstream.assign] Assign and swap:
+ basic_ostringstream& operator=(const basic_ostringstream&) = delete;
basic_ostringstream& operator=(basic_ostringstream&& rhs);
void swap(basic_ostringstream& rhs);
@@ -237,9 +243,11 @@ public:
template <class SAlloc>
explicit basic_stringstream(const basic_string<char_type, traits_type, SAlloc>& s,
ios_base::openmode which = ios_base::out | ios_base::in); // C++20
+ basic_stringstream(const basic_stringstream&) = delete;
basic_stringstream(basic_stringstream&& rhs);
// [stringstream.assign] Assign and swap:
+ basic_stringstream& operator=(const basic_stringstream&) = delete;
basic_stringstream& operator=(basic_stringstream&& rhs);
void swap(basic_stringstream& rhs);
@@ -364,6 +372,7 @@ public:
}
#endif // _LIBCPP_STD_VER >= 20
+ basic_stringbuf(const basic_stringbuf&) = delete;
basic_stringbuf(basic_stringbuf&& __rhs) : __mode_(__rhs.__mode_) { __move_init(std::move(__rhs)); }
#if _LIBCPP_STD_VER >= 20
@@ -374,6 +383,7 @@ public:
#endif
// [stringbuf.assign] Assign and swap:
+ basic_stringbuf& operator=(const basic_stringbuf&) = delete;
basic_stringbuf& operator=(basic_stringbuf&& __rhs);
void swap(basic_stringbuf& __rhs)
#if _LIBCPP_STD_VER >= 20
@@ -822,12 +832,14 @@ public:
: basic_istream<_CharT, _Traits>(std::addressof(__sb_)), __sb_(__s, __wch | ios_base::in) {}
#endif // _LIBCPP_STD_VER >= 20
+ basic_istringstream(const basic_istringstream&) = delete;
_LIBCPP_HIDE_FROM_ABI basic_istringstream(basic_istringstream&& __rhs)
: basic_istream<_CharT, _Traits>(std::move(__rhs)), __sb_(std::move(__rhs.__sb_)) {
basic_istream<_CharT, _Traits>::set_rdbuf(&__sb_);
}
// [istringstream.assign] Assign and swap:
+ basic_istringstream& operator=(const basic_istringstream&) = delete;
basic_istringstream& operator=(basic_istringstream&& __rhs) {
basic_istream<char_type, traits_type>::operator=(std::move(__rhs));
__sb_ = std::move(__rhs.__sb_);
@@ -929,12 +941,14 @@ public:
: basic_ostream<_CharT, _Traits>(std::addressof(__sb_)), __sb_(__s, __wch | ios_base::out) {}
#endif // _LIBCPP_STD_VER >= 20
+ basic_ostringstream(const basic_ostringstream&) = delete;
_LIBCPP_HIDE_FROM_ABI basic_ostringstream(basic_ostringstream&& __rhs)
: basic_ostream<_CharT, _Traits>(std::move(__rhs)), __sb_(std::move(__rhs.__sb_)) {
basic_ostream<_CharT, _Traits>::set_rdbuf(&__sb_);
}
// [ostringstream.assign] Assign and swap:
+ basic_ostringstream& operator=(const basic_ostringstream&) = delete;
basic_ostringstream& operator=(basic_ostringstream&& __rhs) {
basic_ostream<char_type, traits_type>::operator=(std::move(__rhs));
__sb_ = std::move(__rhs.__sb_);
@@ -1040,12 +1054,14 @@ public:
: basic_iostream<_CharT, _Traits>(std::addressof(__sb_)), __sb_(__s, __wch) {}
#endif // _LIBCPP_STD_VER >= 20
+ basic_stringstream(const basic_stringstream&) = delete;
_LIBCPP_HIDE_FROM_ABI basic_stringstream(basic_stringstream&& __rhs)
: basic_iostream<_CharT, _Traits>(std::move(__rhs)), __sb_(std::move(__rhs.__sb_)) {
basic_istream<_CharT, _Traits>::set_rdbuf(&__sb_);
}
// [stringstream.assign] Assign and swap:
+ basic_stringstream& operator=(const basic_stringstream&) = delete;
basic_stringstream& operator=(basic_stringstream&& __rhs) {
basic_iostream<char_type, traits_type>::operator=(std::move(__rhs));
__sb_ = std::move(__rhs.__sb_);
diff --git a/libcxx/test/std/input.output/string.streams/istringstream/istringstream.assign/copy.compile.pass.cpp b/libcxx/test/std/input.output/string.streams/istringstream/istringstream.assign/copy.compile.pass.cpp
new file mode 100644
index 0000000000000..f635006a229c3
--- /dev/null
+++ b/libcxx/test/std/input.output/string.streams/istringstream/istringstream.assign/copy.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringstream
+
+// basic_istringstream& operator=(const basic_istringstream&) = delete;
+
+#include <sstream>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_copy_assignable<std::istringstream>::value, "");
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::is_copy_assignable<std::wistringstream>::value, "");
+#endif
diff --git a/libcxx/test/std/input.output/string.streams/istringstream/istringstream.cons/copy.compile.pass.cpp b/libcxx/test/std/input.output/string.streams/istringstream/istringstream.cons/copy.compile.pass.cpp
new file mode 100644
index 0000000000000..41b91b48044c6
--- /dev/null
+++ b/libcxx/test/std/input.output/string.streams/istringstream/istringstream.cons/copy.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringstream
+
+// basic_istringstream(const basic_istringstream&) = delete;
+
+#include <sstream>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_copy_constructible<std::istringstream>::value, "");
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::is_copy_constructible<std::wistringstream>::value, "");
+#endif
diff --git a/libcxx/test/std/input.output/string.streams/ostringstream/ostringstream.assign/copy.compile.pass.cpp b/libcxx/test/std/input.output/string.streams/ostringstream/ostringstream.assign/copy.compile.pass.cpp
new file mode 100644
index 0000000000000..350bb8ec36434
--- /dev/null
+++ b/libcxx/test/std/input.output/string.streams/ostringstream/ostringstream.assign/copy.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringstream
+
+// basic_stringstream& operator=(const basic_stringstream&) = delete;
+
+#include <sstream>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_copy_constructible<std::stringstream>::value, "");
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::is_copy_constructible<std::wstringstream>::value, "");
+#endif
diff --git a/libcxx/test/std/input.output/string.streams/ostringstream/ostringstream.cons/copy.compile.pass.cpp b/libcxx/test/std/input.output/string.streams/ostringstream/ostringstream.cons/copy.compile.pass.cpp
new file mode 100644
index 0000000000000..decde2d94c665
--- /dev/null
+++ b/libcxx/test/std/input.output/string.streams/ostringstream/ostringstream.cons/copy.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringstream
+
+// basic_ostringstream(const basic_ostringstream&) = delete;
+
+#include <sstream>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_copy_assignable<std::ostringstream>::value, "");
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::is_copy_assignable<std::wostringstream>::value, "");
+#endif
diff --git a/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/copy.compile.pass.cpp b/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/copy.compile.pass.cpp
new file mode 100644
index 0000000000000..fa2c96af97d21
--- /dev/null
+++ b/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/copy.compile.pass.cpp
@@ -0,0 +1,27 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringbuf
+
+// basic_stringbuf(const basic_stringbuf&) = delete;
+
+#include <sstream>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_copy_assignable<std::basic_stringbuf<char>>::value, "");
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::is_copy_assignable<std::basic_stringbuf<wchar_t>>::value, "");
+#endif
diff --git a/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.cons/copy.compile.pass.cpp b/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.cons/copy.compile.pass.cpp
new file mode 100644
index 0000000000000..f3670b45bec09
--- /dev/null
+++ b/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.cons/copy.compile.pass.cpp
@@ -0,0 +1,27 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringbuf
+
+// basic_stringbuf(const basic_stringbuf&) = delete;
+
+#include <sstream>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_copy_constructible<std::basic_stringbuf<char>>::value, "");
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::is_copy_constructible<std::basic_stringbuf<wchar_t>>::value, "");
+#endif
diff --git a/libcxx/test/std/input.output/string.streams/stringstream/stringstream.assign/copy.compile.pass.cpp b/libcxx/test/std/input.output/string.streams/stringstream/stringstream.assign/copy.compile.pass.cpp
new file mode 100644
index 0000000000000..e20ebe2899c3a
--- /dev/null
+++ b/libcxx/test/std/input.output/string.streams/stringstream/stringstream.assign/copy.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringstream
+
+// basic_stringstream& operator=(const basic_stringstream&) = delete;
+
+#include <sstream>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_copy_assignable<std::stringstream>::value, "");
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::is_copy_assignable<std::wstringstream>::value, "");
+#endif
diff --git a/libcxx/test/std/input.output/string.streams/stringstream/stringstream.cons/copy.compile.pass.cpp b/libcxx/test/std/input.output/string.streams/stringstream/stringstream.cons/copy.compile.pass.cpp
new file mode 100644
index 0000000000000..353df09a9a38a
--- /dev/null
+++ b/libcxx/test/std/input.output/string.streams/stringstream/stringstream.cons/copy.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <sstream>
+
+// template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
+// class basic_stringstream
+
+// basic_stringstream(const basic_stringstream&) = delete;
+
+#include <sstream>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(!std::is_copy_constructible< std::stringstream>::value, "");
+
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::is_copy_constructible< std::wstringstream>::value, "");
+#endif
|
|
They are already implicitly deleted because there are move constructors and assignment operators. Explicit deletion is simply not required. |
I assume it is a matter of style to be explicit. As I understand it the implementation tries to stay close to the standard wording (and I like being explicit) I created the PR. In that context do we want any part of this PR or should I drop it? |
mordante
requested changes
Feb 10, 2024
There was a problem hiding this comment.
Thanks for working on this! A few minor things, I'd like to have another look before approving.
Even when this is implicit I strongly prefer to follow the wording of the Standard.
I'm also happy we test these properties since it seems we didn't do that either.
| // template <class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> > | ||
| // class basic_stringstream | ||
|
|
||
| // basic_istringstream& operator=(const basic_istringstream&) = delete; |
There was a problem hiding this comment.
I rather test this is a types.compile.pass this directly lets us test all copy/move properties in one place. I noticed there is already a file test/std/input.output/string.streams/istringstream/types.pass.cpp as a drive-by can you rename this file to test/std/input.output/string.streams/istringstream/types.compile.pass.cpp and remove the main? (Please check whether is applies to the other tests too.)
There was a problem hiding this comment.
Thank you for reviewing! DONE.
|
I'm fine with this, but please change the title. Currently, this implies to me that the members weren't deleted before. Maybe something like "explicitly delete special member functions". |
H-G-Hristov
changed the title
The perils of the non-native language. Thank you for the suggestion! |
H-G-Hristov
commented
Feb 12, 2024
|
|
||
| // Types | ||
|
|
||
| static_assert((std::is_base_of<std::basic_istream<char>, std::basic_istringstream<char> >::value), ""); |
There was a problem hiding this comment.
Should I add tests with wchar_t in this PR? And I think we don't need to double parenthesis () in static_assert. I can do in a follow-up.
There was a problem hiding this comment.
I'd like the wchar_t in this commit.
I expect these test were original normal asserts and in a macro these double parenthesis are needed. It would be great when you can do that in a followup NFC commit.
mordante
approved these changes
Feb 12, 2024
|
|
||
| // Types | ||
|
|
||
| static_assert((std::is_base_of<std::basic_istream<char>, std::basic_istringstream<char> >::value), ""); |
There was a problem hiding this comment.
I'd like the wchar_t in this commit.
I expect these test were original normal asserts and in a macro these double parenthesis are needed. It would be great when you can do that in a followup NFC commit.
Comment on lines
46
to
62
| static_assert(!std::is_copy_constructible<std::istringstream>::value, ""); | ||
| static_assert(!std::is_copy_assignable<std::istringstream>::value, ""); | ||
|
|
||
| #ifndef TEST_HAS_NO_WIDE_CHARACTERS | ||
| static_assert(!std::is_copy_constructible<std::wistringstream>::value, ""); | ||
| static_assert(!std::is_copy_assignable<std::wistringstream>::value, ""); | ||
| #endif | ||
|
|
||
| // Move properties | ||
|
|
||
| static_assert(std::is_move_constructible<std::istringstream>::value, ""); | ||
| static_assert(std::is_move_assignable<std::istringstream>::value, ""); | ||
|
|
||
| #ifndef TEST_HAS_NO_WIDE_CHARACTERS | ||
| static_assert(std::is_move_constructible<std::wistringstream>::value, ""); | ||
| static_assert(std::is_move_assignable<std::wistringstream>::value, ""); | ||
| #endif |
There was a problem hiding this comment.
you copy pasted istringstream and didn't update the changes. The same happened to the other two tests below.
There was a problem hiding this comment.
Sorry about that!
|
@mordante Thank you for the review! |
H-G-Hristov
deleted the
hgh/libcxx/stringstream-Add-deleted-special-member-functions
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The standard declares the copy constructors and copy assign operators as deleted.
References: