[Foreach] rvalue reference binding of temporary ranges

[mimomorin@…]

C++0x range-based for uses auto && to bind lvalue/rvalue references to ranges. A good thing is that there is no need to copy ranges anymore. This also allows us to deal with non-copyable rvalue ranges.

If a compiler supports rvalue references and auto type deduction (either auto or decltype is OK), Boost.Foreach can be modified to do this.

[mimomorin@…]

I will attach patches for boost/foreach.hpp and libs/foreach/test/.

[mimomorin@…]

A patch for libs/foreach/test (against trunk) to test iteration over non-copyable and non-movable rvalue ranges.

[mimomorin@…]

Hmm, I cannot attach a patch for foreach.hpp...

The patch I made does the following:

(Note: contrary to C++0x range-based for, this does not allow mutable iteration over rvalue ranges; Non-const rvalue ranges are bounded to const rvalue references.)

1. Adding a configuration macro

• BOOST_FOREACH_USE_RVALUE_REFERENCE_BINDING
  • When a compiler supports rvalue references and decltype, this macro is defined and rvalue reference binding is used.

2. Adding macros for auto declarations

• BOOST_FOREACH_AUTO_OBJECT(NAME, EXPR)
  • Equivalent to auto NAME = EXPR;.
• BOOST_FOREACH_AUTO_REF_REF(NAME, EXPR)
  • Equivalent to auto && NAME = EXPR;if EXPR is an lvalue.
  • Equivalent to auto const && NAME = EXPR;if EXPR is an rvalue.

3. Adding overloaded functions

• begin
• end
  • For C-strings, this function returns cstr_end_iterator, which is defined as an empty struct. I also overloaded operator != to support C-strings:

    template <typename Iterator>
    inline bool operator !=(Iterator cur, cstr_end_iterator)
    {
        return *cur != 0;
    }
    

• rbegin
• rend

All the regression tests ran successfully with the following compilers (both in C++03 and C++0x modes): gcc-4.3.5, gcc-4.4.5, gcc-4.5.2, gcc-4.6.0, clang (TOT).

Newly added tests (in test_noncopyable_rvalue.patch) ran successfully in a C++0x mode.

[Michel MORIN <mimomorin@…>]

A patch for foreach.hpp (against trunk) to bind temporary ranges to const rvalue references.

[Michel MORIN <mimomorin@…>]

All right, succeeded to attach patches!

[Eric Niebler]

On MSVC 9 and 10, I get:

noncopyable_rvalue_const.cpp(60) : error C2248: 'my_container::my_container' : cannot access private member declared in class 'my_container'
        noncopyable_rvalue_const.cpp(43) : see declaration of 'my_container::my_container'
        noncopyable_rvalue_const.cpp(19) : see declaration of 'my_container'

I can't apply the patch until these issues are resolved. Any ideas?

[Michel Morin <mimomorin@…>]

Thank you for seeing this.

On MSVC 9, the following error should emit…

#ifndef BOOST_FOREACH_USE_RVALUE_REFERENCE_BINDING
# error Expected failure : non-copyable rvalues disallowed
#else

So, the error message you posted is for MSVC 10?

[Eric Niebler]

Ah, yes! I was mistakenly using vc10 even in my vc9 environment. I get the proper error with vc9. The above error is with vc10.

[Michel Morin <mimomorin@…>]

OK :)

Well, I don't have access to MSVC 10, so could you compile the following codes and tell me what compiler error happens?

A.cpp

class my_container
{
public:
    my_container()
    {}

private:
    int array_[5];

    my_container(my_container const &);
    my_container &operator =(my_container const &);

    my_container(my_container &&);
    my_container &operator =(my_container &&);
};

typedef my_container const const_my_container;

int main (int argc, char* argv[])
{
    my_container const && x = const_my_container();

    return 0;
}

B.cpp

class my_container
{
public:
    my_container()
    {}

// private:
    int array_[5];

    my_container(my_container const &);
    my_container &operator =(my_container const &);

    my_container(my_container &&);
    my_container &operator =(my_container &&);
};

typedef my_container const const_my_container;

int main (int argc, char* argv[])
{
    my_container const && x = const_my_container();

    return 0;
}

Thanks in advance.

[Eric Niebler]

A.cpp

1>c:\boost\org\trunk\libs\proto\scratch\main.cpp(21): error C2248: 'my_container::my_container' : cannot access private member declared in class 'my_container'
1>          c:\boost\org\trunk\libs\proto\scratch\main.cpp(10) : see declaration of 'my_container::my_container'
1>          c:\boost\org\trunk\libs\proto\scratch\main.cpp(2) : see declaration of 'my_container'

