Reject linked dylib EII default overrides by qaijuang · Pull Request #156370 · rust-lang/rust

qaijuang · 2026-05-09T17:58:50Z

This PR rejects explicit EII implementations that would override a default implementation already selected through a linked dylib.

The check is intentionally split:

rustc_passes keeps the early, format-independent checks for missing impls and duplicate explicit impls.
rustc_codegen_ssa checks the default-vs-explicit conflict during linking, using the dependency formats selected for the final artifact.

Fixes #156320.

rustbot · 2026-05-09T20:28:21Z

r? @mejrs

rustbot has assigned @mejrs.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 19 candidates

qaijuang · 2026-05-09T20:28:42Z

cc @bjorn3

mejrs · 2026-05-09T21:51:59Z

r? @jdonszelmann (or @bjorn3 perhaps)

rustbot · 2026-05-09T21:52:04Z

jdonszelmann is currently at their maximum review capacity.
They may take a while to respond.

bjorn3 · 2026-05-10T17:08:25Z

Thinking about this a bit more, I'm wondering if maybe check_externally_implementable_items should be done only at link time? tcx.used_crate_source() may report that only a dylib exists even when at link time an rlib will be available and linked in. And tcx.dependency_formats() will report everything as NotLinked inside rlibs and would ideally only be called during the link step for -Zno-link/-Zlink-only to work (it is currently broken because dependency_formats is called during codegen. i've been working on moving this call to the link step for the past several months). On the other hand for all cases that the check caught before this PR, doing it before linking is fine. So splitting it into separate checks for duplicate explicit impls and for a default impl conflicting with an explicit impl would preserve early error messages (especially for cargo check) What do you think @jdonszelmann?

qaijuang · 2026-05-11T10:16:56Z

Thinking about this a bit more, I'm wondering if maybe check_externally_implementable_items should be done only at link time? tcx.used_crate_source() may report that only a dylib exists even when at link time an rlib will be available and linked in. And tcx.dependency_formats() will report everything as NotLinked inside rlibs and would ideally only be called during the link step for -Zno-link/-Zlink-only to work (it is currently broken because dependency_formats is called during codegen.

Nice catch @bjorn3

So splitting it into separate checks for duplicate explicit impls and for a default impl conflicting with an explicit impl would preserve early error messages (especially for cargo check)

right? seems like the next best move

qaijuang · 2026-05-11T15:23:13Z

@bjorn3 I'm done with the split, what do you think?

jdonszelmann · 2026-05-28T12:34:59Z

@bjorn3 that approach seems reasonable to me

…uwer Rollup of 17 pull requests Successful merges: - #158510 (Enable `static_position_independent_executables` on all gnu and musl targets) - #158541 (Move `std::io::Write` to `core::io`) - #158899 (Fix `unaligned_volatile_store` by removing `MemFlags::UNALIGNED`) - #155429 (Support `u128`/`i128` c-variadic arguments) - #156370 (Reject linked dylib EII default overrides) - #157153 (allow `Allocator`s to be used as `#[global_allocator]`s) - #158535 (Support `#[track_caller]` on EII declarations) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158988 (Redo `TokenStreamIter`) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158739 (view-types: HIR lowering) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158886 (Add documentation for the `no_std` attribute) - #158951 (Merge three `MaxUniverse`s into one) - #158961 (Reapply "LLVM 23: Run AssignGUIDPass in some places")

JonathanBrouwer · 2026-07-09T13:03:12Z

💔 I suspect this PR failed tests as part of a rollup
@bors r-

After fixing the problem, consider running a try job for the failed job before re-approving.

Link to failure: https://triage.rust-lang.org/gha-logs/rust-lang/rust/86103732041

rust-bors · 2026-07-09T13:03:19Z

This pull request was unapproved.

View changes since this unapproval

JonathanBrouwer · 2026-07-09T13:05:31Z

Actually not sure it's this pr?
@bors try jobs=aarch64-apple

Reject linked dylib EII default overrides try-job: aarch64-apple

JonathanBrouwer · 2026-07-09T14:29:53Z

Yeah this pr was not the problem
@bors r=bjorn3

rust-bors · 2026-07-09T14:29:57Z

📌 Commit 41bdaed has been approved by bjorn3

It is now in the queue for this repository.

