compiler: Error on layout of enums with invalid reprs #131843

workingjubilee · 2024-10-17T17:32:55Z

Surprising no one, the ICEs with the same message have the same root cause.

Invalid reprs can reach layout computation for various reasons. For instance, the compiler may want to use its layout computations to discern if a combination of layout-affecting attributes results in a valid type to begin with by e.g. computing its size. When the input is bad, return an error reflecting that the answer to the question is not a useful one.

Closes ICE: non-Aggregate field with matching ABI but differing alignment #126966
Closes ICE non-Aggregate field with matching ABI but differing alignment #128870

rustbot · 2024-10-17T17:33:04Z

r? @cjgillot

rustbot has assigned @cjgillot.
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 · 2024-10-17T20:04:52Z

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

src/tools/rust-analyzer/crates/hir-ty/src/layout.rs

compiler/rustc_abi/src/layout.rs

workingjubilee · 2024-10-17T20:28:52Z

No idea why my brain insisted it wasn't fully imported.

workingjubilee · 2024-10-17T20:42:52Z

huh, I thought ./x.py test rust-analyzer would've...

workingjubilee · 2024-10-17T21:02:40Z

oddly ./x.py build rust-analyzer works but ./x.py test rust-analyzer doesn't.

I'll take it, I guess!

workingjubilee · 2024-10-17T21:06:34Z

compiler/rustc_abi/src/layout.rs

@@ -39,7 +39,7 @@ enum NicheBias {
    End,
 }

-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]


I think rust-analyzer's derives of these are probably unnecessary but I'm not gonna fight that war today.

I think they're needed because we put the Result<Arc<Layout>, LayoutError>s into a salsa database.

Huh, interesting.

lnicola · 2024-10-17T21:09:03Z

Yeah, the r-a test suite doesn't get run (and probably doesn't pass, I think there were some issues on i386).

workingjubilee · 2024-10-17T22:51:55Z

~~how bizarre.~~ debug asserts!

workingjubilee · 2024-10-19T00:05:30Z

Hm I feel like this should go to
r? @lukas-code

jieyouxu · 2024-10-19T00:09:43Z

oddly ./x.py build rust-analyzer works but ./x.py test rust-analyzer doesn't.

r-a isn't setup as a test step in boopstrap, it is setup as a build step

workingjubilee · 2024-10-19T00:27:30Z

ah, is it finally time to promote it to its own directory?
git mv ./src/bootstrap ./boopstrap

compiler/rustc_ty_utils/src/layout/invariant.rs

lukas-code · 2024-10-19T11:33:33Z

compiler/rustc_abi/src/layout.rs

-                            assert_eq!(
-                                common_align, field.align.abi,
-                                "non-Aggregate field with matching ABI but differing alignment"
-                            );


This assert is similar to the one in the sanity check: Hitting it means that an invalid layout was already created elsewhere.

Instead of trying to make the consumers of layouts work with invalid layouts, I think we should find the place where the invalid layouts are created to begin with and emit the LayoutCalculatorError::ReprConflict there.

Philosophical discussion about the meaning of ICEs aside, that does result in a much smaller fix. I feel a bit silly for heading down that path as far as I did. Thanks!

lukas-code

Thanks!

lukas-code · 2024-10-20T12:36:55Z

@bors r+ rollup

bors · 2024-10-20T12:36:57Z

📌 Commit 9f4c915 has been approved by lukas-code

It is now in the queue for this repository.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#121560 (Allow `#[deny]` inside `#[forbid]` as a no-op) - rust-lang#131365 (Fix missing rustfmt in msi installer rust-lang#101993) - rust-lang#131647 (Register `src/tools/unicode-table-generator` as a runnable tool) - rust-lang#131843 (compiler: Error on layout of enums with invalid reprs) - rust-lang#131926 (Align boolean option descriptions in `configure.py`) - rust-lang#131961 (compiletest: tidy up how `tidy` and `tidy` (html version) are disambiguated) - rust-lang#131962 (Make `llvm::set_section` take a `&CStr`) - rust-lang#131964 (add latest crash tests) - rust-lang#131965 (remove outdated comment) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#131843 - workingjubilee:thaw-impossible-reprs, r=lukas-code compiler: Error on layout of enums with invalid reprs Surprising no one, the ICEs with the same message have the same root cause. Invalid reprs can reach layout computation for various reasons. For instance, the compiler may want to use its layout computations to discern if a combination of layout-affecting attributes results in a valid type to begin with by e.g. computing its size. When the input is bad, return an error reflecting that the answer to the question is not a useful one.

rustbot assigned cjgillot Oct 17, 2024

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 Oct 17, 2024

This comment has been minimized.

Sign in to view

lnicola reviewed Oct 17, 2024

View reviewed changes

src/tools/rust-analyzer/crates/hir-ty/src/layout.rs Outdated Show resolved Hide resolved

lnicola reviewed Oct 17, 2024

View reviewed changes

compiler/rustc_abi/src/layout.rs Outdated Show resolved Hide resolved

workingjubilee force-pushed the thaw-impossible-reprs branch from a3cd353 to a5338f7 Compare October 17, 2024 20:13

