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

Ignore generated fresh lifetimes in elision check #4266

Merged
merged 6 commits into from Jul 23, 2019

Conversation

oxalica
Copy link
Contributor

@oxalica oxalica commented Jul 9, 2019

fixes #3988

changelog: Ignore generated fresh lifetimes in elision check.

HELP: It seems tests/ui are compiled under edition 2015, and I don't know how to add tests for this properly.

Here is the test input it had already passed:

#![feature(async_await)]
#![allow(dead_code)]

async fn sink1<'a>(_: &'a str) {} // lint
async fn sink1_elided(_: &str) {} // ok

async fn one_to_one<'a>(s: &'a str) -> &'a str { s } // lint
async fn one_to_one_elided(s: &str) -> &str { s } // ok
async fn all_to_one<'a>(a: &'a str, _b: &'a str) -> &'a str { a } // ok
// async fn unrelated(_: &str, _: &str) {} // Not allowed in async fn

// #3988
struct Foo;
impl Foo {
    pub async fn foo(&mut self) {} // ok
}

// rust-lang/rust#61115
async fn print(s: &str) { // ok
    println!("{}", s);
}

fn main() {}

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 10, 2019
@phansch
Copy link
Member

phansch commented Jul 15, 2019

HELP: It seems tests/ui are compiled under edition 2015, and I don't know how to add tests for this properly.

I think you can add the following to the top of the UI test file to use the 2018 edition:

// compile-flags: --edition 2018

@phansch phansch added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jul 17, 2019
@oxalica
Copy link
Contributor Author

oxalica commented Jul 18, 2019

I think you can add the following to the top of the UI test file to use the 2018 edition:

// compile-flags: --edition 2018

Thanks, it works. Tests are now added.

@flip1995
Copy link
Member

Hmm, now util/dev fmt can't deal with the 2018 edition 🤔

@flip1995 flip1995 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jul 19, 2019
@flip1995
Copy link
Member

Adding edition = "2018" here


should solve this.

@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jul 19, 2019
@oxalica
Copy link
Contributor Author

oxalica commented Jul 19, 2019

Adding edition = "2018" here

should solve this.

Setting global fmt edition will cause some tests failed.
Is there any way to skip fmt on one file? #[rustfmt::skip] not works here.

@flip1995
Copy link
Member

I think using Edition = 2018 and fixing the falling test would be the best thing to do here.

@flip1995
Copy link
Member

Thanks! You fixed the first T-async-await issue 🎉

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 23, 2019

📌 Commit 5265ab8 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Jul 23, 2019

⌛ Testing commit 5265ab8 with merge d71e9c4...

bors added a commit that referenced this pull request Jul 23, 2019
Ignore generated fresh lifetimes in elision check

<!--
Thank you for making Clippy better!

We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.

If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `util/dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo fmt`

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

Delete this line and everything above before opening your PR -->

fixes #3988

changelog: Ignore generated fresh lifetimes in elision check.

**HELP**: It seems `tests/ui` are compiled under edition 2015, and I don't know how to add tests for this properly.

Here is the test input it had already passed:
```rust
#![feature(async_await)]
#![allow(dead_code)]

async fn sink1<'a>(_: &'a str) {} // lint
async fn sink1_elided(_: &str) {} // ok

async fn one_to_one<'a>(s: &'a str) -> &'a str { s } // lint
async fn one_to_one_elided(s: &str) -> &str { s } // ok
async fn all_to_one<'a>(a: &'a str, _b: &'a str) -> &'a str { a } // ok
// async fn unrelated(_: &str, _: &str) {} // Not allowed in async fn

// #3988
struct Foo;
impl Foo {
    pub async fn foo(&mut self) {} // ok
}

// rust-lang/rust#61115
async fn print(s: &str) { // ok
    println!("{}", s);
}

fn main() {}

```
@bors
Copy link
Collaborator

bors commented Jul 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing d71e9c4 to master...

@bors bors merged commit 5265ab8 into rust-lang:master Jul 23, 2019
@oxalica oxalica deleted the fix/async_fn_lifetime branch July 23, 2019 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

needless_lifetimes: false positive with async fn
4 participants