Boost C++ Libraries: Ticket #8674: Futures as local named objects can't be returned with implicit move.

owner, status changed

viboes — Sat, 08 Jun 2013 16:05:33 GMT

owner changed from Anthony Williams to viboes
status new → assigned

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?

attachment set

mjklaim@… — Sat, 08 Jun 2013 16:33:17 GMT

attachment → std_future.hpp

VS2012 U2 <future> implementation

mjklaim@… — Sat, 08 Jun 2013 16:34:09 GMT

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.

viboes — Sat, 08 Jun 2013 16:50:19 GMT

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

mjklaim@… — Sat, 08 Jun 2013 16:57:46 GMT

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.

mjklaim@… — Sat, 08 Jun 2013 21:47:33 GMT

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'

viboes — Sun, 09 Jun 2013 10:24:57 GMT

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_)
        {
        }

mjklaim@… — Sun, 09 Jun 2013 11:02:31 GMT

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?

mjklaim@… — Sun, 09 Jun 2013 11:08:10 GMT

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?

mjklaim@… — Sun, 09 Jun 2013 11:16:23 GMT

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)

mjklaim@… — Sun, 09 Jun 2013 11:19:33 GMT

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.

viboes — Sun, 09 Jun 2013 12:19:22 GMT

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

mjklaim@… — Sun, 09 Jun 2013 12:20:17 GMT

I'm happy to help. :)

milestone changed

viboes — Sun, 09 Jun 2013 16:58:21 GMT

milestone To Be Determined → Boost 1.54.0

viboes — Sun, 09 Jun 2013 21:31:49 GMT

Committed in [84718]

status changed; resolution set

viboes — Wed, 12 Jun 2013 21:00:28 GMT

status assigned → closed
resolution → fixed

Committed revision [84750].