impl restriction lowering

[CoCo-Japan-pan]

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

[Urgau]

Looks pretty good. Only some nits.

View changes since this review

[Urgau]

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

Let's keep them.

[CoCo-Japan-pan]

Thanks for the review!
@rustbot ready

[Urgau]

Looks mostly good, just need some positive tests now.

View changes since this review

[Urgau]

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

View changes since this review

[petrochenkov]

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.

[CoCo-Japan-pan]

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?

[petrochenkov]

Yes, that looks like the right place.

[CoCo-Japan-pan]

Moved the path resolution logic to late.rs.
This change allows resolving used paths, as well as paths referring to modules declared later.

https://github.com/CoCo-Japan-pan/rust/blob/dcc1cf5d9966a543cf1e4dc8896e3f2b04e4376c/tests/ui/impl-restriction/restriction_resolution_errors.rs#L33-L38

[petrochenkov]

Most of the logic added to rustc_resolve should be unnecessary, you can just use one of the existing standard path resolution functions used in code nearby in late.rs

(You can also assign this to me once ready.)

[petrochenkov]

Add the crate root for the 2015 edition and emit an error

Why?
For visibilities it is done for backward compatibility and due to early resolution, but impl restrictions are an entirely new context that doesn't need any of that.

Check whether the restricted module or crate is an ancestor

Even this is technically unnecessary for impl restrictions.

[jhpratt]

I'm going to cc @xonx4l who DM'd me on Zulip about a different approach. I told them that I personally favored landing something and then worrying about the finer details later. However, it's obviously not up to only me; that's only me trying to have an implementation rather than the ideal one right off the bat.

Even this is technically unnecessary for impl restrictions.

Regarding this, what makes you say that? The restriction is in fact limited to ancestor modules, so it would necessarily have to be checked for.

[petrochenkov]

The restriction is in fact limited to ancestor modules

Yeah, the ancestor module condition can be added, because that's how visibilities generally work, I'm just telling that technically it will be artificial.

I personally favored landing something

I'm ok with landing something, unless it's 90% technical debt and can be cleaned up to something reasonable easily.

[CoCo-Japan-pan]

I guess smart_resolve_path is what @petrochenkov was referring to. However, it seems to be used for resolving paths of types and values, and I could not find a PathSource variant corresponding to module or crate paths.

In this case, I thought resolve_path would be appropriate, so I added a slightly modified version of it, resolve_module_path. This essentially passes None for both Option<NameSpace> and Option<PathSource> to the internal function resolve_path_with_ribs used by resolve_path.

If my understanding is incorrect, please let me know.

rust/compiler/rustc_resolve/src/late.rs

Lines 4398 to 4413 in 40df631

	fn smart_resolve_path(
	&mut self,
	id: NodeId,
	qself: &Option<Box<QSelf>>,
	path: &Path,
	source: PathSource<'_, 'ast, 'ra>,
	) {
	self.smart_resolve_path_fragment(
	qself,
	&Segment::from_path(path),
	source,
	Finalize::new(id, path.span),
	RecordPartialRes::Yes,
	None,
	);
	}

rust/compiler/rustc_resolve/src/late.rs

Lines 1522 to 1539 in 40df631

	fn resolve_path(
	&mut self,
	path: &[Segment],
	opt_ns: Option<Namespace>, // `None` indicates a module path in import
	finalize: Option<Finalize>,
	source: PathSource<'_, 'ast, 'ra>,
	) -> PathResult<'ra> {
	self.r.cm().resolve_path_with_ribs(
	path,
	opt_ns,
	&self.parent_scope,
	Some(source),
	finalize.map(|finalize| Finalize { stage: Stage::Late, ..finalize }),
	Some(&self.ribs),
	None,
	None,
	Some(&self.diag_metadata),
	)

[xonx4l]

So the approach that I sent @jhpratt was basically because I believe the ideal enforcement should happen within the coherence engine . The idea is what if we integrate impl restriction check into orphan_check_trait_ref in the new trait solver we can trigger a coherence Error similar to E0117 before the solver even begins .
We can add impl_restriction to the interner trait to enforce it universally without duplicating the backend infrastructure Jacob already built.
Thanks!