Skip to content

Commit 89d9de2

Browse files
lemirejkeiser
lemire
and
jkeiser
authored
Adding a check to see whether document::stream copy constructor and assignment actually compile (simdjson#556)
* Currently, document::stream contains an attribute that is a reference: ``` document::parser &parser; ``` Yet we try to have it default on the move operator: ``` stream &operator=(document::stream &&other) = default; stream &operator=(const document::stream &) = delete; // Disallow copying ``` ``` stream(document::stream &&other) = default; stream(const document::stream &) = delete; // Disallow copying ``` I am not sure what the move is supposed to do with the reference. I cannot find where we test the copy constructor and assignment. This has been concerned that it is either dead code or buggy code. * Remove non-working, unnecessary move constructors * We still want to disallow copies. Co-authored-by: John Keiser <john@johnkeiser.com>
1 parent 12c85d3 commit 89d9de2

2 files changed

Lines changed: 15 additions & 15 deletions

File tree

‎include/simdjson/document_stream.h‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,6 @@ class document::stream {
1818
public:
1919
really_inline ~stream() noexcept;
2020

21-
/**
22-
* Take another stream's buffers and state.
23-
*
24-
* @param other The stream to take. Its capacity is zeroed.
25-
*/
26-
stream(document::stream &&other) = default;
27-
stream(const document::stream &) = delete; // Disallow copying
28-
/**
29-
* Take another stream's buffers and state.
30-
*
31-
* @param other The stream to take. Its capacity is zeroed.
32-
*/
33-
stream &operator=(document::stream &&other) = default;
34-
stream &operator=(const document::stream &) = delete; // Disallow copying
35-
3621
/**
3722
* An iterator through a forward-only stream of documents.
3823
*/
@@ -71,6 +56,11 @@ class document::stream {
7156
really_inline iterator end() noexcept;
7257

7358
private:
59+
60+
stream &operator=(const document::stream &) = delete; // Disallow copying
61+
62+
stream(document::stream &other) = delete; // Disallow copying
63+
7464
really_inline stream(document::parser &parser, const uint8_t *buf, size_t len, size_t batch_size, error_code error = SUCCESS) noexcept;
7565

7666
/**

‎tests/basictests.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,16 @@ bool stable_test() {
232232
return newjson == json;
233233
}
234234

235+
static simdjson::document::stream parse_many_stream_return(simdjson::document::parser &parser, simdjson::padded_string &str) {
236+
return parser.parse_many(str);
237+
}
238+
// this is a compilation test
239+
UNUSED static void parse_many_stream_assign() {
240+
simdjson::document::parser parser;
241+
simdjson::padded_string str("{}",2);
242+
simdjson::document::stream s1 = parse_many_stream_return(parser, str);
243+
}
244+
235245
static bool parse_json_message_issue467(char const* message, std::size_t len, size_t expectedcount) {
236246
simdjson::document::parser parser;
237247
size_t count = 0;

0 commit comments

Comments
 (0)