Privacy: move macros handling to early stage

[Bryanskiy]

The patch moves effective visibility computation for macros from rustc_privacy to rustc_resolve. It will enable this optimization: #156228.

However, I found some problems with macro handling while I was doing this. The current implementation was written ~6 years ago and checks the reachability of a definition from a macro by nominal visibility. In general this is incorrect.

For example, in the current implementation modules are not traversed if their nominal visibility is less then the nominal visibility of a module defining macro:

rust/compiler/rustc_privacy/src/lib.rs

Lines 618 to 626 in 29b7590

	DefKind::Mod => {
	if vis.is_accessible_from(module, self.tcx) {
	self.update_macro_reachable(
	LocalModDefId::new_unchecked(def_id),
	module,
	macro_ev,
	);
	}
	}

As a result, in order to compile code like tests/ui/definition-reachable/auxiliary/field-method-macro.rs. we have to additionally traverse types of adt fields:

rust/compiler/rustc_privacy/src/lib.rs

Lines 628 to 638 in 29b7590

	DefKind::Struct | DefKind::Union => {
	// While structs and unions have type privacy, their fields do not.
	let struct_def = self.tcx.adt_def(def_id);
	for field in &struct_def.non_enum_variant().fields {
	let def_id = field.did.expect_local();
	let field_vis = self.tcx.local_visibility(def_id);
	if field_vis.is_accessible_from(module, self.tcx) {
	self.reach(def_id, macro_ev).ty();
	}
	}
	}

This is a hack and the proper solution would be to check definitions with EffectiveVisibilities::is_reachable. I haven’t done this yet, as it would start to trigger many lints as more items become reachable. I think it’s better to leave the change to another commit.

r? @petrochenkov

[petrochenkov]

This exists purely for bug-compatibility with the old algorithm, right?
Could you add a comment about this?

View changes since the review

[petrochenkov]

Suggested change

	for macro_reachable_adt in tcx.resolutions(()).macro_reachable_adts.iter() {
	for &(adt_def_id, adt_eff_vis) in &tcx.resolutions(()).macro_reachable_adts {

View changes since the review

[petrochenkov]

Suggested change

	.cloned()
	.copied()

View changes since the review

[petrochenkov]

This is a change compared to the previous version, module_children_local include imports.
I suspect that we can remove this condition and update imports by their DefIds here too.

View changes since the review

[petrochenkov]

The behavior is different from the previous version.

DefKind::Const { .. } | DefKind::Static { .. } | DefKind::TraitAlias | DefKind::TyAlias are not processed as before.
DefKind::Macro is also not processed recursively.

View changes since the review

[Bryanskiy]

For all def_kind there was update call in the beginning of update_macro_reachable_def:

rust/compiler/rustc_privacy/src/lib.rs

Line 589 in 2aabf3c

self.update(def_id, macro_ev, Level::Reachable);

so second update doesn't make sense at all.

[petrochenkov]

What happens if we reach this case several times with different macro_evs?
The last one will win, but that's not how the update operation worked before.
Can macro_reachable_adts be turned into a proper EffectiveVisibilities table instead?

View changes since the review

[Bryanskiy]

The last one will win, but that's not how the update operation worked before.

Why not? Last macro_ev is the max possible macro_ev because it follows update logic. For fields in rustc_privacy effective visibility is updated at once with max possible value.

[Bryanskiy]

@rustbot ready