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

Remove confusing 'while checking' note from opaque future type mismatches #107201

Merged
merged 1 commit into from
Feb 2, 2023

Conversation

compiler-errors
Copy link
Member

Maybe I'm just misinterpreting the wording of the note. The only value I can see in this note is that it points out where the async's opaque future is coming from, but the way it's doing it is misleading IMO.

For example:

note: while checking the return type of the `async fn`
  --> $DIR/dont-suggest-missing-await.rs:7:24
   |
LL | async fn make_u32() -> u32 {
   |                        ^^^ checked the `Output` of this `async fn`, found opaque type

We point at the type u32 in the HIR, but then say "found opaque type". We also say "while checking"... but we're typechecking a totally different function when we get this type mismatch!

r? @estebank but feel free to reassign and/or take your time reviewing this. I'd be inclined to also discuss reworking the presentation of this type mismatch to restore some of these labels in a way that makes it more clear what it's trying to point out.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 22, 2023
@rust-log-analyzer

This comment has been minimized.

@compiler-errors compiler-errors force-pushed the confusing-async-fn-note branch from c7be912 to a63f5dc Compare January 22, 2023 17:03
@estebank
Copy link
Contributor

I'm not entirely convinced that removing the span is an unambiguous improvement, but the current output isn't great either.

@compiler-errors
Copy link
Member Author

@estebank - I agree, but I think the current wording is bordering on actively confusing.

I'm open to suggestions for a wording, but getting it to fit into the existing machinery might be hard.

@estebank
Copy link
Contributor

I can't further review this now, but I think r=me.

@compiler-errors
Copy link
Member Author

@bors r=estebank rollup

@bors
Copy link
Contributor

bors commented Feb 1, 2023

📌 Commit a63f5dc has been approved by estebank

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 1, 2023

🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened.

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 1, 2023
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Feb 2, 2023
…-note, r=estebank

Remove confusing 'while checking' note from opaque future type mismatches

Maybe I'm just misinterpreting the wording of the note. The only value I can see in this note is that it points out where the async's opaque future is coming from, but the way it's doing it is misleading IMO.

For example:

```rust
note: while checking the return type of the `async fn`
  --> $DIR/dont-suggest-missing-await.rs:7:24
   |
LL | async fn make_u32() -> u32 {
   |                        ^^^ checked the `Output` of this `async fn`, found opaque type
```

We point at the type `u32` in the HIR, but then say "found opaque type". We also say "while checking"... but we're typechecking a totally different function when we get this type mismatch!

r? `@estebank` but feel free to reassign and/or take your time reviewing this. I'd be inclined to also discuss reworking the presentation of this type mismatch to restore some of these labels in a way that makes it more clear what it's trying to point out.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2023
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#107201 (Remove confusing 'while checking' note from opaque future type mismatches)
 - rust-lang#107312 (Add Style Guide rules for let-else statements)
 - rust-lang#107488 (Fix syntax in `-Zunpretty-expanded` output for derived `PartialEq`.)
 - rust-lang#107531 (Inline CSS background images directly into the CSS)
 - rust-lang#107576 (Add proc-macro boilerplate to crt-static test)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 480c4a1 into rust-lang:master Feb 2, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 2, 2023
@compiler-errors compiler-errors deleted the confusing-async-fn-note branch August 11, 2023 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants