Skip to content

Only assert the Parser size on specific arches #135855

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 24, 2025
Merged

Only assert the Parser size on specific arches #135855

merged 1 commit into from
Jan 24, 2025

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jan 22, 2025

The size of this struct depends on the alignment of u128, for example
powerpc64le and s390x have align-8 and end up with only 280 bytes. Our
64-bit tier-1 arches are the same though, so let's just assert on those.

r? nnethercote

@cuviper
Only assert the Parser size on specific arches
aef640a 
The size of this struct depends on the alignment of `u128`, for example
powerpc64le and s390x have align-8 and end up with only 280 bytes. Our
64-bit tier-1 arches are the same though, so let's just assert on those.
@rustbot
Copy link
Collaborator

rustbot commented Jan 22, 2025

Could not assign reviewer from: nnethercote.
User(s) nnethercote are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Jan 22, 2025

r? @oli-obk

rustbot has assigned @oli-obk.
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 Jan 22, 2025
@cuviper
Copy link
Member Author

cuviper commented Jan 22, 2025

I'm nominating this for backport because I ran into this problem on Fedora for 1.85-beta:
#133793 (comment)

@rustbot label +beta-nominated

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 22, 2025
@wesleywiser
Copy link
Member

Nick is on vacation so the assignment didn't go through but I think we can go-ahead and merge this now.

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 22, 2025

📌 Commit aef640a has been approved by wesleywiser

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 Jan 22, 2025
scottmcm
scottmcm reviewed Jan 22, 2025
View reviewed changes
compiler/rustc_parse/src/parser/mod.rs
// nonterminals. Make sure it doesn't unintentionally get bigger.
#[cfg(all(target_pointer_width = "64", not(target_arch = "s390x")))]
// nonterminals. Make sure it doesn't unintentionally get bigger. We only check a few arches
// though, because `TokenTypeSet(u128)` alignment varies on others, changing the total size.
Copy link
Member

Choose a reason for hiding this comment

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

Not this PR, and I don't know if it's worth bothering, but could try changing the other type to a https://doc.rust-lang.org/nightly/nightly-rustc/rustc_data_structures/packed/struct.Pu128.html so it'd be smaller on x64 too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Either that, or put #[repr(packed(8))] directly on TokenTypeSet too. I have no idea if there are enough of these being passed around to really matter though.

jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jan 22, 2025
@jieyouxu
Rollup merge of rust-lang#135855 - cuviper:parser-size, r=wesleywiser
8acb4de 
Only assert the `Parser` size on specific arches

The size of this struct depends on the alignment of `u128`, for example
powerpc64le and s390x have align-8 and end up with only 280 bytes. Our
64-bit tier-1 arches are the same though, so let's just assert on those.

r? nnethercote
@jieyouxu jieyouxu mentioned this pull request Jan 22, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2025
@bors
Auto merge of rust-lang#135862 - jieyouxu:rollup-s3ynwhx, r=jieyouxu
e5eea03 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#135557 (Point at invalid utf-8 span on user's source code)
 - rust-lang#135596 (Properly note when query stack is being cut off)
 - rust-lang#135638 (Make it possible to build GCC on CI)
 - rust-lang#135648 (support wasm inline assembly in `naked_asm!`)
 - rust-lang#135826 (Misc. `rustc_resolve` cleanups)
 - rust-lang#135827 (CI: free disk with in-tree script instead of GitHub Action)
 - rust-lang#135850 (Update the `wasm-component-ld` tool)
 - rust-lang#135855 (Only assert the `Parser` size on specific arches)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 23, 2025
@jhpratt
Rollup merge of rust-lang#135855 - cuviper:parser-size, r=wesleywiser
a91ac7d 
Only assert the `Parser` size on specific arches

The size of this struct depends on the alignment of `u128`, for example
powerpc64le and s390x have align-8 and end up with only 280 bytes. Our
64-bit tier-1 arches are the same though, so let's just assert on those.

r? nnethercote
@jhpratt jhpratt mentioned this pull request Jan 23, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2025
@bors
Auto merge of rust-lang#135917 - jhpratt:rollup-onqgl3k, r=jhpratt
26e039f 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#133138 (Target modifiers (special marked options) are recorded in metainfo)
 - rust-lang#133154 (Reword resolve errors caused by likely missing crate in dep tree)
 - rust-lang#135366 (Enable `unreachable_pub` lint in `test` and `proc_macro` crates)
 - rust-lang#135638 (Make it possible to build GCC on CI)
 - rust-lang#135648 (support wasm inline assembly in `naked_asm!`)
 - rust-lang#135827 (CI: free disk with in-tree script instead of GitHub Action)
 - rust-lang#135855 (Only assert the `Parser` size on specific arches)
 - rust-lang#135878 (ci: use 8 core arm runner for dist-aarch64-linux)
 - rust-lang#135905 (Enable kernel sanitizers for aarch64-unknown-none-softfloat)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 23, 2025
@matthiaskrgr
Rollup merge of rust-lang#135855 - cuviper:parser-size, r=wesleywiser
0a7816f 
Only assert the `Parser` size on specific arches

The size of this struct depends on the alignment of `u128`, for example
powerpc64le and s390x have align-8 and end up with only 280 bytes. Our
64-bit tier-1 arches are the same though, so let's just assert on those.

r? nnethercote
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 23, 2025
Closed
@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Backport labels handled by them.

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jan 23, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2025
@bors
Auto merge of rust-lang#135925 - matthiaskrgr:rollup-5dlaatt, r=jieyouxu
f6d520f 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#133154 (Reword resolve errors caused by likely missing crate in dep tree)
 - rust-lang#135366 (Enable `unreachable_pub` lint in `test` and `proc_macro` crates)
 - rust-lang#135638 (Make it possible to build GCC on CI)
 - rust-lang#135648 (support wasm inline assembly in `naked_asm!`)
 - rust-lang#135827 (CI: free disk with in-tree script instead of GitHub Action)
 - rust-lang#135855 (Only assert the `Parser` size on specific arches)
 - rust-lang#135878 (ci: use 8 core arm runner for dist-aarch64-linux)
 - rust-lang#135905 (Enable kernel sanitizers for aarch64-unknown-none-softfloat)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2025
@bors
Auto merge of rust-lang#135925 - matthiaskrgr:rollup-5dlaatt, r=jieyouxu
c353c12 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#133154 (Reword resolve errors caused by likely missing crate in dep tree)
 - rust-lang#135366 (Enable `unreachable_pub` lint in `test` and `proc_macro` crates)
 - rust-lang#135638 (Make it possible to build GCC on CI)
 - rust-lang#135648 (support wasm inline assembly in `naked_asm!`)
 - rust-lang#135827 (CI: free disk with in-tree script instead of GitHub Action)
 - rust-lang#135855 (Only assert the `Parser` size on specific arches)
 - rust-lang#135878 (ci: use 8 core arm runner for dist-aarch64-linux)
 - rust-lang#135905 (Enable kernel sanitizers for aarch64-unknown-none-softfloat)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 23, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2025
@bors
Auto merge of rust-lang#135959 - matthiaskrgr:rollup-0jenyfw, r=matth…
48ef38d 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#135366 (Enable `unreachable_pub` lint in `test` and `proc_macro` crates)
 - rust-lang#135638 (Make it possible to build GCC on CI)
 - rust-lang#135648 (support wasm inline assembly in `naked_asm!`)
 - rust-lang#135827 (CI: free disk with in-tree script instead of GitHub Action)
 - rust-lang#135855 (Only assert the `Parser` size on specific arches)
 - rust-lang#135878 (ci: use 8 core arm runner for dist-aarch64-linux)
 - rust-lang#135905 (Enable kernel sanitizers for aarch64-unknown-none-softfloat)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ec50812 into rust-lang:master Jan 24, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 24, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2025
@rust-timer
Unrolled build for rust-lang#135855
6380bc9 
Rollup merge of rust-lang#135855 - cuviper:parser-size, r=wesleywiser

Only assert the `Parser` size on specific arches

The size of this struct depends on the alignment of `u128`, for example
powerpc64le and s390x have align-8 and end up with only 280 bytes. Our
64-bit tier-1 arches are the same though, so let's just assert on those.

r? nnethercote
@cuviper cuviper mentioned this pull request Jan 24, 2025
Merged
@cuviper cuviper modified the milestones: 1.86.0, 1.85.0 Jan 24, 2025
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 24, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2025
@bors
Auto merge of rust-lang#136017 - cuviper:beta-next, r=cuviper
750a870 
[beta] backports

- Always force non-trimming of path in `unreachable_patterns` lint rust-lang#135310
- Add Profile Override for Non-Git Sources rust-lang#135433
- resolve symlinks of LLVM tool binaries before copying them rust-lang#135585
- add cache to `AmbiguityCausesVisitor` rust-lang#135618
- When LLVM's location discriminator value limit is exceeded, emit locations with dummy spans instead of dropping them entirely rust-lang#135643
- Temporarily bring back `Rvalue::Len` rust-lang#135709
- make it possible to use ci-rustc on tarball sources rust-lang#135722
- Remove test panic from File::open rust-lang#135837
- Only assert the `Parser` size on specific arches rust-lang#135855

r? cuviper
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2025
@bors
Auto merge of rust-lang#136017 - cuviper:beta-next, r=cuviper
3816a38 
[beta] backports

- Always force non-trimming of path in `unreachable_patterns` lint rust-lang#135310
- Add Profile Override for Non-Git Sources rust-lang#135433
- resolve symlinks of LLVM tool binaries before copying them rust-lang#135585
- add cache to `AmbiguityCausesVisitor` rust-lang#135618
- When LLVM's location discriminator value limit is exceeded, emit locations with dummy spans instead of dropping them entirely rust-lang#135643
- Temporarily bring back `Rvalue::Len` rust-lang#135709
- make it possible to use ci-rustc on tarball sources rust-lang#135722
- Remove test panic from File::open rust-lang#135837
- Only assert the `Parser` size on specific arches rust-lang#135855
- [beta] TRPL: more backward-compatible Edition changes rust-lang#135843

r? cuviper
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2025
@bors
Auto merge of rust-lang#136017 - cuviper:beta-next, r=cuviper
14445aa 
[beta] backports

- Always force non-trimming of path in `unreachable_patterns` lint rust-lang#135310
- Add Profile Override for Non-Git Sources rust-lang#135433
- resolve symlinks of LLVM tool binaries before copying them rust-lang#135585
- add cache to `AmbiguityCausesVisitor` rust-lang#135618
- When LLVM's location discriminator value limit is exceeded, emit locations with dummy spans instead of dropping them entirely rust-lang#135643
- make it possible to use ci-rustc on tarball sources rust-lang#135722
- Remove test panic from File::open rust-lang#135837
- Only assert the `Parser` size on specific arches rust-lang#135855
- [beta] TRPL: more backward-compatible Edition changes rust-lang#135843

r? cuviper
@cuviper cuviper deleted the parser-size branch February 27, 2025 19:35
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
@bors
Auto merge of rust-lang#135959 - matthiaskrgr:rollup-0jenyfw, r=matth…
e33ed93 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#135366 (Enable `unreachable_pub` lint in `test` and `proc_macro` crates)
 - rust-lang#135638 (Make it possible to build GCC on CI)
 - rust-lang#135648 (support wasm inline assembly in `naked_asm!`)
 - rust-lang#135827 (CI: free disk with in-tree script instead of GitHub Action)
 - rust-lang#135855 (Only assert the `Parser` size on specific arches)
 - rust-lang#135878 (ci: use 8 core arm runner for dist-aarch64-linux)
 - rust-lang#135905 (Enable kernel sanitizers for aarch64-unknown-none-softfloat)

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@scottmcm scottmcm scottmcm left review comments

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@oli-obk oli-obk

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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
Milestone
1.85.0
Development

Successfully merging this pull request may close these issues.
7 participants
@cuviper @rustbot @wesleywiser @bors @apiraino @scottmcm @oli-obk