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.

Sign up for GitHub

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

Jump to bottom

Don't suggest types whose inner type is erroneous #91450

Merged
merged 1 commit into from
Dec 5, 2021
Merged

Don't suggest types whose inner type is erroneous #91450

merged 1 commit into from
Dec 5, 2021

Conversation

hkmatsumoto
Copy link
Member

@hkmatsumoto hkmatsumoto commented Dec 2, 2021 •
edited
Loading

Currently, we check if the returned type equals to tcx.ty_error() not to emit
erroneous types, but this has a pitfall; for example,
Option<[type error]> != tcx.ty_error() holds.

Fixes #91371.

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 2, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Dec 2, 2021

☔ The latest upstream changes (presumably #91354) made this pull request unmergeable. Please resolve the merge conflicts.

@Aaron1011
Copy link
Member

Could you add the code from the issue as a test case?

@estebank
Copy link
Contributor

estebank commented Dec 2, 2021

r=me with a test and resolved merge conflict (please squash your changes to a single commit on top of recent master).

hkmatsumoto
hkmatsumoto commented Dec 3, 2021
View reviewed changes
src/test/ui/typeck/issue-91450-inner-ty-error.rs
// This test ensures that the compiler does not suggest `Foo<[type error]>` in diagnostic messages.

fn foo() -> Option<_> {} //~ ERROR: [E0308]
//~^ ERROR: the type placeholder `_` is not allowed
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to add a "negative" test of "help: replace with the correct return type: Foo<[type error]>" here to detect any regressions in the future but IIRC there isn't such annotation ;)

@hkmatsumoto
Copy link
Member Author

I don't think I have the permission to do it but let me try
@bors r=estebank

@bors
Copy link
Contributor

bors commented Dec 3, 2021

@hkmatsumoto: 🔑 Insufficient privileges: Not in reviewers

@hkmatsumoto
Copy link
Member Author

hkmatsumoto commented Dec 3, 2021 •
edited
Loading

Fair enough, @estebank could you take another look?

@estebank
Copy link
Contributor

estebank commented Dec 3, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Dec 3, 2021

📌 Commit f8b8a5c96ad2fc7231d5da8e98d2eb130460f9c3 has been approved by estebank

@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 Dec 3, 2021
@bors
Copy link
Contributor

bors commented Dec 3, 2021

☔ The latest upstream changes (presumably #91491) made this pull request unmergeable. Please resolve the merge conflicts.

@bors 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 Dec 3, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@hkmatsumoto
Don't suggest types whose inner type is erroneous
9b77a1e 
Currently, we check if the returned type equals to `tcx.ty_error()` not to emit
erroneous types, but this has a pitfall; for example,
`Option<[type error]> != tcx.ty_error()` holds.
@jackh726
Copy link
Member

jackh726 commented Dec 5, 2021

@bors r=estebank

@bors
Copy link
Contributor

bors commented Dec 5, 2021

📌 Commit 9b77a1e has been approved by estebank

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 5, 2021
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 5, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 5, 2021
@bors
Auto merge of rust-lang#91555 - matthiaskrgr:rollup-pq0iaq7, r=matthi…
772d51f 
…askrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#90529 (Skip reborrows in AbstractConstBuilder)
 - rust-lang#91437 (Pretty print empty blocks as {})
 - rust-lang#91450 (Don't suggest types whose inner type is erroneous)
 - rust-lang#91535 (Stabilize `-Z emit-future-incompat` as `--json future-incompat`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 609d9a0 into rust-lang:master Dec 5, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 5, 2021
@bors
Copy link
Contributor

bors commented Dec 5, 2021

⌛ Testing commit 9b77a1e with merge 772d51f...

@hkmatsumoto hkmatsumoto deleted the hide-type-error branch December 6, 2021 06:31
@hellow554 hellow554 mentioned this pull request Apr 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@estebank estebank

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.

Compiler suggests [type error] when encountering placeholder in fn return type
8 participants
@hkmatsumoto @rust-highfive @rust-log-analyzer @bors @Aaron1011 @estebank @jackh726 @rustbot