Semantic checks of `impl` restrictions by CoCo-Japan-pan · Pull Request #154661 · rust-lang/rust

CoCo-Japan-pan · 2026-04-01T03:17:50Z

This PR implements semantic checks for impl restrictions proposed in the Restrictions RFC (Tracking Issue #105077), and linked to a GSOC idea/proposal.

It lowers the resolved paths of impl restrictions from the AST to HIR and into TraitDef, and integrates the checks into the coherence phase by extending check_impl. As parsing (#152943) and path resolution (#153556) have already been implemented, this PR provides a working implementation of impl restrictions.

r? @Urgau
cc @jhpratt

rustbot · 2026-04-01T03:17:54Z

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

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Urgau

Looks promising. Left some preliminary comments.

View changes since this review

compiler/rustc_hir/src/intravisit.rs

compiler/rustc_hir/src/hir.rs

Urgau · 2026-04-03T12:41:26Z

compiler/rustc_middle/src/ty/trait_def.rs

+                if restricted_to.krate == krate {
+                    tcx.def_path_str(restricted_to)
+                } else {
+                    tcx.crate_name(restricted_to.krate).to_ident_string()
+                }


This doesn't seems right. tcx.crate_name will only return the default crate name, but one can rename a crate when importing it, eg with extern crate serde as another_serde; or in Cargo.toml.

I think it would be safer to only call def_path_str here.

src/librustdoc/clean/mod.rs

Urgau · 2026-04-03T12:45:18Z

tests/ui/impl-restriction/impl-restriction-check.rs

+impl external::TopLevel for LocalType {} //~ ERROR trait cannot be implemented outside `external_impl_restriction`
+impl external::inner::Inner for LocalType {} //~ ERROR trait cannot be implemented outside `external_impl_restriction`


That's what I was talking about, the error here refers to external_impl_restriction instead of external.

Suggested change

impl external::TopLevel for LocalType {} //~ ERROR trait cannot be implemented outside `external_impl_restriction`

impl external::inner::Inner for LocalType {} //~ ERROR trait cannot be implemented outside `external_impl_restriction`

impl external::TopLevel for LocalType {} //~ ERROR trait cannot be implemented outside `external`

impl external::inner::Inner for LocalType {} //~ ERROR trait cannot be implemented outside `external`

When we use def_path_str for external crates, the second one will be inner instead of external::inner.
Maybe we should use with_no_trimmed_paths!?

You can try with with_no_trimmed_paths!, though I'm surprised it gives you inner and not external::inner. I wonder if def_path_str is a bit confused with external definitions.

Looking at the different overrides maybe with_resolve_crate_name! would be better for us here, as it would avoid always printing the crate name when not necessary.

with_resolve_crate_name!(tcx.def_path_str(restricted_to)) seems to output the same result as tcx.def_path_str(restricted_to) in these test cases, which gives us inner instead of external::inner.

rust/compiler/rustc_public_bridge/src/context/impls.rs

Lines 269 to 270 in 96893dc

// For local definitions, we need to prepend with crate name.

with_resolve_crate_name!(with_no_trimmed_paths!(self.tcx.def_path_str(def_id)))

Only using with_no_trimmed_paths! yields external::inner, while local items appear to omit the crate prefix (e.g., foo::bar).
On the other hand, with_resolve_crate_name!(with_no_trimmed_paths!(self.tcx.def_path_str(def_id))) does not omit the prefix for local items, but uses the crate name (e.g., impl_restriction_check::foo::bar) instead of crate.

Would it make sense to use with_no_trimmed_paths! and then explicitly add the crate prefix for local items?
Or maybe we should just use with_no_trimmed_paths! or with_resolve_crate_name!(with_no_trimmed_paths!(..))?

Given your description it seems like we could with_crate_name! + with_no_trimmed_paths! to get crate:: prefix for local modules and full path for external modules.

It that doesn't work, let's just use with_no_trimmed_paths!. The most important being that we get the resolved crate name when pointing at external modules.

Urgau · 2026-04-03T12:49:11Z

tests/ui/impl-restriction/impl-restriction-check.rs

+        pub(crate) impl(super) trait Bar {}
+        pub impl(crate) trait Baz {}
+        pub(crate) impl(in crate::foo::bar) trait Qux {}
+        pub(crate) impl(in crate::foo) trait FooBar {}


Can you add test case where it's impl(in crate::bar) here, so we can make sure it's not implementable here.

Same with a impl(in external).

crate::bar would be an invalid path, and external is not an ancestor, so both cases should already result in errors during path resolution. Is that what you had in mind?

Well mod bar {} would need to be created, but yes, I would like to have a test for those two cases.

In general, it's always good add tests for edge-case, even if they appear "impossible", you never know what users will throw at the compiler.

For ancestor-related errors involving restrictions to other crates, it might make sense to add such cases to　restriction_resolution_errors.rs

I was pointing here since there is already an aux-build, but feel free to put it where you think it makes the most sense.

CoCo-Japan-pan added 5 commits April 1, 2026 12:01

Lower impl restriction to HIR

d5c756a

Intravisit hir::ImplRestriction

01793db

Lower impl restriction information to TraitDef

bda0fce

Emit error in implementation of restricted trait

cdbccf7

Add UI tests for semantic checks of impl restrictions

222292a

rustbot assigned Urgau Apr 1, 2026

CoCo-Japan-pan changed the title ~~Impl restriction check~~ impl restriction check Apr 1, 2026

CoCo-Japan-pan changed the title ~~impl restriction check~~ Semantic checks of impl restrictions Apr 1, 2026

Urgau reviewed Apr 3, 2026

View reviewed changes

Urgau 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 Apr 3, 2026

CoCo-Japan-pan added 4 commits April 4, 2026 16:41

Call visit_path_segment directly

159f1f2

Remove shorthand from HIR

5863660

Remove the type alias for Path<'hir, DefId>

4f12609

Add a FIXME for rustdoc handling impl restrictions

65effbf

		impl external::TopLevel for LocalType {} //~ ERROR trait cannot be implemented outside `external_impl_restriction`
		impl external::inner::Inner for LocalType {} //~ ERROR trait cannot be implemented outside `external_impl_restriction`

	// For local definitions, we need to prepend with crate name.
	with_resolve_crate_name!(with_no_trimmed_paths!(self.tcx.def_path_str(def_id)))

Uh oh!

Conversation

CoCo-Japan-pan commented Apr 1, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 1, 2026

Uh oh!

Urgau 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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Urgau Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Urgau Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CoCo-Japan-pan commented Apr 1, 2026 •

edited by rustbot

Loading

Urgau left a comment •

edited by rustbot

Loading

Urgau Apr 4, 2026 •

edited

Loading

Urgau Apr 3, 2026 •

edited

Loading