-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Rollup of 5 pull requests #97727
Closed
Closed
Rollup of 5 pull requests #97727
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Normally, `Thread::spawn` takes care of setting the thread's name, if one was provided, but since the main thread wasn't created by calling `Thread::spawn`, we need to call that function in `std::rt::init`. This is mainly useful for system tools like debuggers and profilers which might show the thread name to a user. Prior to these changes, gdb and WinDbg would show all thread names except the main thread's name to a user. I've validated that this patch resolves the issue for both debuggers.
Co-authored-by: Chris Denton <[email protected]>
Avoid zero-sized allocs in ThinBox if T and H are both ZSTs. This was surprisingly tricky, and took longer to get right than expected. `ThinBox` is a surprisingly subtle piece of code. That said, in the end, a lot of this was due to overthinking[^overthink] -- ultimately the fix ended up fairly clean and simple. [^overthink]: Honestly, for a while I was convinced this couldn't be done without allocations or runtime branches in these cases, but that's obviously untrue. Anyway, as a result of spending all that time debugging, I've extended the tests quite a bit, and also added more debug assertions. Many of these helped for subtle bugs I made in the middle (for example, the alloc/drop tracking is because I ended up double-dropping the value in the case where both were ZSTs), they're arguably a bit of overkill at this point, although I imagine they could help in the future too. Anyway, these tests cover a wide range of size/align cases, nd fully pass under miri[^1]. They also do some smoke-check asserting that the value has the correct alignment, although in practice it's totally within the compiler's rights to delete these assertions since we'd have already done UB if they get hit. They have more boilerplate than they really need, but it's not *too* bad on a per-test basis. A notable absence from testing is atypical header types, but at the moment it's impossible to manually implement `Pointee`. It would be really nice to have testing here, since it's not 100% obvious to me that the aligned read/write we use for `H` are correct in the face of arbitrary combinations of `size_of::<H>()`, `align_of::<H>()`, and `align_of::<T>()`. (That said, I spent a while thinking through it and am *pretty* sure it's fine -- I'd just feel... better if we could test some cases for non-ZST headers which have unequal and align). [^1]: Or at least, they pass under miri if I copy the code and tests into a new crate and run miri on it (after making it less stdlibified). Fixes rust-lang#96485. I'd request review `@yaahc,` but I believe you're taking some time away from reviews, so I'll request from the previous PR's reviewer (I think that the context helps, even if the actual change didn't end up being bad here). r? `@joshtriplett`
…isDenton Call the OS function to set the main thread's name on program init Normally, `Thread::spawn` takes care of setting the thread's name, if one was provided, but since the main thread wasn't created by calling `Thread::spawn`, we need to call that function in `std::rt::init`. This is mainly useful for system tools like debuggers and profilers which might show the thread name to a user. Prior to these changes, gdb and WinDbg would show all thread names except the main thread's name to a user. I've validated that this patch resolves the issue for both debuggers.
Lazily allocate and initialize pthread locks. Lazily allocate and initialize pthread locks. This allows {Mutex, Condvar, RwLock}::new() to be const, while still using the platform's native locks for features like priority inheritance and debug tooling. E.g. on macOS, we cannot directly use the (private) APIs that pthread's locks are implemented with, making it impossible for us to use anything other than pthread while still preserving priority inheritance, etc. This PR doesn't yet make the public APIs const. That's for a separate PR with an FCP. Tracking issue: rust-lang#93740
…g, r=wesleywiser Support the `#[expect]` attribute on fn parameters (RFC-2383) A small PR to allow the `#[expect]` attribute on function parameters. Nothing more to say, I hope everyone reading this has a lovely day. --- r? `@wesleywiser` closes: rust-lang#97650 cc: rust-lang#85549
…eywiser Fix reachability analysis for const methods Use `method_might_be_inlined` directly for `ImplItemKind::Fn` instead of duplicating the logic in `def_id_represents_local_inlined_item`. This is parallel to how we use `item_might_be_inlined` for `ItemKind::Fn` in that same body. Fixes rust-lang#97708
rustbot
added
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
T-libs
Relevant to the library team, which will review and decide on the PR/issue.
rollup
A PR which is a rollup
labels
Jun 4, 2022
@bors r+ rollup=never p=5 |
📌 Commit fd27ab2 has been approved by |
bors
added
the
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
label
Jun 4, 2022
⌛ Testing commit fd27ab2 with merge b6096629facc5b41d2de057e625ee54c5eca5d01... |
💔 Test failed - checks-actions |
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
Jun 4, 2022
The job Click to see the possible cause of the failure (guessed by this bot)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
rollup
A PR which is a rollup
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-libs
Relevant to the library team, which will review and decide on the PR/issue.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
#[expect]
attribute on fn parameters (RFC-2383) #97715 (Support the#[expect]
attribute on fn parameters (RFC-2383))Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup