[BUG] Suspicious cpp1 std::move of returning expression

Describe the bug
Hello, first, let me thank you all for great work.

I was learning language from the test cases, and something caught my eye in this line

cppfront/regression-tests/test-results/pure2-synthesize-rightshift-and-rightshifteq.cpp

Line 24 in feff640

return std::move(x) >> 1;

I'm not C++ expert but I believe std::move(x) >> 1 doesn't make much sense in cpp1. It compiles, but I think move is lost to nowhere. (May be I am wrong and x is still marked as to be moved? But I think not)

To Reproduce
I have created this test:

dup: (a:_) -> _ = {
  x := a;
  return x + x;
}

inc: (a:_) -> _ = {
  x := a;
  return x + 1;
}

main: () -> int = {
  obj := 42;
  std::cout << dup(obj) << '\n';
  std::cout << inc(obj);
}

which generates following cpp1:

[[nodiscard]] auto dup(auto const& a) -> auto{
  auto x {a}; 
  return x + std::move(x); 
}

[[nodiscard]] auto inc(auto const& a) -> auto{
  auto x {a}; 
  return std::move(x) + 1; 
}

I believe both of those cases loses information about move.

Additional context
Example uses int, but I have checked with custom type (with operator+) and it behaves the same.
Also its the same for other operators like *, >> etc.

return x + std::move(x);

This is bad, as the move could happen before the copy.

7
#
The postfix-expression is sequenced before each expression in the expression-list and any default argument.
The initialization of a parameter, including every associated value computation and side effect, is indeterminately sequenced with respect to that of any other parameter.

For the sake of completeness, I have though about another example:

join: (a:_, b:_) -> _ = {
  x := a;
  y := b;
  return x + y;
}

which gives

  auto x {a}; 
  auto y {b}; 
  return std::move(x) + std::move(y);

Once again I don't know if that's ok. Should it be std::move(x+y) or move(move(x) + move(y)) or is that fine now.

It's fine as is. I'd suggest checking out https://www.youtube.com/watch?v=knEaMpytRMA&list=PLHTh1InhhwT6c2JNtUiJkaH8YRqzhU7Ag&index=113.

So if std::move is sort of cast to &&, does it boils down to operator+ having (&&) overload? What if custom class doesn't have it? Is then result of + operation needed to be marked with std::move again? Sorry may be I am overthinking that, and examples on std::move usually work with just one value, not operations on them. Anyway I hope spotting x + std::move(x) still helpful here.

operator+'s parameters are usually const&, and you can initialize those with rvalues.

For parameters yes, but I got impression that std::move on return value was needed to enable some compiler optimization on result object, in that case on result of operator+

@JohelEGP

Thanks for the note. Generally the status quo is as-intended, the move from last use should be innocuous if the target doesn't make use of the rvalue-ness.

However, @JohelEGP correctly points out:

return x + std::move(x);

This is bad, as the move could happen before the copy.

Ah yes, we didn't get l-to-r OOEE for function arguments. I should suppress move from last use when there is another use of the variable in the same expression.

Ah yes, we didn't get l-to-r OOEE for function arguments. I should suppress move from last use when there is another use of the variable in the same expression.

According to

[Note 8: If an operator function is invoked using operator notation, argument evaluation is sequenced as specified for the built-in operator; see [over.match.oper].
— end note]

there are some exceptions.

Right, I should have said in the same top-level function-call-expression... not necessarily just in the same parameter list because there could be nested function call expressions, like f(x, g(x)). And as your example points out, the function call expression might be using infix operator notation instead of an explicit function call with a list, so probably the best thing to do is to suppress move-from-last-use when there is another use in the same statement.

I meant that the order of evaluation of these are specified:

Always safe: x ? std::move(x) : std::move(x).
Overloadable, suspicious:
Overloadable. Is a specified order of evaluation a good measure to move-from-last-use? Definitely suspicious! (And so is the reappearance of x itself).

Note that x isn't necessarily used directly. It could be f(y) and g(y) for the left and right hand sides, respectively.

I've noticed that
foo: () -> int = {
lambda := :()->_ = 42;
return lambda();
}

generates following cpp1:
[[nodiscard]] auto foo() -> int{
auto lambda {[]() -> auto { return 42; }};
return std::move(lambda)();
}

Should it be
return std::move( lambda() ); instead?
I suppose we want to move returned value from it, not lambda itself?

Should it be
return std::move( lambda() ); instead?
I suppose we want to move returned value from it, not lambda itself?

No. Check #313 (comment) again. If you want to move from a returned lvalue reference, that should be explicit.

Sorry, it still hard to wrap my head around.

If you want to move from a returned lvalue reference, that should be explicit.
I'm not sure how to express that in pure cpp2.

//foo: () -> (move result : int) = {
//error: a return value cannot be 'move' (it is implicitly 'move'-out)

foo: () -> int = {
lambda := :()->_ = 42;
//return move lambda(); //error
//return std::move(lambda()); //error: std::move is not needed in Cpp2 - use 'move' parameters/arguments instead
}

I have tried also with out parameter, but that compicates API at caller site (have to introduce variable), and still generates cpp1 where lambda moved but not result:
result.construct(std::move(lambda)());

Sorry if I'm overcomplicating or missing something.

When a function returns value it is implicitly return with move passing style. You can change it to forward in some cases.

I don't have my PC near me. Does below code works? I am not sure.

return move lambda();

Nope, that's error: missing ; after return (at 'lambda')

This works to move from a returned lvalue reference: https://godbolt.org/z/r1oPfY89j.

accumulator: type = {
  data: std::vector<int>              = (1, 2, 3);
  get_data: (inout this) -> forward _ = data;
}
main: () = {
  acc: accumulator = ();
  [[assert: acc.get_data().size() == 3]]
  consume_data := (move acc.get_data());
  [[assert: acc.get_data().size() == 0]]
}

Ah, yes. Parentheses are needed.

return (move lambda());

Right?

Oh right, don't know why I haven't tried that. It generates
return (std::move(std::move(lambda)()));

This is the last use of lambda so it will be moved.

Navigation Menu

Repository navigation

[BUG] Suspicious cpp1 std::move of returning expression #313

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

[BUG] Suspicious cpp1 std::move of returning expression #313

Description

Activity

JohelEGP commented on Apr 2, 2023

realgdman commented on Apr 2, 2023

JohelEGP commented on Apr 2, 2023

realgdman commented on Apr 2, 2023

JohelEGP commented on Apr 2, 2023

realgdman commented on Apr 2, 2023

hsutter commented on Apr 8, 2023

JohelEGP commented on Apr 8, 2023

hsutter commented on Apr 9, 2023

JohelEGP commented on Apr 10, 2023

realgdman commented on Apr 12, 2023

JohelEGP commented on Apr 12, 2023

realgdman commented on Apr 12, 2023

filipsajdak commented on Apr 12, 2023

realgdman commented on Apr 12, 2023

JohelEGP commented on Apr 12, 2023

filipsajdak commented on Apr 12, 2023

realgdman commented on Apr 12, 2023

filipsajdak commented on Apr 12, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions