Check unsafe impls on safe EIIs by AsakuraMizu · Pull Request #159783 · rust-lang/rust

AsakuraMizu · 2026-07-23T17:15:24Z

Tracking issue: #125418

Unsafe EIIs require implementations to use #[unsafe(...)], but safe EIIs currently accept this form as well. This should be rejected, just like unsafe impl on a safe trait.

This also fixes existing EII UI tests using this pattern and adds a new regression test.

rustbot · 2026-07-23T17:15:28Z

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-07-23T17:15:30Z

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

Why was this reviewer chosen?

The reviewer was selected based on:

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

mejrs · 2026-07-23T18:18:19Z

Another approach is to emit the unused_unsafe lint instead.

Right now, it is a breaking change to declare a unsafe trait but later remove the unsafe requirement. On the other hand, it is not a breaking change to have a unsafe function and then later remove the unsafe requirement.

This PR proposes to go the former way, where both adding and removing the unsafe requirement are breaking changes.

I'm undecided on what's best (so please don't go off making changes now), but note that this PR is a change to the semantics of the feature, not merely a "fix".

bjorn3 · 2026-07-23T19:30:08Z

I think it makes sense to follow the example of traits given that EIIs are kinda traits that can have a single global impl. If we decide to make removing unsafe from traits no longer a breaking change, we can adjust EII to follow this.

mejrs

That sounds good to me, let's do it.

r? me

View changes since this review

mejrs · 2026-07-24T09:22:59Z

 }

+#[derive(Diagnostic)]
+#[diag("`#[{$name}]` is not unsafe to implement")]


As of #159755 we don't mention the #[/] parts

Suggested change

#[diag("`#[{$name}]` is not unsafe to implement")]

#[diag("`{$name}` is not unsafe to implement")]

mejrs · 2026-07-24T09:26:31Z

+    #[primary_span]
+    #[label("`unsafe` is not allowed here")]
+    pub span: Span,


For consistency with regular attributes I'd prefer if this message pointed at the entire attribute, and then the label only to the redundant unsafe

Suggested change

#[primary_span]

#[label("`unsafe` is not allowed here")]

pub span: Span,

#[primary_span]

pub impl_span: Span,

#[label("`unsafe` is not allowed here")]

pub unsafe_span: Span,

rustbot · 2026-07-24T09:26:59Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

rustbot · 2026-07-24T09:48:59Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

AsakuraMizu · 2026-07-24T09:52:37Z

Resolved conflicts and updated according to review comments.

@rustbot ready

AsakuraMizu · 2026-07-24T09:55:43Z

@rustbot label +F-extern_item_impls

mejrs

Thanks!

View changes since this review

mejrs · 2026-07-24T11:01:40Z

+            impl_unsafe_span: match *impl_safety {
+                Safety::Unsafe(span) => Some(self.lower_span(span)),
+                Safety::Safe(_) | Safety::Default => None,
+            },


I'm fine with this change and would prefer to address the concept of how we carry around "safety" in the compiler in a more comprehensive manner (i.e., not here and not today).

@JonathanBrouwer are you okay with this change or would you rather we lower+carry Safety here?

Using rustc_hir::Safety would be nicer as the type is more descriptive, but it does not contain the span so unless we change that (which is too much for this PR) it's not usable. This is fine for now

JonathanBrouwer

@bors r=mejrs,JonathanBrouwer rollup

View changes since this review

rust-bors · 2026-07-25T18:53:51Z

📌 Commit bae7e21 has been approved by mejrs,JonathanBrouwer

It is now in the queue for this repository.

…r=mejrs,JonathanBrouwer Check unsafe impls on safe EIIs Tracking issue: rust-lang#125418 Unsafe EIIs require implementations to use `#[unsafe(...)]`, but safe EIIs currently accept this form as well. This should be rejected, just like `unsafe impl` on a safe trait. This also fixes existing EII UI tests using this pattern and adds a new regression test.

…uwer Rollup of 24 pull requests Successful merges: - #159638 (bootstrap: Split the `Step` trait into multiple traits) - #159774 (rustc_trait_selection: fix trait solver hang caused by degenerate obligations) - #159837 (line-tables-only test: check that the line number matches the function name) - #159946 (Update Enzyme submodule to imporve llvm-cov) - #159617 (Fix up `#[linkage]` target checking) - #159733 (std: Switch implementations of `thread_local!` for WASI) - #159783 (Check unsafe impls on safe EIIs) - #159810 (Add tuple never coercion collection regression test) - #159821 (Update expect message using the recommended style in binary_heap module) - #159826 (Remove redundant `#[rustc_paren_sugar]` feature gate) - #159846 (Implement `str::copy_from_str`) - #159849 (rustc_parse: Stop returning `Option` from statement parsing) - #159853 (Updated expect messages for `CString` struct and method documentation) - #159875 (More cleanup in `rustc_attr_parsing`) - #159882 (Update expect messages in library/alloc/boxed.rs and library/alloc/string.rs to follow the style guide) - #159891 (Split multiline derives into std/rustc macros) - #159893 (Fix `find_attr` hygiene and `rustc_hir` cleanups) - #159895 (rustc-dev-guide subtree update) - #159902 (Clarify that the expected runtime symbols signature is for the current target only) - #159914 (Fix error in diagnostic on_unmatched_args) - #159917 (spare capacity mut constification) - #159918 (rename abort_unwind → abort_on_unwind) - #159936 (Minor `rustc_ast::ast` doc cleanups) - #159945 (Update expect messages in library/core/src/ptr/non_null.rs)

…r=mejrs,JonathanBrouwer Check unsafe impls on safe EIIs Tracking issue: rust-lang#125418 Unsafe EIIs require implementations to use `#[unsafe(...)]`, but safe EIIs currently accept this form as well. This should be rejected, just like `unsafe impl` on a safe trait. This also fixes existing EII UI tests using this pattern and adds a new regression test.

…uwer Rollup of 28 pull requests Successful merges: - #159638 (bootstrap: Split the `Step` trait into multiple traits) - #159774 (rustc_trait_selection: fix trait solver hang caused by degenerate obligations) - #159837 (line-tables-only test: check that the line number matches the function name) - #159946 (Update Enzyme submodule to imporve llvm-cov) - #159962 (miri subtree update) - #156570 (tests: extend remap-path-prefix-std to all stdlib rlibs) - #159617 (Fix up `#[linkage]` target checking) - #159633 (Improve workings of attribute suggestions) - #159733 (std: Switch implementations of `thread_local!` for WASI) - #159783 (Check unsafe impls on safe EIIs) - #159810 (Add tuple never coercion collection regression test) - #159826 (Remove redundant `#[rustc_paren_sugar]` feature gate) - #159846 (Implement `str::copy_from_str`) - #159849 (rustc_parse: Stop returning `Option` from statement parsing) - #159853 (Updated expect messages for `CString` struct and method documentation) - #159875 (More cleanup in `rustc_attr_parsing`) - #159882 (Update expect messages in library/alloc/boxed.rs and library/alloc/string.rs to follow the style guide) - #159891 (Split multiline derives into std/rustc macros) - #159893 (Fix `find_attr` hygiene and `rustc_hir` cleanups) - #159895 (rustc-dev-guide subtree update) - #159902 (Clarify that the expected runtime symbols signature is for the current target only) - #159914 (Fix error in diagnostic on_unmatched_args) - #159917 (spare capacity mut constification) - #159918 (rename abort_unwind → abort_on_unwind) - #159927 (Remove sve2 from the ImpliedFeatures of AArch64 v9a.) - #159936 (Minor `rustc_ast::ast` doc cleanups) - #159945 (Update expect messages in library/core/src/ptr/non_null.rs) - #159950 (Add CFI tests for return types and never type)

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) 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 Jul 23, 2026

rustbot assigned oli-obk Jul 23, 2026

This comment has been minimized.

Sign in to view

mejrs requested changes Jul 24, 2026

View reviewed changes

rustbot assigned mejrs and unassigned oli-obk Jul 24, 2026

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2026

Check unsafe impls on safe EIIs

bae7e21

AsakuraMizu force-pushed the safe-eii-unsafe-impl branch from e8c3cb3 to bae7e21 Compare July 24, 2026 09:48

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 24, 2026

rustbot added the F-extern_item_impls `#![feature(extern_item_impls)]` label Jul 24, 2026

mejrs approved these changes Jul 24, 2026

View reviewed changes

JonathanBrouwer approved these changes Jul 25, 2026

View reviewed changes

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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2026

JonathanBrouwer mentioned this pull request Jul 25, 2026

Rollup of 15 pull requests #159925

Closed

JonathanBrouwer mentioned this pull request Jul 25, 2026

Rollup of 14 pull requests #159931

Closed

JonathanBrouwer mentioned this pull request Jul 26, 2026

Rollup of 25 pull requests #159952

Closed

JonathanBrouwer mentioned this pull request Jul 26, 2026

Rollup of 24 pull requests #159957

Closed

JonathanBrouwer mentioned this pull request Jul 26, 2026

Rollup of 28 pull requests #159966

Open

	#[diag("`#[{$name}]` is not unsafe to implement")]
	#[diag("`{$name}` is not unsafe to implement")]

Uh oh!

Uh oh!

Conversation

AsakuraMizu commented Jul 23, 2026

Uh oh!

rustbot commented Jul 23, 2026

Uh oh!

rustbot commented Jul 23, 2026

Uh oh!

mejrs commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjorn3 commented Jul 23, 2026

Uh oh!

This comment has been minimized.

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mejrs Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jul 24, 2026

Uh oh!

rustbot commented Jul 24, 2026

Uh oh!

AsakuraMizu commented Jul 24, 2026

Uh oh!

AsakuraMizu commented Jul 24, 2026

Uh oh!

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mejrs commented Jul 23, 2026 •

edited

Loading

mejrs left a comment •

edited by rustbot

Loading

mejrs left a comment •

edited by rustbot

Loading

JonathanBrouwer left a comment •

edited by rustbot

Loading