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

Emit a wrap expr span_bug only if context is not tainted #127409

Merged
merged 1 commit into from
Jul 7, 2024

Conversation

gurry
Copy link
Contributor

@gurry gurry commented Jul 6, 2024

Fixes #127332

The ICE occurs because of this span_bug:

_ => {
// the base expression should always evaluate to a
// struct; however, when EUV is run during typeck, it
// may not. This will generate an error earlier in typeck,
// so we can just ignore it.
span_bug!(with_expr.span, "with expression doesn't evaluate to a struct");
}

which is triggered by the fact that we're trying to use an enum in a with expression instead of a struct.

The issue originates in commit 814bfe9 from PR #127202. As per the title of that commit the ICEing code should not be reachable any more, but looks like it still is.

This PR changes the code so that the span_bug will be emitted only if the context is not tainted by a previous error.

@rustbot
Copy link
Collaborator

rustbot commented Jul 6, 2024

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jul 6, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Jul 6, 2024

r? @oli-obk

Where is the actual error being reported? Can we change something at the error emission site to ensure we don't get here?

@rustbot rustbot assigned oli-obk and unassigned lcnr Jul 6, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Jul 6, 2024

And... is the type actually an enum or is it an error type by any chance?

@gurry gurry force-pushed the 127332-ice-with-expr-not-struct branch from ef01d09 to 1c99bf2 Compare July 7, 2024 07:33
@gurry
Copy link
Contributor Author

gurry commented Jul 7, 2024

r? @oli-obk

Where is the actual error being reported? Can we change something at the error emission site to ensure we don't get here?

The original error which is this:

error[E0436]: functional record update syntax requires a struct
 --> /tmp/icemaker_global_tempdir.kjjtaMiZob00/rustc_testrunner_tmpdir_reporting.bVfRd0uWVozS/mvce.rs:6:22
  |
6 |     Foo::A { x: 6, ..orig };
  |                      ^^^^

is emitted here:

self.dcx()
.emit_err(FunctionalRecordUpdateOnNonStruct { span: base_expr.span });
in the function check_expr_struct_fields.

The code path that ICEs starts later in typeck_with_fallback over here:

fcx.closure_analyze(body);

The call to analyze_closure eventually invokes ExprUseVisitor which is where the ICE occurs.

Unfortunately, I could not think of a way to cause analyze_closure to skip the ICEing code based on the original error in check_expr_struct_fields.

@gurry
Copy link
Contributor Author

gurry commented Jul 7, 2024

And... is the type actually an enum or is it an error type by any chance?

It is an enum i.e. the type returned by this call to kind:

match self.cx.try_structurally_resolve_type(with_expr.span, with_place.place.ty()).kind() {

is an enum ADT.

Fixes an ICE caused when a with expression is not a struct
@gurry gurry force-pushed the 127332-ice-with-expr-not-struct branch from 1c99bf2 to 9da3638 Compare July 7, 2024 10:15
@gurry gurry changed the title Change a span_bug to span_delayed_bug to fix an ICE Emit a wrap expr span_bug only if context is tainted Jul 7, 2024
@gurry gurry changed the title Emit a wrap expr span_bug only if context is tainted Emit a wrap expr span_bug only if context is not tainted Jul 7, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Jul 7, 2024

@bors r+ rollup

Thanks for going through all those hoops for a tiny ICE fix

@bors
Copy link
Contributor

bors commented Jul 7, 2024

📌 Commit 9da3638 has been approved by oli-obk

It is now in the queue for this repository.

@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 Jul 7, 2024
@gurry
Copy link
Contributor Author

gurry commented Jul 7, 2024

@bors r+ rollup

Thanks for going through all those hoops for a tiny ICE fix

No worries at all 😄

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 7, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#127179 (Print `TypeId` as hex for debugging)
 - rust-lang#127189 (LinkedList's Cursor: method to get a ref to the cursor's list)
 - rust-lang#127236 (doc: update config file path in platform-support/wasm32-wasip1-threads.md)
 - rust-lang#127297 (Improve std::Path's Hash quality by avoiding prefix collisions)
 - rust-lang#127308 (Attribute cleanups)
 - rust-lang#127354 (Describe Sized requirements for mem::offset_of)
 - rust-lang#127409 (Emit a wrap expr span_bug only if context is not tainted)
 - rust-lang#127447 (once_lock: make test not take as long in Miri)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1ee6345 into rust-lang:master Jul 7, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 7, 2024
Rollup merge of rust-lang#127409 - gurry:127332-ice-with-expr-not-struct, r=oli-obk

Emit a wrap expr span_bug only if context is not tainted

Fixes rust-lang#127332

The ICE occurs because of this `span_bug`: https://github.com/rust-lang/rust/blob/51917e2e69702e5752bce6a4f3bfd285d0f4ae39/compiler/rustc_hir_typeck/src/expr_use_visitor.rs#L732-L738
which is triggered by the fact that we're trying to use an `enum` in a `with` expression instead of a `struct`.

The issue originates in commit rust-lang@814bfe9   from PR rust-lang#127202. As per the title of that commit the ICEing code should not be reachable any more, but looks like it still is.

This PR changes the code so that the `span_bug` will be emitted only if the context is not tainted by a previous error.
@gurry gurry deleted the 127332-ice-with-expr-not-struct branch July 8, 2024 00:34
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.

ICE: with expression doesn't evaluate to a struct
5 participants