Home Dashboard Directory Help
Search

VS2010 STL for_each implementation breaks existing code by RKG_1


Status: 

Closed
 as Fixed Help for as Fixed


2
1
Sign in
to vote
Type: Bug
ID: 603381
Opened: 9/22/2010 6:51:23 AM
Access Restriction: Public
1
Workaround(s)
view
0
User(s) can reproduce this bug

Description

This affects for_each as defined in <algorithm> in VS2010.

The attached code (Steps to reproduce) does not compile in VS2010. It compiles using VS2008 and older versions.

The reason is that the template arguments are not forwarded to the new inner _For_each function, resulting in the functor being passed by value instead of by reference. A modified implementation is shown below that should resolve the problem:

template<class _InIt,
    class _Fn1> inline
    _Fn1 for_each(_InIt _First, _InIt _Last, _Fn1 _Func)
    {    // perform function for each element
    _DEBUG_RANGE(_First, _Last);
    _DEBUG_POINTER(_Func);
    return (_For_each<_InIt, _Fn1>(_Unchecked(_First), _Unchecked(_Last), _Func));
    }
Details
Sign in to post a comment.
Posted by Microsoft on 1/5/2011 at 8:30 PM
Hi,

An update to my earlier reply: we eliminated an unnecessary and nonconformant functor copy in VC11. However, your code is actually nonconformant, and won't compile cleanly with VC11.

Paragraph 25.2.4 [alg.foreach]/1 of the C++0x Working Paper N3225 says that for_each() "Requires: Function shall meet the requirements of MoveConstructible (Table 36). [ Note: Function need not meet the requirements of CopyConstructible (Table 37). —end note ]" and /3 says "Returns: std::move(f)."

If you attempt to use explicit template arguments to make for_each() take a reference to a functor (and return a reference to a functor), that interacts badly with the return move(f) statement. At the very least you'll get "warning C4172: returning address of local variable or temporary". In fact, this is triggering a non-Standard extension (binding a modifiable lvalue reference to an rvalue) - it's not supposed to compile according to the Standard.

You shouldn't use explicit template arguments with STL functions, except when they're designed that way (e.g. forward<T>(t), make_shared<T>(args)). In the case of for_each(), you should pass your functor by value. If it's expensive to copy, making it movable will eliminate the expense (and our fixed implementation will properly handle this).

As always, if you have any further questions, feel free to E-mail me at stl@microsoft.com .

Stephan T. Lavavej
Visual C++ Libraries Developer
Posted by Microsoft on 11/1/2010 at 1:59 AM
Hi,

Thanks for reporting this bug. We've fixed it, and the fix will be available in VC11.

If you have any further questions, feel free to E-mail me at stl@microsoft.com .

Stephan T. Lavavej
Visual C++ Libraries Developer
Posted by Microsoft on 9/22/2010 at 7:55 PM
Thanks for your feedback.

We are rerouting this issue to the appropriate group within the Visual Studio Product Team for triage and resolution. These specialized experts will follow-up with your issue.
Posted by Microsoft on 9/22/2010 at 5:02 PM
Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)
Sign in to post a workaround.
Posted by ranta on 6/21/2012 at 6:18 AM
Use std::reference_wrapper, as in std::for_each(v.begin(), v.end(), std::ref(f));
Visual C++ 2010 SP1 then inlines it all, resulting in a nice four-instruction loop where m_n is in a register.