Mirrors/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2026-05-08 01:28:18 +03:00
Code Issues Packages Projects Releases Wiki Activity

Rollup merge of #154149 - petrochenkov:globvisglob, r=mu001999

Browse Source 
resolve: Extend `ambiguous_import_visibilities` deprecation lint to glob-vs-glob ambiguities

Continuation of https://github.com/rust-lang/rust/pull/149596, implementation of this comment https://github.com/rust-lang/rust/pull/149596#issuecomment-3646405173 in particular.
FCP for the lint in general - https://github.com/rust-lang/rust/pull/149596#issuecomment-3620449013.
https://github.com/rust-lang/rust/pull/152498 is reverted as a part of the change, but fixes are applied to keep the tests added in that PR working.

To implement this we have to have to track the most and the least visible declarations in an ambiguous glob set.

Part of https://github.com/rust-lang/rust/issues/153961.
r? @yaahc maybe
This commit is contained in:
Jonathan Brouwer
2026-04-29 23:51:29 +02:00
committed by GitHub
parent d0440d3a6a d28ea81a98
commit 6efcbb090b
11 changed files with 256 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/rustc_middle/src/middle/privacy.rs
+15 -7
View File
@@ -8,6 +8,7 @@
use rustc_data_structures::fx::{FxIndexMap, IndexEntry};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_hir::def::DefKind;
use rustc_hir::{ItemKind, Node, UseKind};
use rustc_macros::HashStable;
use rustc_span::def_id::{CRATE_DEF_ID, LocalDefId};
@@ -185,13 +186,20 @@ pub fn check_invariants(&self, tcx: TyCtxt<'_>) {
if !is_impl && tcx.trait_impl_of_assoc(def_id.to_def_id()).is_none() {
let nominal_vis = tcx.visibility(def_id);
if ev.reachable.greater_than(nominal_vis, tcx) {
span_bug!(
span,
"{:?}: reachable {:?} > nominal {:?}",
def_id,
ev.reachable,
nominal_vis,
);
if let Node::Item(item) = tcx.hir_node_by_def_id(def_id)
&& let ItemKind::Use(_, UseKind::Glob) = item.kind
{
// Glob import visibilities can be increased by other
// more public glob imports in cases of ambiguity.
} else {
span_bug!(
span,
"{:?}: reachable {:?} > nominal {:?}",
def_id,
ev.reachable,
nominal_vis,
);
}
}
}
}
compiler/rustc_resolve/src/build_reduced_graph.rs
+3 -1
View File
@@ -93,7 +93,9 @@ fn define_extern(
ambiguity: CmCell::new(ambiguity),
// External ambiguities always report the `AMBIGUOUS_GLOB_IMPORTS` lint at the moment.
warn_ambiguity: CmCell::new(true),
vis: CmCell::new(vis),
initial_vis: vis,
ambiguity_vis_max: CmCell::new(None),
ambiguity_vis_min: CmCell::new(None),
span,
expansion,
parent_module: Some(parent.to_module()),
compiler/rustc_resolve/src/effective_visibilities.rs
+4
View File
@@ -182,6 +182,10 @@ fn update_import(&mut self, decl: Decl<'ra>, parent_id: ParentId<'ra>) {
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);
}
}
fn update_def(
compiler/rustc_resolve/src/ident.rs
+30
View File
@@ -490,6 +490,12 @@ fn resolve_ident_in_scope_set_inner<'r>(
}
Some(Finalize { import, .. }) => import,
};
this.get_mut().maybe_push_glob_vs_glob_vis_ambiguity(
ident,
orig_ident_span,
decl,
import,
);
if let Some(&(innermost_decl, _)) = innermost_results.first() {
// Found another solution, if the first one was "weak", report an error.
@@ -779,6 +785,30 @@ fn resolve_ident_in_scope<'r>(
ret.map_err(ControlFlow::Continue)
}
fn maybe_push_glob_vs_glob_vis_ambiguity(
&mut self,
ident: IdentKey,
orig_ident_span: Span,
decl: Decl<'ra>,
import: Option<ImportSummary>,
) {
let Some(import) = import else { return };
let vis1 = self.import_decl_vis(decl, import);
let vis2 = self.import_decl_vis_ext(decl, import, true);
if vis1 != vis2 {
self.ambiguity_errors.push(AmbiguityError {
kind: AmbiguityKind::GlobVsGlob,
ambig_vis: Some((vis1, vis2)),
ident: ident.orig(orig_ident_span),
b1: decl.ambiguity_vis_max.get().unwrap_or(decl),
b2: decl.ambiguity_vis_min.get().unwrap_or(decl),
scope1: Scope::ModuleGlobs(decl.parent_module.unwrap(), None),
scope2: Scope::ModuleGlobs(decl.parent_module.unwrap(), None),
warning: Some(AmbiguityWarning::GlobImport),
});
}
}
fn maybe_push_ambiguity(
&mut self,
ident: IdentKey,
compiler/rustc_resolve/src/imports.rs
+32 -12
View File
@@ -370,8 +370,17 @@ fn remove_same_import<'ra>(d1: Decl<'ra>, d2: Decl<'ra>) -> (Decl<'ra>, Decl<'ra
impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
pub(crate) fn import_decl_vis(&self, decl: Decl<'ra>, import: ImportSummary) -> Visibility {
self.import_decl_vis_ext(decl, import, false)
}
pub(crate) fn import_decl_vis_ext(
&self,
decl: Decl<'ra>,
import: ImportSummary,
min: bool,
) -> Visibility {
assert!(import.vis.is_accessible_from(import.nearest_parent_mod, self.tcx));
let decl_vis = decl.vis();
let decl_vis = if min { decl.min_vis() } else { decl.vis() };
if decl_vis.partial_cmp(import.vis, self.tcx) == Some(Ordering::Less)
&& decl_vis.is_accessible_from(import.nearest_parent_mod, self.tcx)
&& pub_use_of_private_extern_crate_hack(import, decl).is_none()
@@ -409,7 +418,9 @@ pub(crate) fn new_import_decl(&self, decl: Decl<'ra>, import: Import<'ra>) -> De
ambiguity: CmCell::new(None),
warn_ambiguity: CmCell::new(false),
span: import.span,
vis: CmCell::new(vis.to_def_id()),
initial_vis: vis.to_def_id(),
ambiguity_vis_max: CmCell::new(None),
ambiguity_vis_min: CmCell::new(None),
expansion: import.parent_scope.expansion,
parent_module: Some(import.parent_scope.module),
})
@@ -434,8 +445,6 @@ fn select_glob_decl(
// - A glob decl is overwritten by its clone after setting ambiguity in it.
// FIXME: avoid this by removing `warn_ambiguity`, or by triggering glob re-fetch
// with the same decl in some way.
// - A glob decl is overwritten by a glob decl with larger visibility.
// FIXME: avoid this by updating this visibility in place.
// - A glob decl is overwritten by a glob decl re-fetching an
// overwritten decl from other module (the recursive case).
// Here we are detecting all such re-fetches and overwrite old decls
@@ -449,8 +458,7 @@ fn select_glob_decl(
// FIXME: reenable the asserts when `warn_ambiguity` is removed (#149195).
// assert_ne!(old_deep_decl, deep_decl);
// assert!(old_deep_decl.is_glob_import());
// FIXME: reenable the assert when visibility is updated in place.
// assert!(!deep_decl.is_glob_import());
assert!(!deep_decl.is_glob_import());
if old_glob_decl.ambiguity.get().is_some() && glob_decl.ambiguity.get().is_none() {
// Do not lose glob ambiguities when re-fetching the glob.
glob_decl.ambiguity.set_unchecked(old_glob_decl.ambiguity.get());
@@ -470,12 +478,21 @@ fn select_glob_decl(
// FIXME: remove this when `warn_ambiguity` is removed (#149195).
self.arenas.alloc_decl((*old_glob_decl).clone())
}
} else if glob_decl.vis().greater_than(old_glob_decl.vis(), self.tcx) {
// We are glob-importing the same item but with greater visibility.
} else if let old_vis = old_glob_decl.vis()
&& let vis = glob_decl.vis()
&& old_vis != vis
{
// We are glob-importing the same item but with a different visibility.
// All visibilities here are ordered because all of them are ancestors of `module`.
// FIXME: Update visibility in place, but without regressions
// (#152004, #151124, #152347).
glob_decl
if vis.greater_than(old_vis, self.tcx) {
old_glob_decl.ambiguity_vis_max.set_unchecked(Some(glob_decl));
} else if let old_min_vis = old_glob_decl.min_vis()
&& old_min_vis != vis
&& old_min_vis.greater_than(vis, self.tcx)
{
old_glob_decl.ambiguity_vis_min.set_unchecked(Some(glob_decl));
}
old_glob_decl
} else if glob_decl.is_ambiguity_recursive() && !old_glob_decl.is_ambiguity_recursive() {
// Overwriting a non-ambiguous glob import with an ambiguous glob import.
old_glob_decl.ambiguity.set_unchecked(Some(glob_decl));
@@ -498,6 +515,8 @@ pub(crate) fn try_plant_decl_into_local_module(
) -> Result<(), Decl<'ra>> {
assert!(!decl.warn_ambiguity.get());
assert!(decl.ambiguity.get().is_none());
assert!(decl.ambiguity_vis_max.get().is_none());
assert!(decl.ambiguity_vis_min.get().is_none());
let module = decl.parent_module.unwrap().expect_local();
assert!(self.is_accessible_from(decl.vis(), module.to_module()));
let res = decl.res();
@@ -556,11 +575,12 @@ fn update_local_resolution<T, F>(
.resolution_or_default(module.to_module(), key, orig_ident_span)
.borrow_mut_unchecked();
let old_decl = resolution.determined_decl();
let old_vis = old_decl.map(|d| d.vis());
let t = f(self, resolution);
if let Some(binding) = resolution.determined_decl()
&& old_decl != Some(binding)
&& (old_decl != Some(binding) || old_vis != Some(binding.vis()))
{
(binding, t, warn_ambiguity || old_decl.is_some())
} else {
compiler/rustc_resolve/src/lib.rs
+17 -3
View File
@@ -929,7 +929,13 @@ struct DeclData<'ra> {
warn_ambiguity: CmCell<bool>,
expansion: LocalExpnId,
span: Span,
vis: CmCell<Visibility<DefId>>,
initial_vis: Visibility<DefId>,
/// If the declaration refers to an ambiguous glob set, then this is the most visible
/// declaration from the set, if its visibility is different from `initial_vis`.
ambiguity_vis_max: CmCell<Option<Decl<'ra>>>,
/// If the declaration refers to an ambiguous glob set, then this is the least visible
/// declaration from the set, if its visibility is different from `initial_vis`.
ambiguity_vis_min: CmCell<Option<Decl<'ra>>>,
parent_module: Option<Module<'ra>>,
}
@@ -1049,7 +1055,13 @@ struct AmbiguityError<'ra> {
impl<'ra> DeclData<'ra> {
fn vis(&self) -> Visibility<DefId> {
self.vis.get()
// Select the maximum visibility if there are multiple ambiguous glob imports.
self.ambiguity_vis_max.get().map(|d| d.vis()).unwrap_or_else(|| self.initial_vis)
}
fn min_vis(&self) -> Visibility<DefId> {
// Select the minimum visibility if there are multiple ambiguous glob imports.
self.ambiguity_vis_min.get().map(|d| d.vis()).unwrap_or_else(|| self.initial_vis)
}
fn res(&self) -> Res {
@@ -1505,7 +1517,9 @@ fn new_def_decl(
kind: DeclKind::Def(res),
ambiguity: CmCell::new(None),
warn_ambiguity: CmCell::new(false),
vis: CmCell::new(vis),
initial_vis: vis,
ambiguity_vis_max: CmCell::new(None),
ambiguity_vis_min: CmCell::new(None),
span,
expansion,
parent_module,
tests/ui/imports/ambiguous-import-visibility-globglob-priv.stderr
+6 -6
View File
@@ -5,10 +5,10 @@ LL | use crate::both::private::S;
| ^ private struct import
|
note: the struct import `S` is defined here...
--> $DIR/ambiguous-import-visibility-globglob-priv.rs:8:24
--> $DIR/ambiguous-import-visibility-globglob-priv.rs:7:13
|
LL | pub(super) use crate::m::*;
| ^^^^^^^^^^^
LL | use crate::m::*;
| ^^^^^^^^^^^
note: ...and refers to the struct `S` which is defined here
--> $DIR/ambiguous-import-visibility-globglob-priv.rs:2:5
|
@@ -27,10 +27,10 @@ LL | use crate::both::private::S;
| ^ private struct import
|
note: the struct import `S` is defined here...
--> $DIR/ambiguous-import-visibility-globglob-priv.rs:8:24
--> $DIR/ambiguous-import-visibility-globglob-priv.rs:7:13
|
LL | pub(super) use crate::m::*;
| ^^^^^^^^^^^
LL | use crate::m::*;
| ^^^^^^^^^^^
note: ...and refers to the struct `S` which is defined here
--> $DIR/ambiguous-import-visibility-globglob-priv.rs:2:5
|
tests/ui/imports/ambiguous-import-visibility-globglob.rs
+12 -2
View File
@@ -1,7 +1,5 @@
//@ check-pass
// FIXME: should report "ambiguous import visibility" in all the cases below.
mod m {
pub struct S {}
}
@@ -12,7 +10,11 @@ mod min_vis_first {
pub use crate::m::*;
pub use self::S as S1;
//~^ WARN ambiguous import visibility
//~| WARN this was previously accepted
pub(crate) use self::S as S2;
//~^ WARN ambiguous import visibility
//~| WARN this was previously accepted
use self::S as S3; // OK
}
@@ -22,7 +24,11 @@ mod mid_vis_first {
pub use crate::m::*;
pub use self::S as S1;
//~^ WARN ambiguous import visibility
//~| WARN this was previously accepted
pub(crate) use self::S as S2;
//~^ WARN ambiguous import visibility
//~| WARN this was previously accepted
use self::S as S3; // OK
}
@@ -32,7 +38,11 @@ mod max_vis_first {
pub(crate) use crate::m::*;
pub use self::S as S1;
//~^ WARN ambiguous import visibility
//~| WARN this was previously accepted
pub(crate) use self::S as S2;
//~^ WARN ambiguous import visibility
//~| WARN this was previously accepted
use self::S as S3; // OK
}
tests/ui/imports/ambiguous-import-visibility-globglob.stderr
+135
View File
@@ -0,0 +1,135 @@
warning: ambiguous import visibility: pub or pub(in crate::min_vis_first)
--> $DIR/ambiguous-import-visibility-globglob.rs:12:19
|
LL | pub use self::S as S1;
| ^
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `S` could refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:10:13
|
LL | pub use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
note: `S` could also refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:8:9
|
LL | use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
= note: `#[warn(ambiguous_import_visibilities)]` (part of `#[warn(future_incompatible)]`) on by default
warning: ambiguous import visibility: pub(crate) or pub(in crate::min_vis_first)
--> $DIR/ambiguous-import-visibility-globglob.rs:15:26
|
LL | pub(crate) use self::S as S2;
| ^
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `S` could refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:10:13
|
LL | pub use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
note: `S` could also refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:8:9
|
LL | use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
warning: ambiguous import visibility: pub or pub(in crate::mid_vis_first)
--> $DIR/ambiguous-import-visibility-globglob.rs:26:19
|
LL | pub use self::S as S1;
| ^
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `S` could refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:24:13
|
LL | pub use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
note: `S` could also refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:23:9
|
LL | use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
warning: ambiguous import visibility: pub(crate) or pub(in crate::mid_vis_first)
--> $DIR/ambiguous-import-visibility-globglob.rs:29:26
|
LL | pub(crate) use self::S as S2;
| ^
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `S` could refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:24:13
|
LL | pub use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
note: `S` could also refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:23:9
|
LL | use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
warning: ambiguous import visibility: pub or pub(in crate::max_vis_first)
--> $DIR/ambiguous-import-visibility-globglob.rs:40:19
|
LL | pub use self::S as S1;
| ^
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `S` could refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:36:13
|
LL | pub use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
note: `S` could also refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:37:9
|
LL | use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
warning: ambiguous import visibility: pub(crate) or pub(in crate::max_vis_first)
--> $DIR/ambiguous-import-visibility-globglob.rs:43:26
|
LL | pub(crate) use self::S as S2;
| ^
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `S` could refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:36:13
|
LL | pub use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
note: `S` could also refer to the struct imported here
--> $DIR/ambiguous-import-visibility-globglob.rs:37:9
|
LL | use crate::m::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `S` to disambiguate
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
warning: 6 warnings emitted
tests/ui/imports/overwrite-vis-unused.rs
+2 -2
View File
@@ -1,12 +1,12 @@
// Regression test for issues #152004 and #151124.
//@ check-pass
#![deny(unused)]
mod m {
pub struct S {}
}
use m::*; //~ ERROR unused import: `m::*`
use m::*;
pub use m::*;
fn main() {}
tests/ui/imports/overwrite-vis-unused.stderr
-15
View File
@@ -1,15 +0,0 @@
error: unused import: `m::*`
--> $DIR/overwrite-vis-unused.rs:9:5
|
LL | use m::*;
| ^^^^
|
note: the lint level is defined here
--> $DIR/overwrite-vis-unused.rs:3:9
|
LL | #![deny(unused)]
| ^^^^^^
= note: `#[deny(unused_imports)]` implied by `#[deny(unused)]`
error: aborting due to 1 previous error