Skip to content

Simplify static_assert_sizes. #124008

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

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

nnethercote
Copy link
Contributor

We want to run them on all 64-bit platforms.

r? @ghost

@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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 16, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 16, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

@nnethercote
Copy link
Contributor Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2024
…ze, r=<try>

Simplify `static_assert_size`s.

We want to run them on all 64-bit platforms.

r? `@ghost`
@bors
Copy link
Collaborator

bors commented Apr 16, 2024

⌛ Trying commit 54f8f52 with merge 63f3962...

@Noratrieb
Copy link
Member

i don't think the try build will be useful as it only runs x86-64 linux, but now I can't approve it :D
r=me after the try build is over

@nnethercote
Copy link
Contributor Author

So should we just mark as a non-rollup, let it attempt the merge, and see if anything fails then?

@bors
Copy link
Collaborator

bors commented Apr 16, 2024

☀️ Try build successful - checks-actions
Build commit: 63f3962 (63f3962ab602ee3449789ca7509ab405a2421f3e)

@Noratrieb
Copy link
Member

yeah, that's how we figure out what happens
@bors r+ rollup=iffy

@bors
Copy link
Collaborator

bors commented Apr 16, 2024

📌 Commit 54f8f52 has been approved by Nilstrieb

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 Apr 16, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2024
…ze, r=Nilstrieb

Simplify `static_assert_size`s.

We want to run them on all 64-bit platforms.

r? `@ghost`
@bors
Copy link
Collaborator

bors commented Apr 17, 2024

⌛ Testing commit 54f8f52 with merge 0e3de81...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Apr 17, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 17, 2024
@saethlin
Copy link
Member

saethlin commented Apr 18, 2024

The problem here is the size difference of PathBuf on Windows, because internally it has an extra bool.

It would be neat to bit-pack this away:

pub struct Wtf8Buf {
bytes: Vec<u8>,
/// Do we know that `bytes` holds a valid UTF-8 encoding? We can easily
/// know this if we're constructed from a `String` or `&str`.
///
/// It is possible for `bytes` to have valid UTF-8 without this being
/// set, such as when we're concatenating `&Wtf8`'s and surrogates become
/// paired, as we don't bother to rescan the entire string.
is_known_utf8: bool,
}

We want to run them on all 64-bit platforms.
@nnethercote nnethercote force-pushed the simpler-static_assert_size branch from 54f8f52 to 0d97669 Compare April 18, 2024 05:36
@nnethercote
Copy link
Contributor Author

The problem here is the size difference of PathBuf on Windows, because internally it has an extra bool.

I have updated that assertion accordingly. Let's try again...

@bors r=Nilstrieb

@bors
Copy link
Collaborator

bors commented Apr 18, 2024

📌 Commit 0d97669 has been approved by Nilstrieb

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 Apr 18, 2024
@bors
Copy link
Collaborator

bors commented Apr 18, 2024

⌛ Testing commit 0d97669 with merge c25473f...

@bors
Copy link
Collaborator

bors commented Apr 18, 2024

☀️ Test successful - checks-actions
Approved by: Nilstrieb
Pushing c25473f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 18, 2024
@bors bors merged commit c25473f into rust-lang:master Apr 18, 2024
13 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 18, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c25473f): 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

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)
2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results

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)
2.2% [2.2%, 2.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 678.014s -> 676.875s (-0.17%)
Artifact size: 315.34 MiB -> 315.38 MiB (0.01%)

@nnethercote nnethercote deleted the simpler-static_assert_size branch April 18, 2024 23:50
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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants