Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Double move when passing by move #413

Open
JohelEGP opened this issue May 3, 2023 · 6 comments
Open

[BUG] Double move when passing by move #413

JohelEGP opened this issue May 3, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented May 3, 2023

Title: Double move when passing by move.

Minimal reproducer (https://cpp2.godbolt.org/z/31sPe9xMj):

main: () = {
  x := 0;
  :(y) = 0;
  (move x);
}

Commands:

cppfront -clean-cpp1 main.cpp2

Expected result:

  [](auto const& y) -> void { 0;  }
  (std::move(x));

Actual result and error:

  [](auto const& y) -> void { 0;  }
  (std::move(std::move(x)));
@JohelEGP JohelEGP added the bug Something isn't working label May 3, 2023
@HALL9kv0
Copy link

HALL9kv0 commented May 4, 2023

I fixed it locally, I don't know if it breaks something else in the process.
In cppfront.h, line under 1628:

line 1624          //  For an explicit 'forward' apply forwarding to correct identifier
                           assert (!current_args.empty());
                           if (current_args.back().pass == passing_style::forward) {
                                        add_forward = current_args.back().ptoken == n.identifier;
line 1628              }

I have added :

  // If explicit 'move' is used then don't add move
   if (current_args.back().pass == passing_style::move) {
            add_move = false;
    }  

Let me know if this works or not, I look forward to contributing more actively, but for the time being I'm familiarizing my self with the compiler code.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 4, 2023

Let me know if this works or not, I look forward to contributing more actively, but for the time being I'm familiarizing my self with the compiler code.

I can't know, as I don't have working code.
Just lots of snippets compliant to the grammar.

@HALL9kv0
Copy link

HALL9kv0 commented May 4, 2023

My apologies, I was referring to the contributors.

@JohelEGP
Copy link
Contributor Author

Now I can confirm that it works,
and there's just one change to existing tests:

-    v = X<19>(); std::cout << "move(v) as X<19> = " + cpp2::to_string(int(cpp2::as_<X<19>>((std::move(std::move(v)))))) << std::endl;
+    v = X<19>(); std::cout << "move(v) as X<19> = " + cpp2::to_string(int(cpp2::as_<X<19>>((std::move(v))))) << std::endl;

@JohelEGP
Copy link
Contributor Author

An alternative Expected result
is a diagnostic saying that x will be moved,
and thus the explicit move is redundant.

@JohelEGP
Copy link
Contributor Author

An alternative Expected result
is a diagnostic saying that x will be moved,
and thus the explicit move is redundant.

This isn't true in a generic context.
So I expect move x to always work in a generic context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants