diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs index e20c86d65af3f..8f051cf02d41c 100644 --- a/compiler/rustc_resolve/src/effective_visibilities.rs +++ b/compiler/rustc_resolve/src/effective_visibilities.rs @@ -125,26 +125,42 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> { fn set_bindings_effective_visibilities(&mut self, module_id: LocalDefId) { let module = self.r.expect_module(module_id.to_def_id()); for (_, name_resolution) in self.r.resolutions(module).borrow().iter() { - let Some(mut decl) = name_resolution.borrow().best_decl() else { + let Some(decl) = name_resolution.borrow().best_decl() else { continue; }; - // Set the given effective visibility level to `Level::Direct` and - // sets the rest of the `use` chain to `Level::Reexported` until - // we hit the actual exported item. - let priv_vis = |this: &Self, parent_id, decl| match parent_id { - ParentId::Def(_) => this.current_private_vis, - ParentId::Import(_) => this.r.private_vis_decl(decl), - }; - let mut parent_id = ParentId::Def(module_id); - while let DeclKind::Import { source_decl, .. } = decl.kind { - self.update_import(decl, parent_id, priv_vis(self, parent_id, decl)); - parent_id = ParentId::Import(decl); - decl = source_decl; - } - if let Some(def_id) = decl.res().opt_def_id().and_then(|id| id.as_local()) { - let priv_vis = priv_vis(self, parent_id, decl); - self.update_def(def_id, decl.vis().expect_local(), parent_id, priv_vis); + self.update_decl_chain(decl, ParentId::Def(module_id)); + } + } + + /// Update effective visibilities for the whole reexport chain of a declaration. + /// Set the given effective visibility level to `Level::Direct` and + /// sets the rest of the `use` chain to `Level::Reexported` until + /// we hit the actual exported item. + fn update_decl_chain(&mut self, mut decl: Decl<'ra>, mut parent_id: ParentId<'ra>) { + let priv_vis = |this: &Self, parent_id, decl| match parent_id { + ParentId::Def(_) => this.current_private_vis, + ParentId::Import(_) => this.r.private_vis_decl(decl), + }; + while let DeclKind::Import { source_decl, .. } = decl.kind { + self.update_import(decl, parent_id, priv_vis(self, parent_id, decl)); + if let Some(max_vis_decl) = decl.ambiguity_vis_max.get() { + // The name is exported with the visibility of the most visible declaration + // in its ambiguous glob set (see `DeclData::vis`), so everything on that + // declaration's reexport chain, including the final item, must get its + // effective visibility from that declaration as well. Otherwise the item + // would be considered unreachable by dead code analysis and metadata + // encoding despite being exported (see the regression test + // `ambiguous-import-visibility-globglob-mir.rs`). + // This also avoids the most visible import in an ambiguous glob set + // being reported as unused. + self.update_decl_chain(max_vis_decl, parent_id); } + parent_id = ParentId::Import(decl); + decl = source_decl; + } + if let Some(def_id) = decl.res().opt_def_id().and_then(|id| id.as_local()) { + let priv_vis = priv_vis(self, parent_id, decl); + self.update_def(def_id, decl.vis().expect_local(), parent_id, priv_vis); } } @@ -194,10 +210,6 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> { parent_id.level(), tcx, ); - if let Some(max_vis_decl) = decl.ambiguity_vis_max.get() { - // Avoid the most visible import in an ambiguous glob set being reported as unused. - self.update_import(max_vis_decl, parent_id, priv_vis); - } } fn update_def( diff --git a/tests/ui/imports/ambiguous-import-visibility-globglob-mir.rs b/tests/ui/imports/ambiguous-import-visibility-globglob-mir.rs new file mode 100644 index 0000000000000..6633be4ba6bb3 --- /dev/null +++ b/tests/ui/imports/ambiguous-import-visibility-globglob-mir.rs @@ -0,0 +1,21 @@ +// Regression test for the 1.96 -> 1.97 stable-to-stable regression: an item exported +// only through a public glob, and also glob-imported with restricted visibility through +// a private facade, lost its exported effective visibility. The defining crate then +// skipped encoding its optimized MIR (and warned dead_code) while name resolution still +// exported the item and it remained `cross_crate_inlinable`, so downstream crates failed +// with "missing optimized MIR". This test pins the missing-MIR half; the dead_code half +// is checked by the sibling test `ambiguous-import-visibility-globglob-reachable.rs` +// (via its `#![deny(dead_code)]`). + +//@ build-pass +//@ aux-build:ambiguous-import-visibility-globglob-mir.rs + +extern crate ambiguous_import_visibility_globglob_mir as dep; + +pub fn call_f() -> u32 { + dep::f() +} + +fn main() { + call_f(); +} diff --git a/tests/ui/imports/ambiguous-import-visibility-globglob-reachable.rs b/tests/ui/imports/ambiguous-import-visibility-globglob-reachable.rs new file mode 100644 index 0000000000000..1a533b1d4ee21 --- /dev/null +++ b/tests/ui/imports/ambiguous-import-visibility-globglob-reachable.rs @@ -0,0 +1,26 @@ +// Regression test for the 1.96 -> 1.97 stable-to-stable regression: an item exported +// only through a public glob, and also glob-imported with restricted visibility through +// a private facade, lost its exported effective visibility while name resolution still +// exported it. Downstream: spurious dead_code in this crate, "missing optimized MIR" in +// dependent crates (see ambiguous-import-visibility-globglob-mir.rs). The public glob +// declaration must drive the effective visibility of the whole reexport chain. + +#![feature(rustc_attrs)] +#![allow(internal_features)] +#![deny(dead_code)] + +mod inner { + #[rustc_effective_visibility] + pub fn f() {} //~ ERROR Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub +} + +mod facade { + #[allow(unused_imports)] + pub(crate) use super::inner::f; +} + +#[allow(unused_imports)] +use facade::*; +pub use inner::*; + +fn main() {} diff --git a/tests/ui/imports/ambiguous-import-visibility-globglob-reachable.stderr b/tests/ui/imports/ambiguous-import-visibility-globglob-reachable.stderr new file mode 100644 index 0000000000000..c27e6a2ac807b --- /dev/null +++ b/tests/ui/imports/ambiguous-import-visibility-globglob-reachable.stderr @@ -0,0 +1,8 @@ +error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub + --> $DIR/ambiguous-import-visibility-globglob-reachable.rs:14:5 + | +LL | pub fn f() {} + | ^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/imports/auxiliary/ambiguous-import-visibility-globglob-mir.rs b/tests/ui/imports/auxiliary/ambiguous-import-visibility-globglob-mir.rs new file mode 100644 index 0000000000000..a1e125bd55637 --- /dev/null +++ b/tests/ui/imports/auxiliary/ambiguous-import-visibility-globglob-mir.rs @@ -0,0 +1,19 @@ +// An item exported only through a public glob, while also glob-imported into the +// same module through a facade with restricted visibility. The restricted duplicate +// must not make `f` unreachable: its optimized MIR must still be encoded for +// downstream crates (it is `cross_crate_inlinable`). + +mod inner { + pub fn f() -> u32 { + 42 + } +} + +mod facade { + #[allow(unused_imports)] + pub(crate) use super::inner::f; +} + +#[allow(unused_imports)] +use facade::*; +pub use inner::*;