Boost C++ Libraries: Ticket #7888: circular_buffer should support move semantics

anonymous — Mon, 14 Jan 2013 12:15:06 GMT

Elaborate please. What it is move semantics? Why you cannot store unique_ptrs in the current version?

holmes@… — Mon, 14 Jan 2013 12:59:03 GMT

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&&.

anonymous — Tue, 19 Mar 2013 22:11:18 GMT

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.

anonymous — Wed, 24 Apr 2013 13:51:15 GMT

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.

owner, status changed

Antony Polukhin — Mon, 01 Jul 2013 15:06:49 GMT

owner changed from Jan Gaspar to Antony Polukhin
status new → assigned

Antony Polukhin — Tue, 02 Jul 2013 13:51:57 GMT

(In [84941]) Basic commit of C++11 move constructor and move assignment for circular_buffer (refs #7888). some of the functions marked with BOOST_NOEXCEPT

Antony Polukhin — Mon, 08 Jul 2013 13:41:59 GMT

(In [84984]) Add basic rvalues move support for elements of circular_buffer (refs #7888). Patch uses Boost.Move to emulate rvalues in C++03

Antony Polukhin — Tue, 09 Jul 2013 09:32:46 GMT

(In [84988]) Improved rvalues move support for elements of circular_buffer (refs #7888):

move_if_noexcept uses is_copy_constructible trait
more tests, tests are now more strict
linearize() now works with move-only types

Antony Polukhin — Tue, 09 Jul 2013 15:06:17 GMT

(In [84991]) Improved rvalues move support for elements of circular_buffer (refs #7888):

set_capacity and rset_capacity now work with move-only types if move constructor of type marked with noexcept
force placement ::new usage
minor optimizations (move values in more cases)
more tests

Antony Polukhin — Thu, 11 Jul 2013 14:55:25 GMT

(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

Antony Polukhin — Wed, 17 Jul 2013 14:09:33 GMT

(In [85059]) Fix errors in circular_buffer tests(refs #7888).

Antony Polukhin — Fri, 19 Jul 2013 14:41:42 GMT

(In [85082]) Fixed my own typo (refs #7888)

Antony Polukhin — Mon, 22 Jul 2013 11:44:12 GMT

(In [85101]) Fixed MSVC related bug for rvalues move support of circular_buffer (refs #7888)

Antony Polukhin — Tue, 23 Jul 2013 16:06:39 GMT

(In [85133]) Fix errors in circular_buffer tests(refs #7888).

Antony Polukhin — Thu, 08 Aug 2013 15:32:12 GMT

(In [85240]) Updated documentaion of the circular_buffer to reflect the rvalue references support (refs #7888) + replaced some tabs with whitespaces and added the boost.root parameter to jamfile.v2

Antony Polukhin — Sun, 25 Aug 2013 12:36:52 GMT

(In [85458]) Make move_if_noexcept more strict and move values only if they have noexcept move constructor *and* noexcept move assignment operator (refs #7888)

status changed; resolution set

Antony Polukhin — Thu, 29 Aug 2013 08:39:12 GMT

status assigned → closed
resolution → fixed

(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)

pbf@… — Tue, 12 Nov 2013 21:28:31 GMT

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:

/media/sf_C_DRIVE/src/svn/OTI/ThirdParty/boost_1_55_0_OT1/include/boost/circular_buffer/base.hpp:2157: error: suggest parentheses around ‘&&’ within ‘

’

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.

Antony Polukhin — Tue, 12 Nov 2013 21:51:06 GMT

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.

status changed; resolution deleted

Antony Polukhin — Tue, 12 Nov 2013 21:51:24 GMT

status closed → reopened
resolution fixed

michael.broida@… — Thu, 22 May 2014 22:54:30 GMT

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?

Antony Polukhin — Fri, 23 May 2014 10:21:26 GMT

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.

michael.broida@… — Fri, 23 May 2014 15:27:56 GMT

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. :)

Antony Polukhin — Fri, 23 May 2014 17:33:39 GMT

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?

michael.broida@… — Fri, 23 May 2014 18:00:11 GMT

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.

michael.broida@… — Fri, 23 May 2014 18:04:54 GMT

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.

anonymous — Fri, 23 May 2014 18:41:44 GMT

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.

michael.broida@… — Fri, 23 May 2014 20:56:58 GMT

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. :(

Antony Polukhin — Sat, 24 May 2014 14:16:45 GMT

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! :)

Thu, 16 Oct 2014 14:41:04 GMT

Using Boost 1.56.0 warning C4172 is still reported by VS2010.

milestone changed

Antony Polukhin — Fri, 17 Oct 2014 07:53:17 GMT

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.

status changed; resolution set

Antony Polukhin — Sat, 25 Oct 2014 09:48:12 GMT

status reopened → closed
resolution → fixed