Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4555,6 +4555,11 @@ pub struct Upvar {
pub struct TraitCandidate {
pub def_id: DefId,
pub import_ids: SmallVec<[LocalDefId; 1]>,
// Indicates whether this trait candidate is ambiguously glob imported
// in it's scope. Related to the AMBIGUOUS_GLOB_IMPORTED_TRAIT lint.
// If this is set to true and the trait is used as a result of method lookup, this
// lint is thrown.
pub lint_ambiguous: bool,
}

#[derive(Copy, Clone, Debug, HashStable_Generic)]
Expand Down
27 changes: 25 additions & 2 deletions compiler/rustc_hir_typeck/src/method/confirm.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::fmt::Debug;
use std::ops::Deref;

use rustc_hir as hir;
Expand All @@ -12,7 +13,7 @@ use rustc_hir_analysis::hir_ty_lowering::{
use rustc_infer::infer::{
BoundRegionConversionTime, DefineOpaqueTypes, InferOk, RegionVariableOrigin,
};
use rustc_lint::builtin::SUPERTRAIT_ITEM_SHADOWING_USAGE;
use rustc_lint::builtin::{AMBIGUOUS_GLOB_IMPORTED_TRAIT, SUPERTRAIT_ITEM_SHADOWING_USAGE};
use rustc_middle::traits::ObligationCauseCode;
use rustc_middle::ty::adjustment::{
Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCoercion,
Expand Down Expand Up @@ -149,6 +150,9 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
// Lint when an item is shadowing a supertrait item.
self.lint_shadowed_supertrait_items(pick, segment);

// Lint when a trait is ambiguously imported
self.lint_ambiguously_glob_imported_trait(pick, segment);

// Add any trait/regions obligations specified on the method's type parameters.
// We won't add these if we encountered an illegal sized bound, so that we can use
// a custom error in that case.
Expand Down Expand Up @@ -322,7 +326,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
})
}

probe::TraitPick => {
probe::TraitPick(_) => {
let trait_def_id = pick.item.container_id(self.tcx);

// Make a trait reference `$0 : Trait<$1...$n>`
Expand Down Expand Up @@ -716,6 +720,25 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
);
}

fn lint_ambiguously_glob_imported_trait(
&self,
pick: &probe::Pick<'_>,
segment: &hir::PathSegment<'tcx>,
) {
if pick.kind != probe::PickKind::TraitPick(true) {
return;
}
let trait_name = self.tcx.item_name(pick.item.container_id(self.tcx));
let import_span = self.tcx.hir_span_if_local(pick.import_ids[0].to_def_id()).unwrap();

self.tcx.node_lint(AMBIGUOUS_GLOB_IMPORTED_TRAIT, segment.hir_id, |diag| {
diag.primary_message(format!("Use of ambiguously glob imported trait `{trait_name}`"))
.span(segment.ident.span)
.span_label(import_span, format!("`{trait_name}`imported ambiguously here"))
.help(format!("Import `{trait_name}` explicitly"));
});
}

