Make it more clear what an about async fn's returns when referring to what it returns by guswynn · Pull Request #76765 · rust-lang/rust

guswynn · 2020-09-15T22:01:36Z

This is likely not the ONLY place that this happens to be unclear, but we can move this fn to rustc_middle or something like that and reuse it if need be, to apply it to more diagnostics

One outstanding question I have is, if the fn returns (), should I make the message more clear (what about fn f() vs fn f() -> (), can you tell those apart in the hir?)

R? @tmandry

@rustbot modify labels +A-diagnostics +T-compiler

guswynn · 2020-09-15T22:31:25Z

ci failed but I want to make sure Im on the right path first

estebank · 2020-09-16T22:27:15Z

You'll need to handle this case when including the return type snippet to be impl Future<Output = ()> for this case.

tmandry · 2020-09-16T22:56:39Z

Hmm, this wording doesn't really work if the error is in the function signature itself. I'm confused why the span is pointing here, but one way to make the diagnostic less wrong is to remove the word "here". Then we're just telling you what's happening and providing a span that might help.

sounds good, when I find time (hopefully tomorrow) I will dig into these comments!

camelid · 2020-10-16T02:32:24Z

@guswynn Could you address the review comments?

guswynn · 2020-10-17T19:26:28Z

I'll try to get to it this weekend!

…

On Thu, Oct 15, 2020, 7:32 PM Camelid ***@***.***> wrote: @guswynn <https://github.com/guswynn> Could you address the review comments? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#76765 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJHNDZL2TVYWNFLEGXO4NLSK6WEPANCNFSM4RN2XERA> .

guswynn · 2020-10-17T22:26:37Z

Updated to the comments.
I have 2 questions

@tmandry mentioned in Make it more clear what an about async fn's returns when referring to what it returns #76765 (comment) that the message this works off may be unclear in some cases, should that a different issue?
wording-wise, is it ok in rust-lang to prepend a span_label msg with "note: "?

JohnCSimon · 2020-11-03T05:16:22Z

ping from triage - @guswynn can you mark the 'change requested' comments from camelid as addressed? is this PR ready for review?

guswynn · 2020-11-03T05:29:15Z

@JohnCSimon I believe I marked the comment as resolved, I am.not sure how to clear the request changes.
The pr is ready to review!

tmandry · 2020-11-05T00:03:34Z

 })
 }

