Simplify move constructors. #5409
Conversation
| FilterBuffer::BufferOrView::BufferOrView(BufferOrView&& other) { | ||
| underlying_buffer_.swap(other.underlying_buffer_); | ||
| view_.swap(other.view_); | ||
| std::swap(is_view_, other.is_view_); |
There was a problem hiding this comment.
We were accessing is_view_ while unititialized. A default move constructor suffices here.
teo-tsirpanis
force-pushed
the
teo/fix-werrors
branch
from
510f7ba to
2ccf242
Compare
| @@ -85,9 +87,7 @@ FilterPipeline::FilterPipeline( | |||
| max_chunk_size_ = other.max_chunk_size_; | |||
| } | |||
|
|
|||
| FilterPipeline::FilterPipeline(FilterPipeline&& other) { | |||
| swap(other); | |||
There was a problem hiding this comment.
Same problem as before. The compiler looked through swap and saw uninitialized member access.
This comment was marked as outdated.
This comment was marked as outdated.
teo-tsirpanis
force-pushed
the
teo/fix-werrors
branch
from
2ccf242 to
1421da9
Compare
| @@ -339,7 +339,7 @@ class FilterBuffer { | |||
| shared_ptr<Buffer> underlying_buffer_; | |||
|
|
|||
| /** True if this instance is a view on the underlying buffer. */ | |||
| bool is_view_{false}; | |||
| bool is_view_; | |||
There was a problem hiding this comment.
is_view_ (and FilterPipeline::max_chunk_size_) are always initialized by the constructors, so this is not necessary. And if they ever don't, the compiler will warn-as-error again.
|
LGTM, but one thing comes to my mind:
|
|
It wouldn't have much benefit in this case, since all methods in |
Partially reverts #5412.
TYPE: NO_HISTORY