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

regression: internal compiler error value <= 0xFFFF_FF00 in rustc_abi #111483

Closed
Mark-Simulacrum opened this issue May 11, 2023 · 2 comments
Closed

regression: internal compiler error value <= 0xFFFF_FF00 in rustc_abi #111483

Mark-Simulacrum opened this issue May 11, 2023 · 2 comments
Assignees
@compiler-errors
Labels
I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone
1.70.0

Comments

@Mark-Simulacrum
Copy link
Member

See https://crater-reports.s3.amazonaws.com/beta-1.70-2/beta-2023-05-08/gh/skius.headlessmc/log.txt

thread 'rustc' panicked at 'assertion failed: value <= 0xFFFF_FF00', /rustc/2013813b65016b39d456a12e3b0a90366e9fb5e3/compiler/rustc_abi/src/lib.rs:1407:1
...
@Mark-Simulacrum Mark-Simulacrum added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels May 11, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 11, 2023
@Mark-Simulacrum Mark-Simulacrum added this to the 1.70.0 milestone May 11, 2023
@Mark-Simulacrum Mark-Simulacrum added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label May 11, 2023
@Mark-Simulacrum Mark-Simulacrum mentioned this issue May 12, 2023
Closed
@compiler-errors compiler-errors self-assigned this May 12, 2023
@compiler-errors compiler-errors mentioned this issue May 12, 2023
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 13, 2023
@Dylan-DPC
Rollup merge of rust-lang#111494 - compiler-errors:variant-order, r=p…
8c89601 
…etrochenkov

Encode `VariantIdx` so we can decode ADT variants in the right order

As far as I can tell, we don't guarantee anything about the ordering of `DefId`s and module children...

The code that motivated this PR (rust-lang#111483) looks something like:

```rust
#[derive(Protocol)]
pub enum Data {
    #[protocol(discriminator(0x00))]
    Disconnect(Disconnect),
    EncryptionRequest,
    /* more variants... */
}
```

The specific macro ([`protocol`](https://github.com/dylanmckay/protocol)) doesn't really matter, but as far as I can tell (from calls to `build_reduced_graph`), the presence of that `#[protocol(..)]` helper attribute causes the def-id of the `Disconnect` enum variant to be collected *after* its siblings, and it shows up after the other variants in `module_children`.

When we decode the variants for `Data` in a child crate (an example test, in this case), this means that the `Disconnect` variant is moved to the end of the variants list, and all of the other variants now have incorrect relative discriminant data, causing the ICE.

This PR fixes this by sorting manually by variant index after they are decoded. I guess there are alternative ways of fixing this, such as not reusing `module_children_non_reexports` to encode the order-sensitive ADT variants, or to do some sorting in `rustc_resolve`... but none of those seemed particularly satisfying either.

~I really struggled to create a reproduction here -- it required at least 3 crates, one of which is a proc macro, and then some code to actually compute discriminants in the child crate... Needless to say, I failed to repro this in a test, but I can confirm that it fixes the regression in rust-lang#111483.~ Test exists now.

r? `@petrochenkov` but feel free to reassign. ~Again, sorry for no test, but I hope the explanation at least suggests why a fix like this is likely necessary.~ Feedback is welcome.
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 16, 2023
@Mark-Simulacrum
Copy link
Member Author

Closing as fix, merged and backported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@compiler-errors compiler-errors

Labels
I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.70.0
Development

No branches or pull requests
4 participants
@compiler-errors @Mark-Simulacrum @apiraino @rustbot