B.cpp No error

[Eric Niebler]

P.S. If you have Windows, you have access to MSVC 10: @​http://www.microsoft.com/visualstudio/en-us/products/2010-editions/visual-cpp-express

[Michel Morin <mimomorin@…>]

Thanks for doing this.

Considering the results, I think the error happens due to the incomplete implementation of C++11 (i.e. ​DR 391) in MSVC 10. The code should compile fine in C++11.

The fix for boost/foreach.hpp could be

1. Do nothing (BOOST_FOREACH fails with noncopyable rvalues on MSVC 10).
2. Revert to the C++03 BOOST_FOREACH on MSVC 10.
3. Write a special code for MSVC 10.

I prefer A, since this does not break anything (noncopyable rvalues cannot be handled by the C++03 BOOST_FOREACH, IIUC).

For libs/foreach/test/noncopyable_rvalue_***.hpp, I can change them to pass the tests on MSVC 10 by declaring copy/move ctors and copy/move assignment operators public. But, wouldn't it be better to fail the tests on MSVC 10 to explicitly state that noncopyable rvalues are not supported?

P.S. Will try to setup Visual Studio 2010 Express. Thanks!

[Eric Niebler]

(A) would be fine, I think. We can mark up the expected failures specially. But if you think this is a bug in MSVC, you *must* file a bug here: <​https://connect.microsoft.com/VisualStudio/>. (You may need to open an account there to do so.) Use the small repro case you sent me and refer to the relevant part of the standard. Also, double-check that Comeau and GCC accept the code in 0x mode. You can test with Comeau here: <​http://www.comeaucomputing.com/tryitout/>. Let me know if you have any problems. Let's get this fixed.

[Eric Niebler]

I considered just applying this change, msvc10 be damned, but then I realized that I don't know enough about C++11 to say whether the code is well-formed, and Comeau Online rejected your A.cpp source above. Comeau C++11 support is far from perfect, but it makes me uneasy that both msvc and Comeau reject the code. Can you quote the relevant parts of the standard that make A.cpp well-formed?

[Michel Morin <mimomorin@…>]

I tried Comeau Online and got compilation errors. But it seems that Comeau doesn't recognize rvalue references...

The relevant part is 8.5.3 p5:

If the initializer expression

• is an xvalue, class prvalue, array prvalue or function lvalue and “cv1 T1” is reference-compatible with “cv2 T2”, or
• has a class type (i.e., T2 is a class type), where T1 is not reference-related to T2, and can be implicitly converted to an xvalue, class prvalue, or function lvalue of type “cv3 T3”, where “cv1 T1” is reference-compatible with “cv3 T3”,

then the reference is bound to the value of the initializer expression in the first case and to the result of the conversion in the second case (or, in either case, to an appropriate base class subobject). In the second case, if the reference is an rvalue reference and the second standard conversion sequence of the user-defined conversion sequence includes an lvalue-to-rvalue conversion, the program is ill-formed.

See also links in this answer ​http://stackoverflow.com/questions/1827049/

[Michel Morin <mimomorin@…>]

The problem in MSVC is that it does the access check of the copy constructor when binding references to rvalues even when the check should not occur. (In C++03, the check is legal.)

Here is a minimal code that demonstrates this.

class noncopyable
{
public:
    noncopyable()
    {}

private:
    noncopyable(noncopyable const &);
};

int main (int argc, char* argv[])
{
    noncopyable const& x = noncopyable();

    return 0;
}

This code fails to compile on MSVC, but compiles fine on Comeau.

[Michel Morin <mimomorin@…>]

Replying to eric_niebler:

But if you think this is a bug in MSVC, you *must* file a bug here: <​https://connect.microsoft.com/VisualStudio/>.

Done! ​https://connect.microsoft.com/VisualStudio/feedback/details/690244/

[Michel Morin <mimomorin@…>]

Replying to Michel Morin <mimomorin@…>:

See also links in this answer ​http://stackoverflow.com/questions/1827049/

Specifically, I meant this link ​http://gcc.gnu.org/bugs/#cxx%5Frvalbind

[Eric Niebler]

Committed to trunk in [74584].

[Eric Niebler]

(In [74591]) merge [74584] from trunk, fixes #5475

[Michel Morin <mimomorin@…>]

Thanks for resolving this ticket, Eric!

P.S. The bug reported to Microsoft Connect has been resolved saying "this issue has been fixed. The fix should show up in a future release of Visual C++."

[Eric Niebler]

Very glad to hear it. And thanks for your contribution to Boost!

[Eric Niebler]

There seems to be a problem with this on gcc-4.4.3. See, for instance, ​this. Michel, can you please look into this soon? We're about to lock down the release branch for 1.48, and I hate to regress Boost.Foreach.

[Michel Morin <mimomorin@…>]

I'm investigating now.

[Michel Morin <mimomorin@…>]

The current implementation of C++11 BOOST_FOREACH is based on decltype. However, gcc-4.4 has a bug that prevents us from using decltype in BOOST_FOREACH. (The bug has been fixed in gcc-4.5 and newer versions.)

The reason I used decltype rather than auto is that

• auto && is buggy in gcc-4.4.*, 4.5.*, 4.6.0 and so we should not use it. (This bug is already fixed in gcc-4.6.1 and 4.7.)
• decltype is more widely available than auto.
• binding rvalue ranges to const references can be easily implemented with decltype than with auto.

Short-term fixed would be:

1. Revert to C++03 BOOST_FOREACH on gcc-4.4
2. Disable const reference biding of rvalue ranges on gcc-4.4. That is, rvalue ranges are bound to const or non-const rvalue ranges using auto &&. In spite of the above bug, auto && can be safely used in gcc-4.4, since lvalues are allowed to be bound to rvalue references in gcc-4.4.

A and B are extremely easy to do without causing any error.

Long-term fixed would be

3. Implement "const reference binding of rvalue ranges" with auto && on gcc-4.4. I already implemented it in this ​post.

Comments? I'm ready to make patches.

[Michel Morin <mimomorin@…>]

A patch for fixing gcc-4.4 regression (against trunk). This patch implements C of comment 23.

[Michel Morin <mimomorin@…>]

Attached a patch that implements C of comment 23. This patch should only affect a C++0x mode of gcc-4.4.

I ported the code in this ​post. But, due to auto&& bug in gcc-4.4, I needed to change the code for adding const qualification to rvalue ranges. With this patch, BOOST_FOREACH(DECL, EXPR) does the following in a C++0x mode of gcc-4.4:

1. Expand EXPR's lifetime

   auto && tmp = EXPR
   

2. Add const if EXPR is rvalue

   auto && rng = add_const_if< boost::is_rvalue_reference<decltype( (EXPR) )&&> >(tmp)
   

3. Do the usual range-based for loop over rng

libs/foreach/test/ ran successfully on gcc (4.3 - 4.6 and 4.7 (experimental)) and clang (3.1(trunk) and Apple version 2.1) both in C++03 and C++11 modes. I also tested libs/spirit/test/karma/actions.cpp on gcc-4.4 in a C++11 mode and it ran fine.

[Eric Niebler]

Great! I've applied the patch to both trunk and (throwing caution to the wind) to release. Thanks once again for your help.

[Michel Morin <mimomorin@…>]

Sorry, Eric. Could you revert the previous patch? I found a regression caused by the patch (which was not caught by the test).

I've realized that gcc-4.4 is really a mess to implement "const reference binding of rvalue ranges". We shouldn't try to implement it unless we have reliable test. Bugs in gcc-4.4 are appeared when using BOOST_FOREACH in function templates. We should extend the existing test to cover those cases.

So it seems that we have only two options for this release: A or B of comment 23.

[Michel Morin <mimomorin@…>]

Patch that implements A of comment 23 (against trunk before applying the previous patch).

[Michel Morin <mimomorin@…>]

Attached a patch that implements A (reverting to the C++03 BOOST_FOREACH on gcc-4.4).

I've found that even with the simplest implementation of B

...
if (bool is_rng_defined) = false) {} else
for (auto &&rng = (EXPR); !is_rng_defined; is_rng_defined = true)
...

we can make a regression due to the nasty auto bug in gcc-4.4. So we have only one choice: A.

[Eric Niebler]

OK, done. Thanks again.

[Michel Morin <mimomorin@…>]

On gcc-4.3 (C++11), I found a regression that was not caught by the test. Due to auto type deduction bugs (decltype) in template functions, the following code does not compile on gcc-4.3 (C++11) with Boost trunk

#include <iostream>
#include <vector>
#include <boost/foreach.hpp>

template<typename T>
typename boost::mpl::if_<boost::is_rvalue_reference<T&&>, T const, T>::type
add_const_if_rvalue(T&& t)
{
    return t;
}

template <typename T>
void f()
{
    BOOST_FOREACH(int const& i, add_const_if_rvalue(std::vector<int>(3, 0))) {
        std::cout << i << std::endl;
    }
}

int main (int argc, char* argv[])
{
    f<void>();
    return 0;
}

This code compiles fine on gcc-4.3 (C++11) with Boost.1.47. It also compiles fine on gcc-4.5, 4.6, 4.7 (C++11) with Boost trunk.

Do we need to revert to the C++03 BOOST_FOREACH on gcc-4.3 too? Patch is pretty simple.

boost/foreach.hpp

@@ -34 +34 @@
 // With these C++0x features, temporary collections can be bound to 
 // rvalue references and their lifetime is extended. No copy/move is needed.
 #if !defined(BOOST_NO_DECLTYPE) && !defined(BOOST_NO_RVALUE_REFERENCES)                          \
- && !(BOOST_WORKAROUND(__GNUC__, == 4) && (__GNUC_MINOR__ == 4) && !defined(BOOST_INTEL) &&      \
+ && !(BOOST_WORKAROUND(__GNUC__, == 4) && (__GNUC_MINOR__ <= 4) && !defined(BOOST_INTEL) &&      \
                                                                    !defined(BOOST_CLANG))
 # define BOOST_FOREACH_USE_RVALUE_REFERENCE_BINDING
 // Some compilers let us detect even const-qualified rvalues at compile-time

[Eric Niebler]

Thanks for your thoroughness on this issue. It's much appreciated. I've made your suggested change on trunk and release.

[Michel Morin <mimomorin@…>]

*Regressions for compilers supporting C++11 lambda*

Because the current C++11 BOOST_FOREACH uses decltype rather than auto, it cannot be used with C++11 lambda. This is possibly a serious regression. The following code compiles fine on VC++10 and gcc 4.5-4.7 with Boost 1.47, but fails to compile with Boost trunk:

#include <iostream>
#include <vector>
#include <boost/foreach.hpp>
#define BOOST_RESULT_OF_USE_DECLTYPE
#include <boost/range/adaptor/transformed.hpp>

int main (int argc, char* argv[])
{
    std::vector<int> v(3, 0);
    
    BOOST_FOREACH(int x, v | boost::adaptors::transformed([](int i){ return i + 1; }))
    {
        std::cout << x << std::endl;
    }
        
    return 0;
}

I know the release branch is already closed, but how about deferring C++11 support to Boost 1.49?

Sorry for repeatedly troubling you. I should have noticed this regression earlier and I shouldn't have used decltype to workaround gcc's auto bugs.

[Michel Morin <mimomorin@…>]

To support the C++11 BOOST_FOREACH, we need to address the following:

• Extending test cases

Especially, use BOOST_FOREACH in function templates and use BOOST_FOREACH with C++11 lambda.

• Workaround gcc's auto declaration bugs in function templates

Except gcc 4.7, gcc's auto declarations in function templates do not work correctly. Even with gcc 4.6.1,

    auto&& rng = add_const_if_rvalue(std::vector<int>(3, 0));

fails to compile (in function templates).

It is possible that compilers have other unkown bugs on auto declarations, and so it might be reasonable to have a way to avoid regressions:

• Adding configuration macro BOOST_FOREACH_USE_AUTO_DECLARATIONS

The C++11 BOOST_FOREACH gets enabled on compilers with rvalue references and auto support, only if BOOST_FOREACH_USE_AUTO_DECLARATIONS is defined by users.

[Eric Niebler]

This change has made me nervous, so I have completely reverted all changes for the 1.48 release. We'll give it another go in 1.49.

[Michel Morin <mimomorin@…>]

A patch (against trunk) that implements C++11 BOOST_FOREACH and workarounds for auto declaration bugs in gcc 4.4-4.6.

[Michel Morin <mimomorin@…>]

A patch (against trunk) that adds tests for gcc bugs and C++11 lambdas. This patch also adds tests for noncopyable rvalue collections.

[Michel Morin <mimomorin@…>]

Finally, I came up with workarounds for gcc's auto type deduction bugs. Now BOOST_FOREACH consists of three implementations: C++03 BOOST_FOREACH, C++11 BOOST_FOREACH and workarounds for gcc bugs. All the existing tests and new tests (involving function templates and C++11 lambdas) ran successfully on gcc and clang.

To workaround gcc bugs, I used a hybrid of C++11 and C++03 codes:

1. Bind a const reference to a range using auto const&. If the range is non-const lvalue, constness of the reference is stripped off via const_cast when accessing the reference.
2. begin and end iterators are stored as auto_any_t to avoid the bugs.

This workaround code is enabled in C++11 modes of gcc 4.4-4.6.

On MSVC++ 10, I changed to use the C++03 BOOST_FOREACH code. Since this compiler doesn't support iteration over noncopyable rvalue ranges, it's not worth taking a risk for regressions.

I also added a few tests:

• noncopyable_rvalue_*.cpp: This test emits expected-failures if BOOST_FOREACH_USE_CXX11 or BOOST_FOREACH_USE_CXX11_GCC_WORKAROUND is not defined.
• cxx11_gcc_workaround_*.cpp: This test needs support of rvalue references. On compilers without rvalue reference support, this test succeeds unconditionally.
• cxx11_lambda_*.cpp: This test needs support of C++11 lambdas. On compilers without C++11 lambdas support, this test succeeds unconditionally.
• char_array_fail_*.cpp: A test case that should fail to compile (compile-fail is specified in Jamfile). This test tries to iterating over a char array.
• rvalue_nonconst_access_fail_*.cpp: A test case that should fail to compile (compile-fail is specified in Jamfile). This test tries to capture elements in an rvalue range by non-const references.

Patches for boost/foreach.hpp and libs/foreach/test/ are attached.

[Michel Morin]

Updated patch of foreach_use_cxx11.patch. With this patch, we can iterate over noncopyable temporary ranges on VC++ 11 as well.

[Michel Morin]

I confirmed that VC++ 11 (beta) fixed the bug in reference binding of noncopyable temporaries. So we can iterate over noncopyable rvalue ranges on VC++ 11. I ran the tests on VC++11 with my previous patch, but some tests failed because of VC++'s bug in two-phase name look-up.

So, again, I updated the patch. Now, all the tests (incl. tests for noncopyable/nonmovable rvalue ranges and C++11 lambdas) passed on VC++ 11, gcc 4.4-4.7, clang 2.9-3.1.

Updated patch attached in this ticket: ​https://svn.boost.org/trac/boost/attachment/ticket/5475/foreach_use_cxx11_v2.patch

Also, patch for the tests attached in this ticket: ​https://svn.boost.org/trac/boost/attachment/ticket/5475/foreach_add_new_tests.patch

[Michel Morin]

Before gcc 4.7.1 and gcc 4.8.0 (not yet released), auto type deduction in function templates is done differently than the one in non-template functions. This resulted in many bugs in gcc 4.4-4.6 for auto type deduction in function templates. So my patch does workarounds for those compilers.

In gcc 4.7.0, I don't know of any bugs that affect (normal) C++11 implementation of BOOST_FOREACH. But gcc 4.7.0 still contains bugs of auto type deduction in function templates (c.f. ​PR53484 ("Wrong auto in lambdas in function templates")), and there might be a bug that needs workarounds as in gcc 4.4-4.6.

Would it be better to do workarounds for gcc 4.7.0, too? (My patch does not apply workarounds for gcc 4.7.0.)

[Eric Niebler]

I'll be honest with you, Michel. At this point, given the sheer size and complexity of your patch, and the number of broken compilers that half-implement C++11 that I need to support, and the past difficulties we experienced trying to get this fix to fly, it's looking highly doubtful that I'll be taking this patch. Especially considering that C++11 users have a better option: use C++11's range-based for. I can tell you've invested a lot of effort into this, and I greatly appreciate it.

I would very much like to move BOOST_FOREACH into maintenance mode. It's a relic. I don't expect I'll be adding any new features to it.

I'll leave the bog open for now and may reconsider. But for now, I wouldn't spend any more time on this if I were you.

[Michel Morin]

I would very much like to move BOOST_FOREACH into maintenance mode. It's a relic. I don't expect I'll be adding any new features to it.

I think it's reasonable that BOOST_FOREACH becomes in a maintenance mode.

I'll leave the bog open for now and may reconsider. But for now, I wouldn't spend any more time on this if I were you.

OK. I'll dive into the C++11 world as you do rather than struggle with half-broken C++11 compilers. That would be more fun ;)

Thanks for your reply, Eric!

[Eric Niebler]

Closing this out. I won't be adding any more features to BOOST_FOREACH. Thanks for your efforts, Michel.