+ pub(super) fn future_return_type(


tmandry · 2020-11-05T00:04:56Z

Sorry, this fell off the radar. I think it is fine to merge. Thanks!!

@bors r+ rollup

bors · 2020-11-05T00:04:57Z

📌 Commit 20e032e has been approved by tmandry

JohnTitor · 2020-11-05T05:54:13Z

Failed in https://github.com/rust-lang-ci/rust/runs/1356499780:

2020-11-05T05:10:40.5946149Z stderr:
2020-11-05T05:10:40.5946743Z ------------------------------------------
2020-11-05T05:10:40.5947263Z error: lifetime may not live long enough
2020-11-05T05:10:40.5948197Z --> /checkout/src/test/ui/issues/issue-76547.rs:19:14
2020-11-05T05:10:40.5948757Z |
2020-11-05T05:10:40.5949197Z LL | async fn fut(bufs: &mut [&mut [u8]]) {
2020-11-05T05:10:40.5950093Z | ^^^^ - - let's call the lifetime of this reference `'2`
2020-11-05T05:10:40.5950635Z | | |
2020-11-05T05:10:40.5951435Z | | let's call the lifetime of this reference `'1`
2020-11-05T05:10:40.5952592Z | assignment requires that `'1` must outlive `'2`
2020-11-05T05:10:40.5953032Z 2020-11-05T05:10:40.5953532Z error: lifetime may not live long enough
2020-11-05T05:10:40.5954477Z --> /checkout/src/test/ui/issues/issue-76547.rs:33:15
2020-11-05T05:10:40.5955011Z |
2020-11-05T05:10:40.5955683Z LL | async fn fut2(bufs: &mut [&mut [u8]]) -> i32 {
2020-11-05T05:10:40.5956624Z | ^^^^ - - let's call the lifetime of this reference `'2`
2020-11-05T05:10:40.5957169Z | | |
2020-11-05T05:10:40.5958215Z thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:354:22
2020-11-05T05:10:40.5959270Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2020-11-05T05:10:40.5960410Z | | let's call the lifetime of this reference `'1`
2020-11-05T05:10:40.5961377Z | assignment requires that `'1` must outlive `'2`
2020-11-05T05:10:40.5961809Z 2020-11-05T05:10:40.5962313Z error: aborting due to 2 previous errors
2020-11-05T05:10:40.5963488Z ------------------------------------------
2020-11-05T05:10:40.5964585Z failures:
2020-11-05T05:10:40.5965298Z [ui (nll)] ui/issues/issue-76547.rs
2020-11-05T05:10:40.5965686Z 2020-11-05T05:10:40.5966628Z test result: �[31mFAILED�(B�[m. 10941 passed; 1 failed; 106 ignored; 0 measured; 0 filtered out

@bors r-

camelid · 2020-11-05T07:28:53Z

Here's the diff:

-	error[E0623]: lifetime mismatch- --> $DIR/issue-76547.rs:20:13+	error: lifetime may not live long enough+ --> $DIR/issue-76547.rs:19:14
|
LL | async fn fut(bufs: &mut [&mut [u8]]) {
- | --------- -- | | |- | | this `async fn` implicitly returns an `impl Future<Output = ()>`- | this parameter and the returned future are declared with different lifetimes...-	LL | ListFut(bufs).await- | ^^^^ ...but data from `bufs` is held across an await point here+ | ^^^^ - - let's call the lifetime of this reference `'2`+ | | |+ | | let's call the lifetime of this reference `'1`+ | assignment requires that `'1` must outlive `'2`-	error[E0623]: lifetime mismatch- --> $DIR/issue-76547.rs:34:14+	error: lifetime may not live long enough+ --> $DIR/issue-76547.rs:33:15
|
LL | async fn fut2(bufs: &mut [&mut [u8]]) -> i32 {
- | --------- ---- | | |- | | this `async fn` implicitly returns an `impl Future<Output = i32>`- | this parameter and the returned future are declared with different lifetimes...-	LL | ListFut2(bufs).await- | ^^^^ ...but data from `bufs` is held across an await point here+ | ^^^^ - - let's call the lifetime of this reference `'2`+ | | |+ | | let's call the lifetime of this reference `'1`+ | assignment requires that `'1` must outlive `'2`
error: aborting due to 2 previous errors
-	For more information about this error, try `rustc --explain E0623`.

guswynn · 2020-11-05T15:03:17Z

I'll try to fix the rebase in the next few days when I find time

guswynn · 2020-11-05T15:04:48Z

Here's the diff:

-	error[E0623]: lifetime mismatch- --> $DIR/issue-76547.rs:20:13+	error: lifetime may not live long enough+ --> $DIR/issue-76547.rs:19:14
|
LL | async fn fut(bufs: &mut [&mut [u8]]) {
- | --------- -- | | |- | | this `async fn` implicitly returns an `impl Future<Output = ()>`- | this parameter and the returned future are declared with different lifetimes...-	LL | ListFut(bufs).await- | ^^^^ ...but data from `bufs` is held across an await point here+ | ^^^^ - - let's call the lifetime of this reference `'2`+ | | |+ | | let's call the lifetime of this reference `'1`+ | assignment requires that `'1` must outlive `'2`-	error[E0623]: lifetime mismatch- --> $DIR/issue-76547.rs:34:14+	error: lifetime may not live long enough+ --> $DIR/issue-76547.rs:33:15
|
LL | async fn fut2(bufs: &mut [&mut [u8]]) -> i32 {
- | --------- ---- | | |- | | this `async fn` implicitly returns an `impl Future<Output = i32>`- | this parameter and the returned future are declared with different lifetimes...-	LL | ListFut2(bufs).await- | ^^^^ ...but data from `bufs` is held across an await point here+ | ^^^^ - - let's call the lifetime of this reference `'2`+ | | |+ | | let's call the lifetime of this reference `'1`+ | assignment requires that `'1` must outlive `'2`
error: aborting due to 2 previous errors
-	For more information about this error, try `rustc --explain E0623`.

Looks like the test I ADDED no longer triggers the codepath, so this may be more involved to figure out

JohnTitor · 2020-11-05T16:35:48Z

@guswynn We have a builder that enables nll compare mode so you should tweak the test like https://github.com/rust-lang/rust/pull/78268/files#diff-4e99585b86f1234b2e3f288f2e63e8dde1f23834ba26bc79e69d2aace14fd238.

estebank · 2020-11-06T02:40:57Z

./x.py test src/test/ui --stage 1 --keep-stage 1 --bless --compare-mode=nll

guswynn · 2020-11-08T22:55:03Z

@estebank @JohnTitor are you saying that this has rebased onto something turning on a new nll mode? and in the test I should just turn it off?

estebank · 2020-11-09T18:02:39Z

We are saying that in CI tests check for nll enabled output, but locally by default it doesn't.

Running ./x.py test src/test/ui --stage 1 --keep-stage 1 --bless --compare-mode=nll will bless the nll output files and make it pass CI.

guswynn · 2020-11-10T00:30:40Z

@estebank thats a fantastic command! Added the change! should I make a follow up issue to fix the message for compare-mode=nll? (I am not sure why its different, nll has been stable for ages?)

tmandry · 2020-11-10T01:43:58Z

@estebank thats a fantastic command! Added the change! should I make a follow up issue to fix the message for compare-mode=nll? (I am not sure why its different, nll has been stable for ages?)

Hm yeah, not sure either, it can't hurt to file an issue.

Co-authored-by: Camelid <camelidcamel@gmail.com>

tmandry · 2020-11-10T01:44:13Z

@bors r+ rollup

bors · 2020-11-10T01:44:14Z

📌 Commit 6be4847 has been approved by tmandry

bors · 2020-11-10T01:44:15Z

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

…as-schievink Rollup of 14 pull requests Successful merges: - rust-lang#76765 (Make it more clear what an about async fn's returns when referring to what it returns) - rust-lang#78574 (Use check-pass instead of build-pass in regions ui test suite) - rust-lang#78669 (Use check-pass instead of build-pass in some consts ui test suits) - rust-lang#78847 (Assert that a return place is not used for indexing during integration) - rust-lang#78854 (Workaround for "could not fully normalize" ICE ) - rust-lang#78875 (rustc_target: Further cleanup use of target options) - rust-lang#78887 (Add comments to explain memory usage optimization) - rust-lang#78890 (comment attribution fix) - rust-lang#78896 (Clarified description of write! macro) - rust-lang#78897 (Add missing newline to error message of the default OOM hook) - rust-lang#78898 (add regression test for rust-lang#78892) - rust-lang#78908 ((rustdoc) [src] link for types defined by macros shows invocation, not defintion) - rust-lang#78910 (Fix links to stabilized versions of some intrinsics) - rust-lang#78912 (Add macro test for min-const-generics) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned tmandry Sep 15, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 15, 2020

guswynn force-pushed the async_return branch from 8190bd7 to 9f2f83bCompare September 15, 2020 22:02

rustbot added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 15, 2020

jyn514 added the A-async-await Area: Async & Await label Sep 15, 2020

tmandry reviewed Sep 15, 2020
View reviewed changes

Comment threadsrc/test/ui/issues/issue-76547.stderr Outdated

guswynn force-pushed the async_return branch from d54aad9 to 10cc864Compare September 16, 2020 17:26

estebank reviewed Sep 16, 2020
View reviewed changes

Comment threadcompiler/rustc_infer/src/infer/error_reporting/nice_region_error/different_lifetimes.rs Outdated

estebank reviewed Sep 16, 2020
View reviewed changes

Comment threadsrc/test/ui/self/elision/ref-self-async.stderr Outdated

estebank reviewed Sep 16, 2020
View reviewed changes

tmandry reviewed Sep 16, 2020
View reviewed changes

camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2020

guswynn force-pushed the async_return branch 2 times, most recently from 31a944d to 3d66d3bCompare October 17, 2020 22:24

camelid requested changes Oct 17, 2020
View reviewed changes

Comment thread...iler/rustc_infer/src/infer/error_reporting/nice_region_error/.different_lifetimes.rs.rustfmt Outdated

Make it more clear when complaining about async fn's return types
20e032e

guswynn force-pushed the async_return branch from 3d66d3b to 20e032eCompare October 17, 2020 22:58

camelid 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 (such as code changes or more information) from the author. labels Nov 3, 2020

tmandry reviewed Nov 5, 2020
View reviewed changes

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 5, 2020

add nll compare mode stderr file
aaf06d8

camelid reviewed Nov 10, 2020
View reviewed changes

Comment threadsrc/test/ui/issues/issue-76547.rs Outdated

Update src/test/ui/issues/issue-76547.rs
6be4847
Co-authored-by: Camelid <camelidcamel@gmail.com>

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 10, 2020

guswynn mentioned this pull request Nov 10, 2020
What does compare-mode=nll do these days? #78915
Closed

jonas-schievink mentioned this pull request Nov 10, 2020
Rollup of 14 pull requests #78920
Merged

bors merged commit 9596e34 into rust-lang:masterNov 11, 2020

rustbot added this to the 1.49.0 milestone Nov 11, 2020

guswynn mentioned this pull request Dec 17, 2020
Improve lifetime name annotations for closures & async functions #76468
Merged

guswynn deleted the async_return branch February 19, 2021 21:34

sjakobi mentioned this pull request Mar 26, 2021
async fn can't handle multiple lifetimes in a slice of slices #76547
Closed

Uh oh!

Uh oh!

Conversation

guswynn commented Sep 15, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guswynn commented Sep 15, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

estebankSep 16, 2020

Choose a reason for hiding this comment

Uh oh!

tmandrySep 16, 2020

Choose a reason for hiding this comment

Uh oh!

guswynnSep 16, 2020

Choose a reason for hiding this comment

Uh oh!

camelid commented Oct 16, 2020

Uh oh!

guswynn commented Oct 17, 2020 via email

Uh oh!

guswynn commented Oct 17, 2020

Uh oh!

Uh oh!

JohnCSimon commented Nov 3, 2020

Uh oh!

guswynn commented Nov 3, 2020

Uh oh!

tmandryNov 5, 2020

Choose a reason for hiding this comment

Uh oh!

tmandry commented Nov 5, 2020

Uh oh!

bors commented Nov 5, 2020

Uh oh!

JohnTitor commented Nov 5, 2020

Uh oh!

camelid commented Nov 5, 2020

Uh oh!

guswynn commented Nov 5, 2020

Uh oh!

guswynn commented Nov 5, 2020

Uh oh!

JohnTitor commented Nov 5, 2020

Uh oh!

estebank commented Nov 6, 2020

Uh oh!

guswynn commented Nov 8, 2020

Uh oh!

estebank commented Nov 9, 2020

Uh oh!

guswynn commented Nov 10, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tmandry commented Nov 10, 2020

Uh oh!

tmandry commented Nov 10, 2020

Uh oh!

bors commented Nov 10, 2020

Uh oh!

bors commented Nov 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

guswynn commented Sep 15, 2020•
edited
Loading

guswynn commented Nov 10, 2020•
edited
Loading