Lambda Works on Latest Visual Studio, but Doesn't Work Elsewhere

Question

So I've written a nasty lambda to satisfy a "shortest amount of code necessary to achieve this" question:

values.resize(distance(
    begin(values),
    remove_if(begin(values), end(values),
        [i = 0U, it = cbegin(intervals), end = cend(intervals)](const auto&) mutable {
        return it != end && ++i > it->first && (i <= it->second || (++it, true));
    })
));

My problem is that on Visual Studio Community 2015 Update 3 version 14.0.25425.01 this outputs the desired:

4.2 9.1 2.3 0.6 6.4 3.6 1.4 7.5

But on all the other compilers I've tried I get:

4.2 2.3 0.6 1.2 0.3 1.4 2.5 7.5

Can anyone tell me what's causing the different behavior?

I believe clang since version 3.4 produces your desired output too: [see here](http://melpon.org/wandbox/permlink/Ia69zQqk3HmuEMCf) — W.F., Dec 12 '16 at 15:21
@Havenard That's the comma operator: http://en.cppreference.com/w/cpp/language/operator_other#Built-in_comma_operator — Jonathan Mee, Dec 12 '16 at 15:24
@W.F. Great observation. I know this was *just* fixed in Visual Studio, cause if I try to run this same code [on Update 1](http://rextester.com/QMEU55701) I get the wrong answer. — Jonathan Mee, Dec 12 '16 at 15:35
You've got undefined behaviour there ... `++i > it->first && (i ` involves using i twice, once with the pre-increment operator. — UKMonkey, Dec 12 '16 at 15:37
@UKMonkey You have a source for that? The left hand side of the logical-and should complete first and I would think that after that the value of `i` would be incremented. — Jonathan Mee, Dec 12 '16 at 15:47
@W.F. Something fishy is afoot. If I use clang here it doesn't work: http://coliru.stacked-crooked.com/a/b8c6c21dcfdf5e30 An optimization problem perhaps? — Jonathan Mee, Dec 12 '16 at 15:48
@JonathanMee yes also on godbold.org clang 3.4.1 has a problem with compilation(?) PS. I think UKMonkey may be right in his diagnosis, although can't find reference right now for that... — W.F., Dec 12 '16 at 15:58
@UKMonkey OK I got a source: http://stackoverflow.com/a/4176333/2642059 this *is* defined behavior, you're thinking of when a pre-incremented value is used in a comma separated list. — Jonathan Mee, Dec 12 '16 at 15:59
@W.F. I just found a source, that is an incorrect diagnosis. This behavior is defined (although sketchy.) — Jonathan Mee, Dec 12 '16 at 16:00
Good job finding that - knew I'd seen it somewhere... in any case, "nasty lambda" is pretty accurate ;) — UKMonkey, Dec 12 '16 at 16:06

score 10 · Accepted Answer · answered Dec 12 '16 at 16:00

You are relying on the fact that the exact closure you pass into the algorithm is the one used as the predicate, but the standard allows it to be copied:

[algorithms.general]/10 (N4140): [Note: Unless otherwise specified, algorithms that take function objects as arguments are permitted to copy those function objects freely. Programmers for whom object identity is important should consider using a wrapper class that points to a noncopied implementation object such as reference_wrapper (20.9.3), or some equivalent solution. —end note ]

This is exactly what libstdc++ does. From v6.2.1:

template<typename _ForwardIterator, typename _Predicate>
_ForwardIterator
__remove_if(_ForwardIterator __first, _ForwardIterator __last,
            _Predicate __pred)
{
    __first = std::__find_if(__first, __last, __pred);
    if (__first == __last)
    return __first;
    _ForwardIterator __result = __first;
    ++__first;
    for (; __first != __last; ++__first)
    if (!__pred(__first))
        {
        *__result = _GLIBCXX_MOVE(*__first);
        ++__result;
        }
    return __result;
}

That call to std::__find_if at the start of the function copies __pred, which means that the value of i is incremented a bit within std::__find_if, but this doesn't change what's going on at the call site.

To fix this problem, you could use std::ref:

auto clos = [i = 0U, it = cbegin(intervals), end = cend(intervals)](const auto&) mutable {
    return it != end && ++i > it->first && (i <= it->second || (++it, true));
};
values.resize(distance(begin(values), std::remove_if(begin(values), end(values), std::ref(clos))));

Live demo

Gah! It's `mutable`! Why in the world would libstdc++ copy it. That's the dumbest thing I've ever seen, or at least almost as dumb as my evil lambda... — Jonathan Mee, Dec 12 '16 at 16:11
@JonathanMee the signatures of most algorithms take their functors by value, at least according to cppreference. — jaggedSpire, Dec 12 '16 at 16:48
@jaggedSpire It doesn't really matter that much how the standard algorithm accepts the argument, it's more about what it does with it internally. Which makes my code implementation defined (something I strive to avoid.) — Jonathan Mee, Dec 12 '16 at 18:48
@JonathanMee my point was more along the lines of it getting copied at least once due to the by-value parameter, and more times if it's implemented in terms of other algorithms — jaggedSpire, Dec 12 '16 at 19:03
@jaggedSpire I can't figure out why the standard would be implemented like this. I've asked a follow up question here: http://stackoverflow.com/q/41108386/2642059 — Jonathan Mee, Dec 12 '16 at 19:48

Lambda Works on Latest Visual Studio, but Doesn't Work Elsewhere

1 Answers1

Linked