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 blanket silencing of "type annotation needed" errors #64746

Merged
merged 3 commits into from Sep 25, 2019

Conversation

@estebank
Copy link
Contributor

commented Sep 24, 2019

Remove blanket check for existence of other errors before emitting "type annotation needed" errors, and add some eager checks to avoid adding obligations when they refer to types that reference [type error] in order to reduce unneeded errors.

Fix #64084.

estebank added 2 commits Sep 24, 2019
Remove blanket check for existence of other errors before emitting
"type annotation needed" errors, and add some eager checks to avoid
adding obligations when they refer to types that reference
`[type error]` in order to reduce unneded errors.
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Sep 24, 2019

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -1155,6 +1157,15 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
);
debug!("instantiate_opaque_types: ty_var={:?}", ty_var);

for predicate in &bounds.predicates {

This comment has been minimized.

Copy link
@Centril

Centril Sep 24, 2019

Member

We could make a method returning -> bool here and use if stuff.obligation_references_error() { return ty_var; } here.

This comment has been minimized.

Copy link
@estebank

estebank Sep 24, 2019

Author Contributor

I didn't want to hide the O(n) behavior of the added code so that I could maybe nerd-snipe someone to point me a place where we could do the same with no added time ^_^

This comment has been minimized.

Copy link
@Centril

Centril Sep 24, 2019

Member

Can't you just leave a doc comment about the O(n) complexity? This is in diagnostics code anyways so what does O(n) matter?

This comment has been minimized.

Copy link
@estebank

estebank Sep 24, 2019

Author Contributor

Because it it needs to go over the predicates vec twice instead of once for every case. This is to add the obligations, which may or may not be met.

@cramertj

This comment has been minimized.

Copy link
Member

commented Sep 24, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2019

📌 Commit b7ca1c5 has been approved by cramertj

Centril added a commit to Centril/rust that referenced this pull request Sep 25, 2019
…ns-on-err, r=cramertj

Remove blanket silencing of "type annotation needed" errors

Remove blanket check for existence of other errors before emitting "type annotation needed" errors, and add some eager checks to avoid adding obligations when they refer to types that reference `[type error]` in order to reduce unneeded errors.

Fix rust-lang#64084.
bors added a commit that referenced this pull request Sep 25, 2019
Rollup of 7 pull requests

Successful merges:

 - #64324 (rustc: Fix mixing crates with different `share_generics`)
 - #64428 (Error explanation e0524)
 - #64481 (A more explanatory thread local storage panic message)
 - #64599 (Rustdoc render async function re-export)
 - #64743 (Update cargo)
 - #64746 (Remove blanket silencing of "type annotation needed" errors)
 - #64753 (Don't emit explain with json short messages.)

Failed merges:

r? @ghost
@bors bors merged commit b7ca1c5 into rust-lang:master Sep 25, 2019
4 checks passed
4 checks passed
pr Build #20190924.40 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
phansch added a commit to phansch/rust-clippy that referenced this pull request Sep 25, 2019
bors added a commit to rust-lang/rust-clippy that referenced this pull request Sep 25, 2019
Rustup to rust-lang/rust#64746

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.