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

Continue to borrowck even if there were previous errors #120550

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Feb 1, 2024

but only from the perspective of the whole compiler. Individual items should not get borrowcked if their MIR is tainted by errors.

r? @estebank @nnethercote

@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 Feb 1, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 1, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Comment on lines -965 to +975
self.cont_ln
.get(&sc)
.cloned()
.unwrap_or_else(|| span_bug!(expr.span, "continue to unknown label"))
self.cont_ln.get(&sc).cloned().unwrap_or_else(|| {
self.ir.tcx.dcx().span_delayed_bug(expr.span, "continue to unknown label");
self.ir.add_live_node(ErrNode)
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't (yet) know that we should not be doing liveness checks, because the error happens in check_mod_loops for which we have no path to forward a taint to here ("here" being check_liveness)

Copy link
Member

Choose a reason for hiding this comment

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

fwiw this (at least used to?) fire here I believe: #113379

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh neat. More evidence that those ICEs I've caused are preexisting, but hard to trigger

Comment on lines +658 to +668
let Some(drops) = self.scopes.breakable_scopes[break_index].continue_drops.as_mut()
else {
self.tcx.dcx().span_delayed_bug(
source_info.span,
"unlabelled `continue` within labelled block",
);
self.cfg.terminate(block, source_info, TerminatorKind::Unreachable);

return self.cfg.start_new_block().unit();
};
drops
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also part of the continue without target issue

Comment on lines +48 to +54
error: lifetime may not live long enough
--> $DIR/underscore-lifetime-binders.rs:16:43
|
LL | fn foo2(_: &'_ u8, y: &'_ u8) -> &'_ u8 { y }
| - ^ returning this value requires that `'1` must outlive `'static`
| |
| let's call the lifetime of this reference `'1`
Copy link
Contributor

Choose a reason for hiding this comment

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

This error is a shame, and it is because we use ReStatic as a fallback. That was a reasonable solution before this change, but after we should likely introduce a ReError and silence lifetime errors that involve them.

@@ -15,18 +15,18 @@ fn no_arms_or_guards(x: Void) {
//~^ ERROR a never pattern is always unreachable
None => {}
}
match None::<Void> {
match None::<Void> { //~ ERROR: `Some(_)` not covered
Some(!) if true,
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this not a parse error!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is, but we fall back to generating all arms up to and including the parse error arm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried replacing the entire match with an error expression, but then we lose the information that we don't need a semicolon after it. I guess I could encode that fact in the error variant. I'll try that out tomorrow

Copy link
Contributor

Choose a reason for hiding this comment

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

I was just surprised that we didn't have an //~ERROR annotation in the test.

@oli-obk oli-obk 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 Feb 1, 2024
@nnethercote
Copy link
Contributor

What is the motivation for this PR?

if let Some(reported) = tcx.dcx().has_errors() { Err(reported) } else { Ok(()) }
Ok(())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the motivation of this PR. removing another has_errors that affects compilation progress

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@rust-log-analyzer

This comment has been minimized.

Comment on lines 60 to +63
for i in 0..4 { //~ ERROR `for` is not allowed in a `const`
//~^ ERROR: cannot call
//~| ERROR: mutable references
//~| ERROR: cannot convert
Copy link
Contributor

Choose a reason for hiding this comment

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

We should deduplicate these errors in constants, but it's fine for now.

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

r=me

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 2, 2024

@bors r=estebank

@bors
Copy link
Contributor

bors commented Feb 2, 2024

📌 Commit 6ff8054 has been approved by estebank

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 2, 2024

🌲 The tree is currently closed for pull requests below priority 100. 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 2, 2024
@rust-log-analyzer

This comment has been minimized.

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 2, 2024

@bors r-

@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 Feb 2, 2024
@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 2, 2024

@bors r=estebank

@bors
Copy link
Contributor

bors commented Feb 2, 2024

📌 Commit 0f3976b has been approved by estebank

It is now in the queue for this repository.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 12, 2024
…=compiler-errors

fix ICE for deref coercions with type errors

Follow-up to rust-lang#120895, where I made types with errors go through the full coercion code, which is necessary if we want to build MIR for bodies with errors (rust-lang#120550).

The code for coercing `&T` to `&U` currently assumes that autoderef for `&T` will succeed for at least two steps (`&T` and `T`):

https://github.com/rust-lang/rust/blob/b17491c8f6d555386104dfd82004c01bfef09c95/compiler/rustc_hir_typeck/src/coercion.rs#L339-L464

But for types with errors, we previously only returned the no-op autoderef step (`&{type error}` -> `&{type error}`) and then stopped early. This PR changes autoderef for types with errors to still go through the built-in derefs (e.g. `&&{type error}` -> `&{type error}` -> `{type error}`) and only stop early when it would have to go looking for `Deref` trait impls.

fixes rust-lang#120945

r? `@compiler-errors` or compiler
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 12, 2024
…=compiler-errors

fix ICE for deref coercions with type errors

Follow-up to rust-lang#120895, where I made types with errors go through the full coercion code, which is necessary if we want to build MIR for bodies with errors (rust-lang#120550).

The code for coercing `&T` to `&U` currently assumes that autoderef for `&T` will succeed for at least two steps (`&T` and `T`):

https://github.com/rust-lang/rust/blob/b17491c8f6d555386104dfd82004c01bfef09c95/compiler/rustc_hir_typeck/src/coercion.rs#L339-L464

But for types with errors, we previously only returned the no-op autoderef step (`&{type error}` -> `&{type error}`) and then stopped early. This PR changes autoderef for types with errors to still go through the built-in derefs (e.g. `&&{type error}` -> `&{type error}` -> `{type error}`) and only stop early when it would have to go looking for `Deref` trait impls.

fixes rust-lang#120945

r? ``@compiler-errors`` or compiler
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2024
Rollup merge of rust-lang#120972 - lukas-code:autoderef-type-error, r=compiler-errors

fix ICE for deref coercions with type errors

Follow-up to rust-lang#120895, where I made types with errors go through the full coercion code, which is necessary if we want to build MIR for bodies with errors (rust-lang#120550).

The code for coercing `&T` to `&U` currently assumes that autoderef for `&T` will succeed for at least two steps (`&T` and `T`):

https://github.com/rust-lang/rust/blob/b17491c8f6d555386104dfd82004c01bfef09c95/compiler/rustc_hir_typeck/src/coercion.rs#L339-L464

But for types with errors, we previously only returned the no-op autoderef step (`&{type error}` -> `&{type error}`) and then stopped early. This PR changes autoderef for types with errors to still go through the built-in derefs (e.g. `&&{type error}` -> `&{type error}` -> `{type error}`) and only stop early when it would have to go looking for `Deref` trait impls.

fixes rust-lang#120945

r? ``@compiler-errors`` or compiler
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
…-errors

fix ICE for deref coercions with type errors

Follow-up to rust-lang/rust#120895, where I made types with errors go through the full coercion code, which is necessary if we want to build MIR for bodies with errors (rust-lang/rust#120550).

The code for coercing `&T` to `&U` currently assumes that autoderef for `&T` will succeed for at least two steps (`&T` and `T`):

https://github.com/rust-lang/rust/blob/b17491c8f6d555386104dfd82004c01bfef09c95/compiler/rustc_hir_typeck/src/coercion.rs#L339-L464

But for types with errors, we previously only returned the no-op autoderef step (`&{type error}` -> `&{type error}`) and then stopped early. This PR changes autoderef for types with errors to still go through the built-in derefs (e.g. `&&{type error}` -> `&{type error}` -> `{type error}`) and only stop early when it would have to go looking for `Deref` trait impls.

fixes rust-lang/rust#120945

r? ``@compiler-errors`` or compiler
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
…-errors

fix ICE for deref coercions with type errors

Follow-up to rust-lang/rust#120895, where I made types with errors go through the full coercion code, which is necessary if we want to build MIR for bodies with errors (rust-lang/rust#120550).

The code for coercing `&T` to `&U` currently assumes that autoderef for `&T` will succeed for at least two steps (`&T` and `T`):

https://github.com/rust-lang/rust/blob/b17491c8f6d555386104dfd82004c01bfef09c95/compiler/rustc_hir_typeck/src/coercion.rs#L339-L464

But for types with errors, we previously only returned the no-op autoderef step (`&{type error}` -> `&{type error}`) and then stopped early. This PR changes autoderef for types with errors to still go through the built-in derefs (e.g. `&&{type error}` -> `&{type error}` -> `{type error}`) and only stop early when it would have to go looking for `Deref` trait impls.

fixes rust-lang/rust#120945

r? ``@compiler-errors`` or compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.