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

Extend async fn test suite to account for ref mut? patterns #60509

Closed
Centril opened this issue May 3, 2019 · 4 comments · Fixed by #60535
Closed

Extend async fn test suite to account for ref mut? patterns #60509

Centril opened this issue May 3, 2019 · 4 comments · Fixed by #60535
Labels
A-async-await Area: Async & Await A-testsuite Area: The testsuite used to check the correctness of rustc AsyncAwait-Polish Async-await issues that are part of the "polish" area E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented May 3, 2019

In #60501 the handling of mut patterns in arguments is fixed (#60498).

However, the test suite should also be extended to account for:

type A = Vec<u32>;

async fn foo(ref mut vec: A) { ... }

and:

async fn foo(ref vec: A) { ... }

as well as nestings of those:

async fn foo((ref mut c, ref d): (A, A)) { ... }

async fn foo((a, mut b): (A, A)) { ... }

Please be creative in testing for combinatorial interactions when closing this issue :)

@Centril Centril added A-testsuite Area: The testsuite used to check the correctness of rustc E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area labels May 3, 2019
@matthewjasper
Copy link
Contributor

type A = Vec<u32>;

async fn foo(ref mut vec: A) { ... }

This appears to be broken on nightly, desugaring into:

fn foo(ref mut vec: A) {
    static move || {
        let vec = vec; // let ref mut vec = vec; once #60501 is merged, AFAICT
    }   
}

Which the borrow checker complains about, because the generator is now effective capturing by-ref.

The correct lowering is:

fn foo(__arg1: A) {
    static move || {
        let __arg1 = __arg1; 
        let ref mut vec = __arg1;
    }   
}

@taiki-e
Copy link
Member

taiki-e commented May 4, 2019

This appears to be broken too:

async fn foo((ref mut c, ref d): (A, A)) { ... }

Error:

error[E0596]: cannot borrow `__arg0.0` as mutable, as `__arg0` is not declared as mutable
  --> code\src\lib.rs:17:15
   |
17 | async fn foo((ref mut c, ref d): (A, A)) {}
   |              -^^^^^^^^^--------
   |              ||
   |              |cannot borrow as mutable
   |              help: consider changing this to be mutable: `mut __arg0`

@taiki-e
Copy link
Member

taiki-e commented May 4, 2019

In addition, the following code gets the wrong warning because I forgot to remove mut from inputs in #60501 (I overlooked the warning being ignored in the test).

async fn bar(n: u32, mut vec: Vec<u32>) {
    vec.push(n)
}

Warning:

warning: variable does not need to be mutable
 --> code\src\lib.rs:8:22
  |
8 | async fn bar(n: u32, mut vec: Vec<u32>) {
  |                      ----^^^
  |                      |
  |                      help: remove this `mut`
  |
  = note: #[warn(unused_mut)] on by default

@taiki-e
Copy link
Member

taiki-e commented May 4, 2019

Opened #60535 to close this issue and to fix all the errors/warnings reported here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-testsuite Area: The testsuite used to check the correctness of rustc AsyncAwait-Polish Async-await issues that are part of the "polish" area E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants