Opened 10 years ago
Closed 8 years ago
#7888 closed Feature Requests (fixed)
circular_buffer should support move semantics
Reported by: | Owned by: | Antony Polukhin | |
---|---|---|---|
Milestone: | Boost 1.57.0 | Component: | circular_buffer |
Version: | Boost 1.52.0 | Severity: | Problem |
Keywords: | circular_buffer move emplace | Cc: |
Description
I would like to use circular buffer to store unique_ptr's or other moveable objects. I hope this feature will be added.
Change History (32)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
a description of the term move semantics can be found here http://www.cprogramming.com/c++11/rvalue-references-and-move-semantics-in-c++11.html
Here is an example of what I would like to do which works with a std::deque :
std::deque<std::unique_ptr<int>> myQueue; std::unique_ptr<int> myP(new int(4)); myQueue.push_back(std::move(myP)); //transfer ownership to the queue auto otherP = std::move(myQueue.front()); //transfer ownership back from the queue to a new unique_ptr myQueue.pop_front();
This doesn't work with boost::circular_buffer because there is only one overload of push_back and it takes a const T&. I need another overload that takes T&&.
comment:3 by , 10 years ago
I would also like to see this feature implemented in circular_buffer. At this moment it is actually possible to use std::shared_ptr. But, in order to maintain the same semantics as other containers in C++11 STL, circular_buffer should implement move semantics as well. Once that happens, using std::unique_ptr will be straightforward, bringing a potential performance improvement compared to std::shared_ptr.
comment:4 by , 9 years ago
One more request for this. If you store more complex elements inside the circular buffer, you don't want to have overhead code to care for the destruction of the elements when they are pushed out of the ring.
comment:5 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 9 years ago
comment:7 by , 9 years ago
comment:8 by , 9 years ago
comment:9 by , 9 years ago
comment:10 by , 9 years ago
(In [85003]) Improved rvalues move support for elements of circular_buffer (refs #7888):
- all
erase
methods now use move construction to to move elements - space optimized circullar buffer now has move constructor, move assignment and functions that work with rvalues
- more methods marked as BOOST_NOEXCEPT
- much more tests
comment:13 by , 9 years ago
comment:15 by , 9 years ago
comment:16 by , 9 years ago
comment:17 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
(In [85510]) Big merge of Boost.CircularBuffer :
- Full merge of QuickBoock documentation from Paul A. Bristow
- Merged rvalue references support with tests and documentation (fixed #7888)
follow-up: 19 comment:18 by , 9 years ago
This change has unfortunately introduced fresh warnings when building against boost 1.55 using VisualStudio 2010 with warning level 4. I now see:
1>C:\src\svn\OTI\ThirdParty\boost_1_55_0_OT1\include\boost/circular_buffer/base.hpp(199): warning C4172: returning address of local variable or temporary
Likewise, building using gcc 4.4.7 on RedHat produces the following warning:
’ |
Shall I open a new ticket for this issue? Unfortunately it's causing my builds to break, since I build with warnings-as-errors on both platforms. I'll need to either disable the warnings, or fix the problems.
comment:19 by , 9 years ago
Replying to pbf@…:
This change has unfortunately introduced fresh warnings when building against boost 1.55 using VisualStudio 2010 with warning level 4. I now see:
Great thanks for reporting that issue! I'll take care of those warnings, no need to open a separate ticket.
comment:20 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
follow-up: 22 comment:21 by , 8 years ago
We have hit the same NEW warnings in circular_buffer in Boost 1.55.0 with VS2010 and all warnings treated as errors. This issue shows "reopened", but that was six months ago and no more info. Any indication of when it will be fixed, and/or a patch we can apply ourselves?
comment:22 by , 8 years ago
Replying to michael.broida@…:
We have hit the same NEW warnings in circular_buffer in Boost 1.55.0 with VS2010 and all warnings treated as errors. This issue shows "reopened", but that was six months ago and no more info. Any indication of when it will be fixed, and/or a patch we can apply ourselves?
Warning with parentheses is fixed in develop and master branches
Other warning is not fixed. The simplest workaround would be to disable this warning at all.
Correct solution would be to move the move_if_noexcept
into the Boost.Move module and take care of it there. Unfortunately I had no time to take care of that issue.
follow-up: 24 comment:23 by , 8 years ago
OK. The unfixed "returning address of local variable or temporary" error is the one I'm seeing. It looks like only one line in our code leads to that warning, so I will disable that warning for that single line.
I hope you (or someone) has time to make the full fix before the next release. :) Thank you. :)
comment:24 by , 8 years ago
Replying to michael.broida@…:
only one line in our code leads to that warning
Could you please provide that line here? What function of the circular_buffer was called?
comment:25 by , 8 years ago
Apparently it's just the circular_buffer ctor.
(scopenames removed to make it easier to read.)
struct TestSetupContainers { TestSetupContainers() { output_object_pool_ = new GrowingMemoryPool<sizeof(OutputObject)>(11,0, "TestSetupContainers MemoryPool"); OutputObject::SetMemoryPool(output_object_pool_); output_buffer_.resize(10); } GrowingMemoryPool<sizeof(OutputObject)> *output_object_pool_; boost::circular_buffer<boost::shared_ptr<OutputObject>> output_buffer_; };
That last line is in our file OutputObject-UT.cpp line 20, the line triggering this long warning:
(This is with VS2010; Boost 1.55.0)
(long paths shortened to ....)
1> OutputObject-UT.cpp 1>....include\boost\circular_buffer\base.hpp(201): error C2220: warning treated as error - no 'object' file generated 1> ....include\boost\circular_buffer\base.hpp(2199) : see reference to function template instantiation 'const boost::shared_ptr<T> &boost::circular_buffer<boost::shared_ptr<T>>::move_if_noexcept<boost::shared_ptr<T>>(ValT &)' being compiled 1> with 1> [ 1> T=OutputObject, 1> ValT=boost::shared_ptr<OutputObject> 1> ] 1> ....include\boost\circular_buffer\base.hpp(2191) : while compiling class template member function 'boost::cb_details::iterator<Buff,Traits> boost::circular_buffer<T>::erase(boost::cb_details::iterator<Buff,Traits>,boost::cb_details::iterator<Buff,Traits>)' 1> with 1> [ 1> Buff=boost::circular_buffer<boost::shared_ptr<OutputObject>>, 1> Traits=boost::cb_details::nonconst_traits<std::allocator<OutputObjectHandle>>, 1> T=boost::shared_ptr<OutputObject> 1> ] 1> ....outputobject-ut.cpp(20) : see reference to class template instantiation 'boost::circular_buffer<T>' being compiled 1> with 1> [ 1> T=boost::shared_ptr<OutputObject> 1> ] 1>....include\boost\circular_buffer\base.hpp(201): warning C4172: returning address of local variable or temporary
I -was- able to avoid the warning by #pragma disabling it for the entire method in Boost's circular_buffer\base.hpp lines 189-200. That just seems like cheating, though. :) (I can't even figure out that method name!)
I hope that helps. Let me know if you need more information.
follow-up: 29 comment:26 by , 8 years ago
Oops. That error message was from a version where I was experimenting with code mods. With NO mods from Boost 1.55.0, the messages point to different lines:
- "base.hpp(201)" should be "base.hpp(199)" (two places)
- "base.hpp(2199)" should be "base.hpp(2196)"
- "base.hpp(2191)" should be "base.hpp(2188)"
Sorry for that glitch.
comment:27 by , 8 years ago
Ah-ha! I did some further testing.\ Though all the messages point to the line declaring the circular_buffer, the warnings only appear when the TestSetupContainer struct's ctor contains this line:
output_buffer_.resize(10);
So it would appear that the call to "resize" actually triggers the warnings.
I commented out everything in that test file except that struct declaration (and necessary #includes), just to be sure.
I also changed the declaration of "output_buffer_" to these types:
"output_buffer_" type | result |
---|---|
boost::circular_buffer<int> | NO warning |
boost::circular_buffer<boost::shared_ptr<int>> | SAME warning |
boost::circular_buffer<std::shared_ptr<int>> | SAME warning |
If I think of anything else, I'll post more here.
comment:28 by , 8 years ago
Aaargh! Now I'm very confused.
Forget that bit about the "resize" call. NOW I get the warning whether that line is present or not.
I know it went away when I commented that line out earlier. But now it's back. And the last two alternate types in the table in my previous post worked the same way. But now they don't.
I think Visual Studio 2010 hates me. :(
comment:29 by , 8 years ago
Replying to michael.broida@…:
With NO mods from Boost 1.55.0, the messages point to different lines:
- "base.hpp(201)" should be "base.hpp(199)" (two places)
- "base.hpp(2199)" should be "base.hpp(2196)"
- "base.hpp(2191)" should be "base.hpp(2188)"
Those lines look pretty innocent. move_if_noexcept
is used there for non temporaries and must not cause any troubles.
I think Visual Studio 2010 hates me. :(
Sometimes I think that there is a hidden "Do respect the programmer!" switch somewhere in Windows registry (in Linux it's on by default). Please tell me if you'll find it! :)
comment:31 by , 8 years ago
Milestone: | To Be Determined → Boost 1.57.0 |
---|
I hope that the last warnings were fixed by this commit, which is already merged to release branch of 1_57_0.
comment:32 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Elaborate please. What it is move semantics? Why you cannot store unique_ptrs in the current version?