Opened 9 years ago
Closed 9 years ago
#8674 closed Bugs (fixed)
Futures as local named objects can't be returned with implicit move.
Reported by: | Owned by: | viboes | |
---|---|---|---|
Milestone: | Boost 1.54.0 | Component: | thread |
Version: | Boost 1.53.0 | Severity: | Problem |
Keywords: | Cc: |
Description
I could test only with VS 2012 Update 2 so this might be related to a compiler bug, but I can't test the boost code with other compilers/platforms right now.
I have this simple test:
#include <iostream> #define USE_STD 0 #define USE_BOOST 1 #define USED_THREAD_API USE_BOOST #if USED_THREAD_API == USE_BOOST # define BOOST_THREAD_VERSION 4 # include <boost/thread/future.hpp> using boost::future; using boost::async; #endif #if USED_THREAD_API == USE_STD # include <future> using std::future; using std::async; #endif future<void> do_something() { auto result = async( []{ std::cout<< "A\n"; } ); std::cout << "B\n"; return result; // error here } int main() { do_something().wait(); std::cout << "Hello, World!" << std::endl; }
Both default Debug and Release modes fail to compile on VS2012 U2:
1>------ Build started: Project: Test_MoveReturnFuture, Configuration: Debug Win32 ------ 1> main.cpp 1>e:\projects\tests\test_movereturnfuture\test_movereturnfuture\main.cpp(29): error C2248: 'boost::future<R>::future' : cannot access private member declared in class 'boost::future<R>' 1> with 1> [ 1> R=void 1> ] 1> e:\projects\sdk\boost\boost\include\boost-1_53\boost\thread\future.hpp(1406) : see declaration of 'boost::future<R>::future' 1> with 1> [ 1> R=void 1> ] ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
Changing this line:
#define USED_THREAD_API USE_BOOST
To this:
#define USED_THREAD_API USE_STD
Makes it compile and execute fine.
I see that there are move constructors in the definition of future so I don't know at all what's the difference between the two implementations.
Another way to avoid the problem is to explicitly move the future:
future<void> do_something() { auto result = async( []{ std::cout<< "A\n"; } ); std::cout << "B\n"; return std::move(result); }
Which is what I have been doing in my code because I was initially thinking it was a compiler bug, but this Q/A made me reconsider: http://stackoverflow.com/questions/4986673/c11-rvalues-and-move-semantics-confusion
I cannot test Boost 1.54 at the moment.
Attachments (1)
Change History (16)
follow-up: 3 comment:1 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 6 comment:2 by , 9 years ago
I copy/pasted the <future> file code into the attached one. It's the one provided with VS2012 U2. I don't use the community betas.
Note that the problem might be a VS compiler bug that might be fixed in the community beta. If someone can test the same code with the community beta it would help.
comment:3 by , 9 years ago
Replying to viboes:
IMO, the code is correct. I have tested it with gcc-4.7.2/4.8.0, clang-3.2 and it works as expected with both defines USED_THREAD_API USE_BOOST or USED_THREAD_API USE_STD.
Could you show the implementation of std::future constructors of the library you are using?
I have tried with VS 2010, but there is no <future> header. I confirm the bug with VS 2010 and #define USED_THREAD_API USE_BOOST. Unfortunately I've a windows XP and I can not install VS 2012 :(
comment:4 by , 9 years ago
Note that I'm using VS2012 Update 2 but there is Update 3 Release Candidate. The log don't seem to have anything related to this bug.
Also, I'm using 64 and 32 bits and have the error whatever the case.
comment:5 by , 9 years ago
I asked on a french C++ forum if someone could test with VS2012 November CTP1 (which add several c++11 features but can't be used safely in production).
Loic Joly reported the same error (which copying here);
1>------ Début de la génération*: Projet*: TestFutureCTP (Microsoft Visual C++ Compiler Nov 2012 CTP), Configuration*: Debug Win32 ------ 1> 'Microsoft Visual C++ Compiler Nov 2012 CTP' is for testing purposes only. 1> Source.cpp 1> ****\visual studio 2012\projects\testfuturectp\source.cpp(29): error C2248: 'boost::future<void>::future' : cannot access private member declared in class 'boost::future<void>' 1> d:\boost\boost_1_53_0\boost\thread\future.hpp(1406) : see declaration of 'boost::future<void>::future'
comment:6 by , 9 years ago
Replying to mjklaim@…:
I copy/pasted the <future> file code into the attached one. It's the one provided with VS2012 U2. I don't use the community betas.
Note that the problem might be a VS compiler bug that might be fixed in the community beta. If someone can test the same code with the community beta it would help.
I wonder if the following constructor is the one that is been used by the compiler.
1126 future(const _Mybase& _State) 1127 : _Mybase(_State, true) 1128 { // construct from associated asynchronous state object 1129 }
This will clearly be a compiler bug (and indirectly a library bug).
Could you try adding this public constructor to Boost.Thread to confirm it
BOOST_THREAD_FUTURE(base_type const& b): base_type(b.future_) { }
comment:7 by , 9 years ago
Could you try adding this public constructor to Boost.Thread to confirm it
If I add this code it don't compile and give the same exact error. But the error points to this line:
BOOST_THREAD_MOVABLE_ONLY(BOOST_THREAD_FUTURE)
Obviously, removing it (and keeping the modification) makes it compile.
I don't know if it works though, I would have to recompile boost.thread to check on runtime. Isn't this modification duplicating the value?
comment:8 by , 9 years ago
I checked by debugging step-by-step the std::future behaviour and it does use the move constructor, not the other one. So no, it's not using the base-copy constructor.
Maybe boost.thread futres's move-only macro is hiding the move constructor in some ways?
comment:9 by , 9 years ago
Ok I found something. In delete.hpp:
/** * BOOST_THREAD_DELETE_COPY_CTOR deletes the copy constructor when the compiler supports it or * makes it private. * * BOOST_THREAD_DELETE_COPY_ASSIGN deletes the copy assignment when the compiler supports it or * makes it private. */ #ifndef BOOST_NO_CXX11_DELETED_FUNCTIONS #define BOOST_THREAD_DELETE_COPY_CTOR(CLASS) \ CLASS(CLASS const&) = delete; \ #define BOOST_THREAD_DELETE_COPY_ASSIGN(CLASS) \ CLASS& operator=(CLASS const&) = delete; #else // BOOST_NO_CXX11_DELETED_FUNCTIONS #define BOOST_THREAD_DELETE_COPY_CTOR(CLASS) \ private: \ CLASS(CLASS&); \ // <--- HERE public: #define BOOST_THREAD_DELETE_COPY_ASSIGN(CLASS) \ private: \ CLASS& operator=(CLASS&); \ // <--- HERE public: #endif // BOOST_NO_CXX11_DELETED_FUNCTIONS
Notice that the non-c++11 implementation define private copy constructor and assignation using non-const references.
#define BOOST_THREAD_DELETE_COPY_CTOR(CLASS) \ private: \ CLASS(CLASS const &); \ // <--- HERE public: #define BOOST_THREAD_DELETE_COPY_ASSIGN(CLASS) \ private: \ CLASS& operator=(CLASS const &); \ // <--- HERE public:
This compiles for me. (I just can't run it right now because it requires recompiling boost.thread)
comment:10 by , 9 years ago
Just to clarify: that it works without modification of the future class.
So basically, these private constructor/assignation were selected as the right match instead of the moving constructor/assignation, just because they had non-const reference. I'm a bit lost on if it's a compiler bug or not.
comment:11 by , 9 years ago
Thanks for the analysis. I have checked the const reference patch with VS 2010 and it works now.
I will run the whole regression test before committing these changes.
Thanks again, Vicente
comment:13 by , 9 years ago
Milestone: | To Be Determined → Boost 1.54.0 |
---|
comment:15 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Committed revision [84750].
IMO, the code is correct. I have tested it with gcc-4.7.2/4.8.0, clang-3.2 and it works as expected with both defines USED_THREAD_API USE_BOOST or USED_THREAD_API USE_STD.
Could you show the implementation of std::future constructors of the library you are using?