lnicola approved these changes Oct 17, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

workingjubilee force-pushed the thaw-impossible-reprs branch from a5338f7 to dbf0719 Compare October 17, 2024 20:28

This comment has been minimized.

Sign in to view

workingjubilee force-pushed the thaw-impossible-reprs branch from dbf0719 to 382a3f8 Compare October 17, 2024 21:02

workingjubilee commented Oct 17, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

workingjubilee force-pushed the thaw-impossible-reprs branch from 382a3f8 to f090e48 Compare October 17, 2024 22:02

workingjubilee changed the title ~~compiler: Do not ICE on impossible reprs~~ compiler: Do not ICE if a union has an invalid repr for a field Oct 17, 2024

workingjubilee changed the title ~~compiler: Do not ICE if a union has an invalid repr for a field~~ compiler: Do not ICE when union fields have invalid reprs Oct 17, 2024

This comment has been minimized.

Sign in to view

workingjubilee force-pushed the thaw-impossible-reprs branch 2 times, most recently from 5a9c4dd to 12ce35a Compare October 18, 2024 00:26

rustbot assigned lukas-code and unassigned cjgillot Oct 19, 2024

lukas-code reviewed Oct 19, 2024

View reviewed changes

workingjubilee mentioned this pull request Oct 19, 2024

compiler: Adopt rust-analyzer impls for LayoutCalculatorError #131942

Merged

workingjubilee force-pushed the thaw-impossible-reprs branch from 12ce35a to b3c1d23 Compare October 20, 2024 08:42

workingjubilee added 2 commits October 20, 2024 02:09

compiler: pre-move code for fixing enum layout ICEs

68d1fd9

compiler: Reject impossible reprs during enum layout

9f4c915

workingjubilee force-pushed the thaw-impossible-reprs branch from b3c1d23 to 9f4c915 Compare October 20, 2024 09:14

workingjubilee changed the title ~~compiler: Do not ICE when union fields have invalid reprs~~ compiler: Error on layout of enums with invalid reprs Oct 20, 2024

lukas-code approved these changes Oct 20, 2024

View reviewed changes

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 Oct 20, 2024

matthiaskrgr mentioned this pull request Oct 20, 2024

Rollup of 9 pull requests #131970

Merged

bors merged commit 6d43de6 into rust-lang:master Oct 20, 2024
6 checks passed

rustbot added this to the 1.84.0 milestone Oct 20, 2024

workingjubilee deleted the thaw-impossible-reprs branch October 20, 2024 19:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiler: Error on layout of enums with invalid reprs #131843

compiler: Error on layout of enums with invalid reprs #131843

workingjubilee commented Oct 17, 2024 •

edited

Loading

rustbot commented Oct 17, 2024

This comment has been minimized.

rustbot commented Oct 17, 2024

This comment has been minimized.

workingjubilee commented Oct 17, 2024

This comment has been minimized.

workingjubilee commented Oct 17, 2024

workingjubilee commented Oct 17, 2024

workingjubilee Oct 17, 2024

lnicola Oct 18, 2024 •

edited

Loading

workingjubilee Oct 19, 2024

lnicola commented Oct 17, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

workingjubilee commented Oct 17, 2024 •

edited

Loading

workingjubilee commented Oct 19, 2024

jieyouxu commented Oct 19, 2024 •

edited

Loading

workingjubilee commented Oct 19, 2024

lukas-code Oct 19, 2024

workingjubilee Oct 20, 2024

lukas-code left a comment

lukas-code commented Oct 20, 2024

bors commented Oct 20, 2024

compiler: Error on layout of enums with invalid reprs #131843

compiler: Error on layout of enums with invalid reprs #131843

Conversation

workingjubilee commented Oct 17, 2024 • edited Loading

rustbot commented Oct 17, 2024

This comment has been minimized.

rustbot commented Oct 17, 2024

This comment has been minimized.

workingjubilee commented Oct 17, 2024

This comment has been minimized.

workingjubilee commented Oct 17, 2024

workingjubilee commented Oct 17, 2024

workingjubilee Oct 17, 2024

Choose a reason for hiding this comment

lnicola Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

workingjubilee Oct 19, 2024

Choose a reason for hiding this comment

lnicola commented Oct 17, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

workingjubilee commented Oct 17, 2024 • edited Loading

workingjubilee commented Oct 19, 2024

jieyouxu commented Oct 19, 2024 • edited Loading

workingjubilee commented Oct 19, 2024

lukas-code Oct 19, 2024

Choose a reason for hiding this comment

workingjubilee Oct 20, 2024

Choose a reason for hiding this comment

lukas-code left a comment

Choose a reason for hiding this comment

lukas-code commented Oct 20, 2024

bors commented Oct 20, 2024

workingjubilee commented Oct 17, 2024 •

edited

Loading

lnicola Oct 18, 2024 •

edited

Loading

lnicola commented Oct 17, 2024 •

edited

Loading

workingjubilee commented Oct 17, 2024 •

edited

Loading

jieyouxu commented Oct 19, 2024 •

edited

Loading