…e, r=bjorn3 Reject linked dylib EII default overrides This PR rejects explicit EII implementations that would override a default implementation already selected through a linked dylib. The check is intentionally split: - rustc_passes keeps the early, format-independent checks for missing impls and duplicate explicit impls. - rustc_codegen_ssa checks the default-vs-explicit conflict during linking, using the dependency formats selected for the final artifact. Fixes rust-lang#156320.

rust-bors · 2026-07-09T16:31:09Z

☀️ Try build successful (CI)
Build commit: ca8c27e (ca8c27e642e2f130c798e317e248073b03588e9b)
Base parent: 561ea2b (561ea2bfa3022226082e2db39d6ccb241e9e34bc)

…uwer Rollup of 21 pull requests Successful merges: - #150946 (intrinsics: Add a fallback for non-const libm float functions) - #158510 (Enable `static_position_independent_executables` on all gnu and musl targets) - #158541 (Move `std::io::Write` to `core::io`) - #158899 (Fix `unaligned_volatile_store` by removing `MemFlags::UNALIGNED`) - #155429 (Support `u128`/`i128` c-variadic arguments) - #156370 (Reject linked dylib EII default overrides) - #156508 (Infer all anonymous lifetimes in assoc consts as `'static`) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158859 (Improve `-Zls` diagnostic message on `.rs` files) - #158988 (Redo `TokenStreamIter`) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158739 (view-types: HIR lowering) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158886 (Add documentation for the `no_std` attribute) - #158940 (Implement feature `char_to_u32`) - #158951 (Merge three `MaxUniverse`s into one) - #158961 (Reapply "LLVM 23: Run AssignGUIDPass in some places") - #158996 ([compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes) - #159005 (Use `as_lang_item` instead of repeatedly matching)

…e, r=bjorn3 Reject linked dylib EII default overrides This PR rejects explicit EII implementations that would override a default implementation already selected through a linked dylib. The check is intentionally split: - rustc_passes keeps the early, format-independent checks for missing impls and duplicate explicit impls. - rustc_codegen_ssa checks the default-vs-explicit conflict during linking, using the dependency formats selected for the final artifact. Fixes rust-lang#156320.

…uwer Rollup of 24 pull requests Successful merges: - #150946 (intrinsics: Add a fallback for non-const libm float functions) - #158510 (Enable `static_position_independent_executables` on all gnu and musl targets) - #158541 (Move `std::io::Write` to `core::io`) - #158899 (Fix `unaligned_volatile_store` by removing `MemFlags::UNALIGNED`) - #156027 (Consider captured regions for opaque type region liveness.) - #156370 (Reject linked dylib EII default overrides) - #156508 (Infer all anonymous lifetimes in assoc consts as `'static`) - #157561 (rustdoc: do not include extra stuff in span) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158859 (Improve `-Zls` diagnostic message on `.rs` files) - #158988 (Redo `TokenStreamIter`) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158384 (Allow BackwardIncompatibleDropHint in polonius legacy) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158739 (view-types: HIR lowering) - #158877 (borrowck: Keep returned `path` from `best_blame_constraint()` consistent) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158886 (Add documentation for the `no_std` attribute) - #158940 (Implement feature `char_to_u32`) - #158951 (Merge three `MaxUniverse`s into one) - #158961 (Reapply "LLVM 23: Run AssignGUIDPass in some places") - #158995 (Use REST API in linkchecker script) - #158996 ([compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes)

…e, r=bjorn3 Reject linked dylib EII default overrides This PR rejects explicit EII implementations that would override a default implementation already selected through a linked dylib. The check is intentionally split: - rustc_passes keeps the early, format-independent checks for missing impls and duplicate explicit impls. - rustc_codegen_ssa checks the default-vs-explicit conflict during linking, using the dependency formats selected for the final artifact. Fixes rust-lang#156320.

Rollup of 24 pull requests Successful merges: - #150946 (intrinsics: Add a fallback for non-const libm float functions) - #158541 (Move `std::io::Write` to `core::io`) - #156027 (Consider captured regions for opaque type region liveness.) - #156370 (Reject linked dylib EII default overrides) - #156508 (Infer all anonymous lifetimes in assoc consts as `'static`) - #157561 (rustdoc: do not include extra stuff in span) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158859 (Improve `-Zls` diagnostic message on `.rs` files) - #158988 (Redo `TokenStreamIter`) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158384 (Allow BackwardIncompatibleDropHint in polonius legacy) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158739 (view-types: HIR lowering) - #158877 (borrowck: Keep returned `path` from `best_blame_constraint()` consistent) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158886 (Add documentation for the `no_std` attribute) - #158940 (Implement feature `char_to_u32`) - #158951 (Merge three `MaxUniverse`s into one) - #158960 (Fix bootstrap submodule path prefix matching) - #158961 (Reapply "LLVM 23: Run AssignGUIDPass in some places") - #158995 (Use REST API in linkchecker script) - #158996 ([compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes) - #159036 (bootstrap: expand '@argfile' arguments to rustc shim)

Rollup merge of #156370 - qaijuang:eii-dylib-default-override, r=bjorn3 Reject linked dylib EII default overrides This PR rejects explicit EII implementations that would override a default implementation already selected through a linked dylib. The check is intentionally split: - rustc_passes keeps the early, format-independent checks for missing impls and duplicate explicit impls. - rustc_codegen_ssa checks the default-vs-explicit conflict during linking, using the dependency formats selected for the final artifact. Fixes #156320.

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 9, 2026

qaijuang marked this pull request as ready for review May 9, 2026 20:28

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 9, 2026

rustbot assigned mejrs May 9, 2026

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 9, 2026

rustbot assigned jdonszelmann and unassigned mejrs May 9, 2026

qaijuang force-pushed the eii-dylib-default-override branch from dfc410a to 44e8bb8 Compare May 11, 2026 15:11

qaijuang changed the title ~~Reject EII overrides of dylib defaults~~ Reject linked dylib EII default overrides May 11, 2026

This comment has been minimized.

Sign in to view

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 14, 2026

qaijuang force-pushed the eii-dylib-default-override branch from 5af5be7 to b41cef3 Compare May 14, 2026 20:14

rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 14, 2026

bjorn3 reviewed May 28, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_ssa/src/back/link.rs Outdated

bjorn3 reviewed May 28, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_ssa/src/base.rs Outdated

qaijuang force-pushed the eii-dylib-default-override branch from 3827ccb to 621745e Compare May 28, 2026 17:36

This comment has been minimized.

Sign in to view

rust-bors Bot 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 9, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Jul 9, 2026

Auto merge of #156370 - qaijuang:eii-dylib-default-override, r=<try>

ca8c27e

Reject linked dylib EII default overrides try-job: aarch64-apple

rust-bors Bot 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 9, 2026

JonathanBrouwer mentioned this pull request Jul 9, 2026

Rollup of 21 pull requests #159020

Closed

JonathanBrouwer mentioned this pull request Jul 9, 2026

Rollup of 21 pull requests #159024

Closed

JonathanBrouwer mentioned this pull request Jul 9, 2026

Rollup of 24 pull requests #159037

Closed

This was referenced Jul 9, 2026

Rollup of 25 pull requests #159045

Closed

Rollup of 24 pull requests #159046

Merged

rust-bors Bot merged commit 9210cb9 into rust-lang:main Jul 10, 2026
14 checks passed

rustbot added this to the 1.99.0 milestone Jul 10, 2026

qaijuang deleted the eii-dylib-default-override branch July 10, 2026 07:00

Uh oh!

Uh oh!

Conversation

qaijuang commented May 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented May 9, 2026

Uh oh!

qaijuang commented May 9, 2026

Uh oh!

mejrs commented May 9, 2026

Uh oh!

rustbot commented May 9, 2026

Uh oh!

bjorn3 commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qaijuang commented May 11, 2026

Uh oh!

qaijuang commented May 11, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jdonszelmann commented May 28, 2026

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

JonathanBrouwer commented Jul 9, 2026

Uh oh!

rust-bors Bot commented Jul 9, 2026

Uh oh!

This comment has been minimized.

JonathanBrouwer commented Jul 9, 2026

Uh oh!

This comment has been minimized.

JonathanBrouwer commented Jul 9, 2026

Uh oh!

rust-bors Bot commented Jul 9, 2026

Uh oh!

rust-bors Bot commented Jul 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

qaijuang commented May 9, 2026 •

edited by rustbot

Loading

bjorn3 commented May 10, 2026 •

edited

Loading