Skip to content

impl restriction lowering#153556

Open
CoCo-Japan-pan wants to merge 4 commits intorust-lang:mainfrom
CoCo-Japan-pan:impl-restriction-lowering
Open

impl restriction lowering#153556
CoCo-Japan-pan wants to merge 4 commits intorust-lang:mainfrom
CoCo-Japan-pan:impl-restriction-lowering

Conversation

@CoCo-Japan-pan
Copy link
Contributor

@CoCo-Japan-pan CoCo-Japan-pan commented Mar 8, 2026 •
edited by rustbot
Loading

This PR is linked to a GSoC proposal and is part of the progress toward implementing impl restrictions proposed in RFC 3323.
This PR implements the lowering of impl restriction information to rustc_middle.

The UI tests cover path resolution errors, except for RestrictionResolutionError::ModuleOnly and RestrictionResolutionError::Indeterminate, as it seemed to be no existing UI tests for their visibility counterparts (ModuleOnly and Indeterminate).
This implementation basically follows the pattern used in #141754. It also introduces the enum RestrictionTarget in compiler/rustc_resolve/src/lib.rs to allow error handling to be shared with mut restrictions that will be added later.

r? @Urgau
cc @jhpratt

@rustbot rustbot added 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 Mar 8, 2026
@CoCo-Japan-pan CoCo-Japan-pan changed the title Impl restriction lowering impl restriction lowering Mar 8, 2026
Urgau
Urgau reviewed Mar 8, 2026
View reviewed changes
Copy link
Member

@Urgau Urgau left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Looks pretty good. Only some nits.

View changes since this review

compiler/rustc_resolve/src/lib.rs
Comment on lines +354 to +355
Indeterminate(Span),
ModuleOnly(Span),
Copy link
Member

@Urgau Urgau Mar 8, 2026

Choose a reason for hiding this comment

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

I tried to generate those errors, but I wasn't able to generate them. Probably some weird edge-case.

Let's keep them.

@Urgau 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 Mar 8, 2026
@CoCo-Japan-pan CoCo-Japan-pan force-pushed the impl-restriction-lowering branch 2 times, most recently from deed8e1 to 596133b Compare March 8, 2026 13:38
@CoCo-Japan-pan
Copy link
Contributor Author

CoCo-Japan-pan commented Mar 8, 2026

Thanks for the review!
@rustbot ready

@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 Mar 8, 2026
Urgau
Urgau reviewed Mar 8, 2026
View reviewed changes
Copy link
Member

@Urgau Urgau left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Looks mostly good, just need some positive tests now.

View changes since this review

@CoCo-Japan-pan CoCo-Japan-pan force-pushed the impl-restriction-lowering branch from 596133b to 40df631 Compare March 9, 2026 06:47
Urgau
Urgau approved these changes Mar 9, 2026
View reviewed changes
Copy link
Member

@Urgau Urgau left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Looks good to me. I will let @jhpratt do the final review.

View changes since this review

petrochenkov
petrochenkov reviewed Mar 10, 2026
View reviewed changes
compiler/rustc_resolve/src/build_reduced_graph.rs

ItemKind::Enum(ident, _, _) | ItemKind::Trait(box ast::Trait { ident, .. }) => {
ItemKind::Trait(box ast::Trait { ident, ref impl_restriction, .. }) => {
let impl_restriction = self.resolve_impl_restriction(impl_restriction);
Copy link
Contributor

@petrochenkov petrochenkov Mar 10, 2026

Choose a reason for hiding this comment

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

build_reduced_graph is the most inappropriate place for resolving names, item visibilities are resolved here only out of necessity, and it still causes issues (like #60552 and similar).

Visibilities in restrictions are not necessary for putting declarations into modules, so these visibilities should be safely resolve-able in somewhere in late.rs without adding hacks that this PR adds.

Copy link
Contributor Author

@CoCo-Japan-pan CoCo-Japan-pan Mar 10, 2026

Choose a reason for hiding this comment

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

rust/compiler/rustc_resolve/src/late.rs

Lines 2804 to 2821 in 40df631

ItemKind::Trait(box Trait { generics, bounds, items, .. }) => {
// Create a new rib for the trait-wide type parameters.
self.with_generic_param_rib(
&generics.params,
RibKind::Item(HasGenericParams::Yes(generics.span), def_kind),
item.id,
LifetimeBinderKind::Item,
generics.span,
|this| {
let local_def_id = this.r.local_def_id(item.id).to_def_id();
this.with_self_rib(Res::SelfTyParam { trait_: local_def_id }, |this| {
this.visit_generics(generics);
walk_list!(this, visit_param_bound, bounds, BoundKind::SuperTraits);
this.resolve_trait_items(items);
});
},
);
}

Maybe I should resolve and add impl restrictions here?

Copy link
Contributor

@petrochenkov petrochenkov Mar 10, 2026

Choose a reason for hiding this comment

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

Yes, that looks like the right place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@Urgau Urgau Urgau approved these changes

Assignees

@Urgau Urgau

Labels

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@CoCo-Japan-pan @Urgau @petrochenkov @rustbot