Skip to content

Commit

Permalink
resolve: re-export ambiguity as warning
Browse files Browse the repository at this point in the history
  • Loading branch information
bvanjoi committed Aug 10, 2023
1 parent 832db2f commit 626c193
Show file tree
Hide file tree
Showing 18 changed files with 207 additions and 32 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1869,6 +1869,9 @@ pub fn report_ambiguity_error<'a, G: EmissionGuarantee>(
for help_msg in ambiguity.b1_help_msgs {
db.help(help_msg);
}
if ambiguity.extern_crate {
return;
}
db.span_note(ambiguity.b2_span, ambiguity.b2_note_msg);
for help_msg in ambiguity.b2_help_msgs {
db.help(help_msg);
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,9 @@ impl<HCX> ToStableHashKey<HCX> for LintId {

#[derive(Debug)]
pub struct AmbiguityErrorDiag {
/// Is this a ambiguity binding come from
/// extern crate?
pub extern_crate: bool,
pub msg: String,
pub span: Span,
pub label_span: Span,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
let res = Res::Def(self.def_kind(id), self.local_def_id(id));
let vis = self.get_visibility(id);

ModChild { ident, res, vis, reexport_chain: Default::default() }
ModChild { ident, res, vis, reexport_chain: Default::default(), ambiguity: false }
}

/// Iterates over all named children of the given module,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,6 @@ pub struct ModChild {
/// Reexport chain linking this module child to its original reexported item.
/// Empty if the module child is a proper item.
pub reexport_chain: SmallVec<[Reexport; 2]>,
/// Is this an ambiguous binding?
pub ambiguity: bool,
}
56 changes: 49 additions & 7 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::def_collector::collect_definitions;
use crate::imports::{ImportData, ImportKind};
use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
use crate::{errors, BindingKey, MacroData, NameBindingData};
use crate::{errors, AmbiguityKind, BindingKey, MacroData, NameBindingData};
use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};
Expand Down Expand Up @@ -62,6 +62,46 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span,
}
}

impl<'a, Id: Into<DefId>> ToNameBinding<'a>
for (Module<'a>, ty::Visibility<Id>, Span, LocalExpnId, bool)
{
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> NameBinding<'a> {
let mut binding = NameBindingData {
kind: NameBindingKind::Module(self.0),
ambiguity: None,
warn_ambiguity: false,
vis: self.1.to_def_id(),
span: self.2,
expansion: self.3,
};
if self.4 {
binding.ambiguity =
Some((arenas.alloc_name_binding(binding.clone()), AmbiguityKind::ExternalCrate));
binding.warn_ambiguity = true;
}
arenas.alloc_name_binding(binding)
}
}

impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span, LocalExpnId, bool) {
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> NameBinding<'a> {
let mut binding = NameBindingData {
kind: NameBindingKind::Res(self.0),
ambiguity: None,
warn_ambiguity: false,
vis: self.1.to_def_id(),
span: self.2,
expansion: self.3,
};
if self.4 {
binding.ambiguity =
Some((arenas.alloc_name_binding(binding.clone()), AmbiguityKind::ExternalCrate));
binding.warn_ambiguity = true;
}
arenas.alloc_name_binding(binding)
}
}

impl<'a, 'tcx> Resolver<'a, 'tcx> {
/// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined;
/// otherwise, reports an error.
Expand All @@ -71,7 +111,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
{
let binding = def.to_name_binding(self.arenas);
let key = self.new_disambiguated_key(ident, ns);
if let Err(old_binding) = self.try_define(parent, key, binding, false) {
if let Err(old_binding) = self.try_define(parent, key, binding, binding.warn_ambiguity) {
self.report_conflict(parent, ident, ns, old_binding, binding);
}
}
Expand Down Expand Up @@ -932,7 +972,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
/// Builds the reduced graph for a single item in an external crate.
fn build_reduced_graph_for_external_crate_res(&mut self, child: &ModChild) {
let parent = self.parent_scope.module;
let ModChild { ident, res, vis, ref reexport_chain } = *child;
let ModChild { ident, res, vis, ref reexport_chain, ambiguity } = *child;
let span = self.r.def_span(
reexport_chain
.first()
Expand All @@ -945,7 +985,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
match res {
Res::Def(DefKind::Mod | DefKind::Enum | DefKind::Trait, def_id) => {
let module = self.r.expect_module(def_id);
self.r.define(parent, ident, TypeNS, (module, vis, span, expansion));
self.r.define(parent, ident, TypeNS, (module, vis, span, expansion, ambiguity));
}
Res::Def(
DefKind::Struct
Expand All @@ -959,7 +999,9 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
_,
)
| Res::PrimTy(..)
| Res::ToolMod => self.r.define(parent, ident, TypeNS, (res, vis, span, expansion)),
| Res::ToolMod => {
self.r.define(parent, ident, TypeNS, (res, vis, span, expansion, ambiguity))
}
Res::Def(
DefKind::Fn
| DefKind::AssocFn
Expand All @@ -968,9 +1010,9 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
| DefKind::AssocConst
| DefKind::Ctor(..),
_,
) => self.r.define(parent, ident, ValueNS, (res, vis, span, expansion)),
) => self.r.define(parent, ident, ValueNS, (res, vis, span, expansion, ambiguity)),
Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => {
self.r.define(parent, ident, MacroNS, (res, vis, span, expansion))
self.r.define(parent, ident, MacroNS, (res, vis, span, expansion, ambiguity))
}
Res::Def(
DefKind::TyParam
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
for ambiguity_error in &self.ambiguity_errors {
let diag = self.ambiguity_diagnostics(ambiguity_error);
if ambiguity_error.warning {
let NameBindingKind::Import { import, .. } = ambiguity_error.b1.0.kind else {
unreachable!()
};
let root_id =
if let NameBindingKind::Import { import, .. } = ambiguity_error.b1.0.kind {
import.root_id
} else if ambiguity_error.kind == AmbiguityKind::ExternalCrate {
krate.id
} else {
unreachable!()
};
self.lint_buffer.buffer_lint_with_diagnostic(
AMBIGUOUS_GLOB_IMPORTS,
import.root_id,
root_id,
ambiguity_error.ident.span,
diag.msg.to_string(),
BuiltinLintDiagnostics::AmbiguousGlobImports { diag },
Expand Down Expand Up @@ -1604,6 +1609,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let (b2_span, b2_note_msg, b2_help_msgs) = could_refer_to(b2, misc2, " also");

AmbiguityErrorDiag {
extern_crate: kind == AmbiguityKind::ExternalCrate,
msg: format!("`{ident}` is ambiguous"),
span: ident.span,
label_span: ident.span,
Expand Down
19 changes: 10 additions & 9 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1407,11 +1407,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
};
if self.is_accessible_from(binding.vis, scope) {
let imported_binding = self.import(binding, import);
let warn_ambiguity = self
.resolution(import.parent_scope.module, key)
.borrow()
.binding()
.is_some_and(|binding| binding.is_warn_ambiguity());
let warn_ambiguity = imported_binding.is_warn_ambiguity();
let _ = self.try_define(
import.parent_scope.module,
key,
Expand All @@ -1436,16 +1432,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {

module.for_each_child(self, |this, ident, _, binding| {
let res = binding.res().expect_non_local();
let error_ambiguity = binding.is_ambiguity() && !binding.warn_ambiguity;
if res != def::Res::Err && !error_ambiguity {
if res != def::Res::Err {
let mut reexport_chain = SmallVec::new();
let mut next_binding = binding;
while let NameBindingKind::Import { binding, import, .. } = next_binding.kind {
reexport_chain.push(import.simplify(this));
next_binding = binding;
}

children.push(ModChild { ident, res, vis: binding.vis, reexport_chain });
let ambiguity = binding.is_ambiguity();
children.push(ModChild {
ident,
res,
vis: binding.vis,
reexport_chain,
ambiguity,
});
}
});

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,7 @@ enum AmbiguityKind {
GlobVsGlob,
GlobVsExpanded,
MoreExpandedVsOuter,
ExternalCrate,
}

impl AmbiguityKind {
Expand All @@ -749,6 +750,7 @@ impl AmbiguityKind {
AmbiguityKind::MoreExpandedVsOuter => {
"a conflict between a macro-expanded name and a less macro-expanded name from outer scope during import or macro resolution"
}
AmbiguityKind::ExternalCrate => "it has some error in extern crate",
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/imports/ambiguous-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ extern crate ambiguous_1;

fn main() {
ambiguous_1::id();
//~^ WARNING `id` is ambiguous
//~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
}
18 changes: 18 additions & 0 deletions tests/ui/imports/ambiguous-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
warning: `id` is ambiguous
--> $DIR/ambiguous-2.rs:8:18
|
LL | ambiguous_1::id();
| ^^ ambiguous name
|
= 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 #114095 <https://github.com/rust-lang/rust/issues/114095>
= note: ambiguous because of it has some error in extern crate
note: `id` could refer to the function defined here
--> $DIR/auxiliary/../ambiguous-1.rs:24:9
|
LL | pub use openssl::*;
| ^^^^^^^
= note: `#[warn(ambiguous_glob_imports)]` on by default

warning: 1 warning emitted

3 changes: 2 additions & 1 deletion tests/ui/imports/ambiguous-4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ extern crate ambiguous_4_extern;

fn main() {
ambiguous_4_extern::id();
// `warning_ambiguous` had been lost at metadata.
//~^ WARNING `id` is ambiguous
//~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
}
18 changes: 18 additions & 0 deletions tests/ui/imports/ambiguous-4.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
warning: `id` is ambiguous
--> $DIR/ambiguous-4.rs:7:25
|
LL | ambiguous_4_extern::id();
| ^^ ambiguous name
|
= 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 #114095 <https://github.com/rust-lang/rust/issues/114095>
= note: ambiguous because of it has some error in extern crate
note: `id` could refer to the function defined here
--> $DIR/auxiliary/../ambiguous-4-extern.rs:10:9
|
LL | pub use evp::*;
| ^^^
= note: `#[warn(ambiguous_glob_imports)]` on by default

warning: 1 warning emitted

2 changes: 2 additions & 0 deletions tests/ui/imports/extern-with-ambiguous-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@ mod s {
use s::*;
use extern_with_ambiguous_2_extern::*;
use error::*;
//~^ WARNING `error` is ambiguous
//~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

fn main() {}
25 changes: 25 additions & 0 deletions tests/ui/imports/extern-with-ambiguous-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
warning: `error` is ambiguous
--> $DIR/extern-with-ambiguous-2.rs:14:5
|
LL | use error::*;
| ^^^^^ ambiguous name
|
= 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 #114095 <https://github.com/rust-lang/rust/issues/114095>
= note: ambiguous because of multiple glob imports of a name in the same module
note: `error` could refer to the module imported here
--> $DIR/extern-with-ambiguous-2.rs:12:5
|
LL | use s::*;
| ^^^^
= help: consider adding an explicit import of `error` to disambiguate
note: `error` could also refer to the module imported here
--> $DIR/extern-with-ambiguous-2.rs:13:5
|
LL | use extern_with_ambiguous_2_extern::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: consider adding an explicit import of `error` to disambiguate
= note: `#[warn(ambiguous_glob_imports)]` on by default

warning: 1 warning emitted

2 changes: 2 additions & 0 deletions tests/ui/imports/extern-with-ambiguous-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@ mod s {
use s::*;
use extern_with_ambiguous_3_extern::*;
use error::*;
//~^ WARNING `error` is ambiguous
//~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

fn main() {}
25 changes: 25 additions & 0 deletions tests/ui/imports/extern-with-ambiguous-3.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
warning: `error` is ambiguous
--> $DIR/extern-with-ambiguous-3.rs:15:5
|
LL | use error::*;
| ^^^^^ ambiguous name
|
= 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 #114095 <https://github.com/rust-lang/rust/issues/114095>
= note: ambiguous because of multiple glob imports of a name in the same module
note: `error` could refer to the module imported here
--> $DIR/extern-with-ambiguous-3.rs:13:5
|
LL | use s::*;
| ^^^^
= help: consider adding an explicit import of `error` to disambiguate
note: `error` could also refer to the module imported here
--> $DIR/extern-with-ambiguous-3.rs:14:5
|
LL | use extern_with_ambiguous_3_extern::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: consider adding an explicit import of `error` to disambiguate
= note: `#[warn(ambiguous_glob_imports)]` on by default

warning: 1 warning emitted

9 changes: 7 additions & 2 deletions tests/ui/imports/glob-conflict-cross-crate.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
// check-pass
// aux-build:glob-conflict.rs

extern crate glob_conflict;

fn main() {
glob_conflict::f(); //~ ERROR cannot find function `f` in crate `glob_conflict`
glob_conflict::glob::f(); //~ ERROR cannot find function `f` in module `glob_conflict::glob`
glob_conflict::f();
//~^ WARNING `f` is ambiguous
//~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
glob_conflict::glob::f();
//~^ WARNING `f` is ambiguous
//~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
}
Loading

0 comments on commit 626c193

Please sign in to comment.