Skip to content

Simplify library dependencies on compiler-builtins #144683

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 3 commits into from
Aug 1, 2025

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Jul 30, 2025

The three panic-related library crates need to have access to core, and compiler-builtins needs to be in the crate graph. Rather than specifying both dependencies, switch these crates to use rustc-std-workspace-core which already does this.

This means there is now a single place that the compiler-builtins dependency needs to get configured, for everything other than alloc and std.

The second commit removes compiler-builtins from std (more details in the message).

r? @ghost

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 30, 2025
@tgross35 tgross35 changed the title Use core via rustc-std-workspace-core in library/panic* Simplify library dependencies on compiler-builtins Jul 30, 2025
@tgross35 tgross35 marked this pull request as ready for review July 30, 2025 14:26
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 30, 2025

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@tgross35
Copy link
Contributor Author

Things pass locally, assuming similar luck in CI
r? @bjorn3

@rust-log-analyzer

This comment has been minimized.

@tgross35 tgross35 force-pushed the builtins-via-std-workspace branch from 38dea0f to d7654b9 Compare July 30, 2025 14:38
@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 30, 2025
@bjorn3
Copy link
Member

bjorn3 commented Jul 30, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 30, 2025

📌 Commit d7654b9 has been approved by bjorn3

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 Jul 30, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Jul 31, 2025
…e, r=bjorn3

Simplify library dependencies on `compiler-builtins`

The three panic-related library crates need to have access to `core`, and `compiler-builtins` needs to be in the crate graph. Rather than specifying both dependencies, switch these crates to use `rustc-std-workspace-core` which already does this.

This means there is now a single place that the `compiler-builtins` dependency needs to get configured, for everything other than `alloc` and `std`.

The second commit removes `compiler-builtins` from `std` (more details in the message).
bors added a commit that referenced this pull request Jul 31, 2025
Rollup of 6 pull requests

Successful merges:

 - #135975 (Implement `push_mut`)
 - #143672 (Fix Box allocator drop elaboration)
 - #144232 (Implement support for `become` and explicit tail call codegen for the LLVM backend)
 - #144663 (coverage: Re-land "Enlarge empty spans during MIR instrumentation")
 - #144683 (Simplify library dependencies on `compiler-builtins`)
 - #144685 (Only extract lang items once in codegen_fn_attrs)

r? `@ghost`
`@rustbot` modify labels: rollup
@Noratrieb
Copy link
Member

@bors r-
failed in rollup because UEFI does funny things with compiler-builtins that I imagine one is not supposed to do: #144709 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 31, 2025
@tgross35
Copy link
Contributor Author

...okay what?

Added in 61e550a, @Ayush1325 why doesn't that create slices and do == rather than calling into compiler_builtins?

@tgross35
Copy link
Contributor Author

@bors2 try jobs=dist-various-2

@rust-bors
Copy link

rust-bors bot commented Jul 31, 2025

⌛ Trying commit 4f7f450 with merge 282345f

To cancel the try build, run the command @bors try cancel.

rust-bors bot added a commit that referenced this pull request Jul 31, 2025
Simplify library dependencies on `compiler-builtins`

try-job: dist-various-2
Copy link
Member

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

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

on the uefi change

@rust-bors
Copy link

rust-bors bot commented Jul 31, 2025

☀️ Try build successful (CI)
Build commit: 282345f (282345f2d55ac3d928eedb1cbfa2f6fc285f8969, parent: adcb3d3b4cd3b7c4cde642f3ed537037f293738e)

tgross35 added 2 commits July 31, 2025 22:47
`compiler_builtins` shouldn't be called directly. Change the `PartialEq`
implementation for `DevicePathNode` to use slice equality instead, which
will call `memcmp`/`bcmp` via the intrinsic.
The three panic-related library crates need to have access to `core`,
and `compiler-builtins` needs to be in the crate graph. Rather than
specifying both dependencies, switch these crates to use
`rustc-std-workspace-core` which already does this.

This means there is now a single place that the `compiler-builtins`
dependency needs to get configured, for everything other than `alloc`
and `std`.
`compiler-builtins` is already in the crate graph via `alloc`, and all
features related to `compiler-builtins` goes through `alloc`. There
isn't any reason that `std` needs this direct dependency, so remove it.
@tgross35 tgross35 force-pushed the builtins-via-std-workspace branch from 4f7f450 to 42bf044 Compare July 31, 2025 22:47
@tgross35
Copy link
Contributor Author

Thanks Nora!

@bors r=bjorn3,Noratrieb

@bors
Copy link
Collaborator

bors commented Jul 31, 2025

📌 Commit 42bf044 has been approved by bjorn3,Noratrieb

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 31, 2025
bors added a commit that referenced this pull request Aug 1, 2025
Rollup of 7 pull requests

Successful merges:

 - #143849 (rustdoc: never link to unnamable items)
 - #144683 (Simplify library dependencies on `compiler-builtins`)
 - #144691 (Extend `is_case_difference` to handle digit-letter confusables)
 - #144700 (rustdoc-json: Move `#[macro_export]` from `Other` to it's own  variant)
 - #144751 (Add correct dynamic_lib_extension for aix)
 - #144757 (Ping Muscraft when emitter change)
 - #144759 (triagebot: Label `compiler-builtins` T-libs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3f5f045 into rust-lang:master Aug 1, 2025
10 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Aug 1, 2025
rust-timer added a commit that referenced this pull request Aug 1, 2025
Rollup merge of #144683 - tgross35:builtins-via-std-workspace, r=bjorn3,Noratrieb

Simplify library dependencies on `compiler-builtins`

The three panic-related library crates need to have access to `core`, and `compiler-builtins` needs to be in the crate graph. Rather than specifying both dependencies, switch these crates to use `rustc-std-workspace-core` which already does this.

This means there is now a single place that the `compiler-builtins` dependency needs to get configured, for everything other than `alloc` and `std`.

The second commit removes `compiler-builtins` from `std` (more details in the message).
@tgross35 tgross35 deleted the builtins-via-std-workspace branch August 1, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants