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

layout computation: gracefully handle unsized types in unexpected locations #129970

Merged
merged 4 commits into from
Sep 17, 2024

Conversation

lukas-code
Copy link
Member

@lukas-code lukas-code commented Sep 4, 2024

This PR reworks the layout computation to eagerly return an error when encountering an unsized field where a sized field was expected, rather than delaying a bug and attempting to recover a layout. This is required, because with trivially false where clauses like [T]: Sized, any field can possible be an unsized type, without causing a compile error.

Since this PR removes the delayed_bug method from the LayoutCalculator trait, it essentially becomes the same as the HasDataLayout trait, so I've also refactored the LayoutCalculator to be a simple wrapper struct around a type that implements HasDataLayout.

The majority of the diff is whitespace changes, so viewing with whitespace ignored is advised.

implements #123169 (comment)

r? @compiler-errors or compiler

fixes #123134
fixes #124182
fixes #126939
fixes #127737

@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 Sep 4, 2024
@rustbot
Copy link
Collaborator

rustbot commented Sep 4, 2024

The Miri subtree was changed

cc @rust-lang/miri

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

Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

r-a side

@rustbot
Copy link
Collaborator

rustbot commented Sep 15, 2024

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

@bors
Copy link
Contributor

bors commented Sep 16, 2024

☔ The latest upstream changes (presumably #130415) made this pull request unmergeable. Please resolve the merge conflicts.

Lukas Markeffsky added 2 commits September 17, 2024 00:06
For structs that cannot be unsized, the layout algorithm sometimes moves
unsized fields to the end of the struct, which circumvented the error
for unexpected unsized fields and returned an unsized layout anyway.

This commit makes it so that the unexpected unsized error is always
returned for structs that cannot be unsized, allowing us to remove an
old hack and fixing some old ICE.
@lukas-code lukas-code changed the title fix delayed bugs from layout computation with unsized types in unexpected locations layout computation: gracefully handle unsized types in unexpected locations Sep 16, 2024
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Great work!

@compiler-errors
Copy link
Member

r=me when green

@compiler-errors
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Sep 16, 2024

📌 Commit 20d2414 has been approved by compiler-errors

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 Sep 16, 2024
@bors
Copy link
Contributor

bors commented Sep 17, 2024

⌛ Testing commit 20d2414 with merge e2dc1a1...

@bors
Copy link
Contributor

bors commented Sep 17, 2024

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing e2dc1a1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 17, 2024
@bors bors merged commit e2dc1a1 into rust-lang:master Sep 17, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 17, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e2dc1a1): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.3% [-2.3%, -2.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.3% [-2.3%, -2.3%] 1

Cycles

Results (primary -0.6%, secondary 5.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
5.2% [5.2%, 5.2%] 1
Improvements ✅
(primary)
-0.6% [-0.6%, -0.6%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.6% [-0.6%, -0.6%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 760.616s -> 760.716s (0.01%)
Artifact size: 341.32 MiB -> 341.27 MiB (-0.02%)

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
6 participants