5

The reason for the question is that I've seen code like this:

auto fun(std::vector<Foo>&& v) {
    std::vector<Bar> w;
    for (auto&& e : v /* not an rvalue, but keep reading */) {
        w.push_back(std::move(e));
    }
    // do stuff with w
}

which is marked as erroneous by static analisys tools, as the forwarding reference e is being std::moved instead of being std::forwarded.

On the other hand, v binds for sure to a prvalue or an xvalue (something the client knows to be or wants fun to treat as a temporary), because its type is an rvalue reference. Yes, I see that the body of the function doesn't state in any way that v cannot be used after the for loop, but that would only lead me to think that I should change

  • for (auto&& e : v) to for (auto&& e : std::move(v)),
  • and auto&& to E&&, assuming something along the lines of using E = std::decay_t<decltype(v)>::value_type;.

As far as I've understood, the first point doesn't have the effect I would have expected. In fact, std::move seems to have no effect as far as the for is concerned. In turn, e keeps being initialized from an lvalue (at least if the frequent case that operator[] returns a reference for the type of v), and the second point simply causes a compilation error.

As an additional reference, the note ¹ from this answer reads (with reference to range-for loops)

You cannot detect if you are iterating over a temporary (or other rvalue)

which seems to confirm that I just can't do that.

But looking at how a range-for loop is desugared, what would be wrong in changing range-declaration = *__begin; to range-declaration = std::move(*__begin); when range-expression is an rvalue?

Enlico
  • 23,259
  • 6
  • 48
  • 102
  • `std::forward` is used for perfect forwarding when you have argument controlled by deducible template parameter. `std::forward` is kind of smart version of `std::move`, so in your scenario it doesn't give anything. – Marek R Dec 07 '22 at 16:34
  • 4
    `v` isn't an rvalue, it has a name so it is an lvalue. You also can't gaurantee that `v` is bound to a temporary because you can do things like: `std::vector bar = { stuff }; fun(std::move(bar));` – NathanOliver Dec 07 '22 at 16:34
  • @NathanOliver, surely `v` isn't an rvalue, but `std::move(v)` is. As regards the temporary-ness of the argument corresponding to `v`, clearly the caller of `fun(std::move(bar))` knows they're saying "consider this a temporary". – Enlico Dec 07 '22 at 16:37
  • 1
    `std::move(v)` is also not a temporary, as it returns a reference and all references are lvalues. – NathanOliver Dec 07 '22 at 16:37
  • @NathanOliver, ok, I might have worded it wrong, but still `std::move(v)` is an xvalue, so with `std::move` I'm expressing that whatever I'm handing it too can steal stuff from it. – Enlico Dec 07 '22 at 16:40
  • The fact that you can move a container doesn't mean you can move from it's contained values. See e.g. `std::set<>`, which has `const` keys. – lorro Dec 07 '22 at 16:44
  • @Enlico For most values, nothing would happen. (I do have classes around where it's different and where `T(const T&&)` is implemented, but it's only to avoid shared_ptr for it.) What I was raising, it's not a generic solution that solves the entire problem. Perhaps we could use a `std::for_each_move()` algorithm that containers could specialize... – lorro Dec 07 '22 at 16:50
  • 2
    Even if you manage to move `v`, `std::vector` does not have r-value overload for `begin()` or `end()`, so the issue is more with initialization of the begin/end iterator (for the standard snippet). Moving the iterator does not create a move iterator, it just creates an other standard iterator. What you would need here is a way to create a moveable view of your container where `begin()` and `end()` would return move iterator similar to `make_move_iterator`. – Holt Dec 07 '22 at 16:54
  • I'd suggest just write `std::move(v)` in your example for better clarity. – apple apple Dec 07 '22 at 17:26
  • and I believe `*__begin` should return rvalue if *the iterator itself* see appropriate. a temporary container can pretty well held non-temporary data. – apple apple Dec 07 '22 at 17:29

1 Answers1

5

the container is rvalue doesn't means what it contains is

// note: in real code this could be T&& and have no idea it's a std::span
void foo(std::span<X>&& s){
    // explicitly std::move should be required
    // because the code may want to use `s` later
    for(auto&& v : std::move(s)){
        // if the for loop forward the value category of <range-expression> to items
        // then decltype(v) is X&&, and this wrongly move-out the data
        X t = std::forward<decltype(v)>(v); 
    }
}

int main(){
    X x[2];
    foo(x);
}

the decision should be done by

* the container/adaptor should always return move_iterator, overload for rvalue begin/end would not work in current standard, the normal one is always called. Plus no standard container support them either.

apple apple
  • 10,292
  • 2
  • 16
  • 36
  • So I guess that in c++17 and c++20 probably the most practical way is to give up on the range-based `for` and simply do `for (auto i = std::make_move_iterator(v.begin()); i != std::move_iterator(v.end()); ++i) {` so in the statement any `*i` will be an rvalue. – Enlico Dec 12 '22 at 09:30
  • 1
    @Enlico I'd say the the most practical way is just `std::move` it. the static analyzer fail to recognize a correct pattern doesn't make it invalid. – apple apple Dec 12 '22 at 14:49
  • Actually, would you clarify your first comment in the code? I don't understand what you're trying to say, as generally the wanting to be able to use an entity past an expression means I'm not `std::move`ing it in that expression. – Enlico Jan 11 '23 at 08:32
  • @Enlico it's in respond of `/* not an rvalue, but keep reading */` in your question. – apple apple Jan 11 '23 at 14:03
  • My comment was to say _since I'm not `std::move`ing `v`, I know I shouldn't even expect `auto&&` to resolve to `Foo&&`, but if you keep reading you'll see that even `std::move`ing `v`, `auto&&` keeps not being deduced as `Foo&&`_. – Enlico Jan 11 '23 at 16:08
  • 1
    @Enlico yes I understood that correctly, I just point out that it will need `std::move` (or `std::forward`) *even after apply what you suggest*. – apple apple Jan 11 '23 at 16:17