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

Ensure that drop order of `async fn` matches `fn` and that users cannot refer to generated arguments. #60437

Merged
merged 3 commits into from May 2, 2019

Conversation

Projects
None yet
4 participants
@davidtwco
Copy link
Member

commented May 1, 2019

Fixes #60236 and fixes #60438.

This PR modifies the lowering of async fn arguments so that the
drop order matches the equivalent fn.

Previously, async function arguments were lowered as shown below:

async fn foo(<pattern>: <ty>) {
  async move {
  }
} // <-- dropped as you "exit" the fn

// ...becomes...
fn foo(__arg0: <ty>) {
  async move {
    let <pattern> = __arg0;
  } // <-- dropped as you "exit" the async block
}

After this PR, async function arguments will be lowered as:

async fn foo(<pattern>: <ty>, <pattern>: <ty>, <pattern>: <ty>) {
  async move {
  }
} // <-- dropped as you "exit" the fn

// ...becomes...
fn foo(__arg0: <ty>, __arg1: <ty>, __arg2: <ty>) {
  async move {
    let __arg2 = __arg2;
    let <pattern> = __arg2;
    let __arg1 = __arg1;
    let <pattern> = __arg1;
    let __arg0 = __arg0;
    let <pattern> = __arg0;
  } // <-- dropped as you "exit" the async block
}

If <pattern> is a simple ident, then it is lowered to a single
let <pattern> = <pattern>; statement as an optimization.

This PR also stops users from referring to the generated __argN
identifiers.

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

left a comment

r=me with a "master comment" detailing the strategy and a renamed test file =)

Show resolved Hide resolved src/test/run-pass/issue-54716.rs Outdated
Show resolved Hide resolved src/librustc/hir/lowering.rs
Ensure that drop order of `async fn` matches `fn`.
This commit modifies the lowering of `async fn` arguments so that the
drop order matches the equivalent `fn`.

Previously, async function arguments were lowered as shown below:

    async fn foo(<pattern>: <ty>) {
      async move {
      }
    } // <-- dropped as you "exit" the fn

    // ...becomes...
    fn foo(__arg0: <ty>) {
      async move {
        let <pattern> = __arg0;
      } // <-- dropped as you "exit" the async block
    }

After this PR, async function arguments will be lowered as:

    async fn foo(<pattern>: <ty>, <pattern>: <ty>, <pattern>: <ty>) {
      async move {
      }
    } // <-- dropped as you "exit" the fn

    // ...becomes...
    fn foo(__arg0: <ty>, __arg1: <ty>, __arg2: <ty>) {
      async move {
        let __arg2 = __arg2;
        let <pattern> = __arg2;
        let __arg1 = __arg1;
        let <pattern> = __arg1;
        let __arg0 = __arg0;
        let <pattern> = __arg0;
      } // <-- dropped as you "exit" the async block
    }

If `<pattern>` is a simple ident, then it is lowered to a single
`let <pattern> = <pattern>;` statement as an optimization.

@davidtwco davidtwco force-pushed the davidtwco:issue-60236 branch from 20e89f6 to 0ab625e May 1, 2019

@davidtwco davidtwco changed the title Ensure that drop order of `async fn` matches `fn`. Ensure that drop order of `async fn` matches `fn` and that users cannot refer to generated arguments. May 1, 2019

@nikomatsakis
Copy link
Contributor

left a comment

Actually can we simplify the new test?

davidtwco added some commits May 1, 2019

Ensure that users cannot use generated arguments.
This commit gensyms the generated ident for replacement arguments so
that users cannot refer to them. It also ensures that levenshtein
distance suggestions do not suggest gensymed identifiers.

@davidtwco davidtwco force-pushed the davidtwco:issue-60236 branch from 0ab625e to 1fedb0a May 1, 2019

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

📌 Commit 1fedb0a has been approved by nikomatsakis

Centril added a commit to Centril/rust that referenced this pull request May 1, 2019

Rollup merge of rust-lang#60437 - davidtwco:issue-60236, r=nikomatsakis
Ensure that drop order of `async fn` matches `fn` and that users cannot refer to generated arguments.

Fixes rust-lang#60236 and fixes rust-lang#60438.

This PR modifies the lowering of `async fn` arguments so that the
drop order matches the equivalent `fn`.

Previously, async function arguments were lowered as shown below:

    async fn foo(<pattern>: <ty>) {
      async move {
      }
    } // <-- dropped as you "exit" the fn

    // ...becomes...
    fn foo(__arg0: <ty>) {
      async move {
        let <pattern> = __arg0;
      } // <-- dropped as you "exit" the async block
    }

After this PR, async function arguments will be lowered as:

    async fn foo(<pattern>: <ty>, <pattern>: <ty>, <pattern>: <ty>) {
      async move {
      }
    } // <-- dropped as you "exit" the fn

    // ...becomes...
    fn foo(__arg0: <ty>, __arg1: <ty>, __arg2: <ty>) {
      async move {
        let __arg2 = __arg2;
        let <pattern> = __arg2;
        let __arg1 = __arg1;
        let <pattern> = __arg1;
        let __arg0 = __arg0;
        let <pattern> = __arg0;
      } // <-- dropped as you "exit" the async block
    }

If `<pattern>` is a simple ident, then it is lowered to a single
`let <pattern> = <pattern>;` statement as an optimization.

This PR also stops users from referring to the generated `__argN`
identifiers.

r? @nikomatsakis

@Centril Centril referenced this pull request May 1, 2019

Merged

Rollup of 7 pull requests #60460

bors added a commit that referenced this pull request May 2, 2019

Auto merge of #60460 - Centril:rollup-gz5bc8i, r=Centril
Rollup of 7 pull requests

Successful merges:

 - #59634 (Added an explanation for the E0704 error.)
 - #60348 (move some functions from parser.rs to diagostics.rs)
 - #60385 (Emit metadata files earlier)
 - #60428 (Refactor `eval_body_using_ecx` so that it doesn't need to query for MIR)
 - #60437 (Ensure that drop order of `async fn` matches `fn` and that users cannot refer to generated arguments.)
 - #60439 (doc: Warn about possible zombie apocalypse)
 - #60452 (Remove Context and ContextKind)

Failed merges:

r? @ghost

@bors bors merged commit 1fedb0a into rust-lang:master May 2, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@davidtwco davidtwco deleted the davidtwco:issue-60236 branch May 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.