fn upcast(
&mut self,
source_trait_ref: ty::PolyTraitRef<'tcx>,
Expand Down
46 changes: 34 additions & 12 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub(crate) struct Candidate<'tcx> {
pub(crate) enum CandidateKind<'tcx> {
InherentImplCandidate { impl_def_id: DefId, receiver_steps: usize },
ObjectCandidate(ty::PolyTraitRef<'tcx>),
TraitCandidate(ty::PolyTraitRef<'tcx>),
TraitCandidate(ty::PolyTraitRef<'tcx>, bool /* lint_ambiguous */),
WhereClauseCandidate(ty::PolyTraitRef<'tcx>),
}

Expand Down Expand Up @@ -235,7 +235,10 @@ pub(crate) struct Pick<'tcx> {
pub(crate) enum PickKind<'tcx> {
InherentImplPick,
ObjectPick,
TraitPick,
TraitPick(
// Is Ambiguously Imported
bool,
),
WhereClausePick(
// Trait
ty::PolyTraitRef<'tcx>,
Expand Down Expand Up @@ -560,7 +563,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
probe_cx.push_candidate(
Candidate {
item,
kind: CandidateKind::TraitCandidate(ty::Binder::dummy(trait_ref)),
kind: CandidateKind::TraitCandidate(
ty::Binder::dummy(trait_ref),
false,
),
import_ids: smallvec![],
},
false,
Expand Down Expand Up @@ -1018,6 +1024,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
self.assemble_extension_candidates_for_trait(
&trait_candidate.import_ids,
trait_did,
trait_candidate.lint_ambiguous,
);
}
}
Expand All @@ -1029,7 +1036,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
let mut duplicates = FxHashSet::default();
for trait_info in suggest::all_traits(self.tcx) {
if duplicates.insert(trait_info.def_id) {
self.assemble_extension_candidates_for_trait(&smallvec![], trait_info.def_id);
self.assemble_extension_candidates_for_trait(
&smallvec![],
trait_info.def_id,
false,
);
}
}
}
Expand All @@ -1055,6 +1066,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
&mut self,
import_ids: &SmallVec<[LocalDefId; 1]>,
trait_def_id: DefId,
lint_ambiguous: bool,
) {
let trait_args = self.fresh_args_for_item(self.span, trait_def_id);
let trait_ref = ty::TraitRef::new_from_args(self.tcx, trait_def_id, trait_args);
Expand All @@ -1076,7 +1088,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
Candidate {
item,
import_ids: import_ids.clone(),
kind: TraitCandidate(bound_trait_ref),
kind: TraitCandidate(bound_trait_ref, lint_ambiguous),
},
false,
);
Expand All @@ -1099,7 +1111,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
Candidate {
item,
import_ids: import_ids.clone(),
kind: TraitCandidate(ty::Binder::dummy(trait_ref)),
kind: TraitCandidate(ty::Binder::dummy(trait_ref), lint_ambiguous),
},
false,
);
Expand Down Expand Up @@ -1842,7 +1854,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
ObjectCandidate(_) | WhereClauseCandidate(_) => {
CandidateSource::Trait(candidate.item.container_id(self.tcx))
}
TraitCandidate(trait_ref) => self.probe(|_| {
TraitCandidate(trait_ref, _) => self.probe(|_| {
let trait_ref = self.instantiate_binder_with_fresh_vars(
self.span,
BoundRegionConversionTime::FnCall,
Expand Down Expand Up @@ -1872,7 +1884,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
fn candidate_source_from_pick(&self, pick: &Pick<'tcx>) -> CandidateSource {
match pick.kind {
InherentImplPick => CandidateSource::Impl(pick.item.container_id(self.tcx)),
ObjectPick | WhereClausePick(_) | TraitPick => {
ObjectPick | WhereClausePick(_) | TraitPick(_) => {
CandidateSource::Trait(pick.item.container_id(self.tcx))
}
}
Expand Down Expand Up @@ -1948,7 +1960,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
impl_bounds,
));
}
TraitCandidate(poly_trait_ref) => {
TraitCandidate(poly_trait_ref, _) => {
// Some trait methods are excluded for arrays before 2021.
// (`array.into_iter()` wants a slice iterator for compatibility.)
if let Some(method_name) = self.method_name {
Expand Down Expand Up @@ -2274,11 +2286,16 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
}
}

let lint_ambiguous = match probes[0].0.kind {
TraitCandidate(_, lint) => lint,
_ => false,
};

// FIXME: check the return type here somehow.
// If so, just use this trait and call it a day.
Some(Pick {
item: probes[0].0.item,
kind: TraitPick,
kind: TraitPick(lint_ambiguous),
import_ids: probes[0].0.import_ids.clone(),
autoderefs: 0,
autoref_or_ptr_adjustment: None,
Expand Down Expand Up @@ -2348,9 +2365,14 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
}
}

let lint_ambiguous = match probes[0].0.kind {
TraitCandidate(_, lint) => lint,
_ => false,
};

Some(Pick {
item: child_candidate.item,
kind: TraitPick,
kind: TraitPick(lint_ambiguous),
import_ids: child_candidate.import_ids.clone(),
autoderefs: 0,
autoref_or_ptr_adjustment: None,
Expand Down Expand Up @@ -2611,7 +2633,7 @@ impl<'tcx> Candidate<'tcx> {
kind: match self.kind {
InherentImplCandidate { .. } => InherentImplPick,
ObjectCandidate(_) => ObjectPick,
TraitCandidate(_) => TraitPick,
TraitCandidate(_, lint_ambiguous) => TraitPick(lint_ambiguous),
WhereClauseCandidate(trait_ref) => {
// Only trait derived from where-clauses should
// appear here, so they should not contain any
Expand Down
56 changes: 56 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ declare_lint_pass! {
AARCH64_SOFTFLOAT_NEON,
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
AMBIGUOUS_ASSOCIATED_ITEMS,
AMBIGUOUS_GLOB_IMPORTED_TRAIT,
AMBIGUOUS_GLOB_IMPORTS,
AMBIGUOUS_GLOB_REEXPORTS,
ARITHMETIC_OVERFLOW,
Expand Down Expand Up @@ -4437,6 +4438,61 @@ declare_lint! {
};
}

declare_lint! {
/// The `ambiguous_trait_glob_imports` lint reports uses of traits that are
/// imported ambiguously via glob imports. Previously, this was not enforced
/// due to a bug in rustc.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(ambiguous_trait_glob_imports)]
/// mod m1 {
/// pub trait Trait {
/// fn method1(&self) {}
/// }
/// impl Trait for u8 {}
/// }
/// mod m2 {
/// pub trait Trait {
/// fn method2(&self) {}
/// }
/// impl Trait for u8 {}
/// }
///
/// fn main() {
/// use m1::*;
/// use m2::*;
/// 0u8.method1();
/// 0u8.method2();
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// When multiple traits with the same name are brought into scope through glob imports,
/// one trait becomes the "primary" one while the others are shadowed. Methods from the
/// shadowed traits (e.g. `method2`) become inaccessible, while methods from the "primary"
/// trait (e.g. `method1`) still resolve. Ideally, none of the ambiguous traits would be in scope,
/// but we have to allow this for now because of backwards compatibility.
/// This lint reports uses of these "primary" traits that are ambiguous.
///
/// This is a [future-incompatible] lint to transition this to a
/// hard error in the future.
///
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub AMBIGUOUS_GLOB_IMPORTED_TRAIT,
Warn,
"detects usages of ambiguously glob imported traits",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseError,
reference: "issue #147992 <https://github.com/rust-lang/rust/issues/147992>",
report_in_deps: false,
};
}

declare_lint! {
/// The `refining_impl_trait_reachable` lint detects `impl Trait` return
/// types in method signatures that are refined by a publically reachable
Expand Down
33 changes: 27 additions & 6 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,8 +609,18 @@ struct ModuleData<'ra> {
globs: CmRefCell<Vec<Import<'ra>>>,

/// Used to memoize the traits in this module for faster searches through all traits in scope.
traits:
CmRefCell<Option<Box<[(Macros20NormalizedIdent, NameBinding<'ra>, Option<Module<'ra>>)]>>>,
traits: CmRefCell<
Option<
Box<
[(
Macros20NormalizedIdent,
NameBinding<'ra>,
Option<Module<'ra>>,
bool, /* lint_ambiguous*/
)],
>,
>,
>,

/// Span of the module itself. Used for error reporting.
span: Span,
Expand Down Expand Up @@ -707,7 +717,12 @@ impl<'ra> Module<'ra> {
return;
}
if let Res::Def(DefKind::Trait | DefKind::TraitAlias, def_id) = binding.res() {
collected_traits.push((name, binding, r.as_ref().get_module(def_id)))
collected_traits.push((
name,
binding,
r.as_ref().get_module(def_id),
binding.is_ambiguity_recursive(),
));
}
});
*traits = Some(collected_traits.into_boxed_slice());
Expand Down Expand Up @@ -1893,7 +1908,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if let Some(module) = current_trait {
if self.trait_may_have_item(Some(module), assoc_item) {
let def_id = module.def_id();
found_traits.push(TraitCandidate { def_id, import_ids: smallvec![] });
found_traits.push(TraitCandidate {
def_id,
import_ids: smallvec![],
lint_ambiguous: false,
});
}
}

Expand Down Expand Up @@ -1928,11 +1947,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
) {
module.ensure_traits(self);
let traits = module.traits.borrow();
for &(trait_name, trait_binding, trait_module) in traits.as_ref().unwrap().iter() {
for &(trait_name, trait_binding, trait_module, lint_ambiguous) in
traits.as_ref().unwrap().iter()
{
if self.trait_may_have_item(trait_module, assoc_item) {
let def_id = trait_binding.res().def_id();
let import_ids = self.find_transitive_imports(&trait_binding.kind, trait_name.0);
found_traits.push(TraitCandidate { def_id, import_ids });
found_traits.push(TraitCandidate { def_id, import_ids, lint_ambiguous });
}
}
}
Expand Down
Loading
Loading