Skip to content

Commit

Permalink
Auto merge of rust-lang#125782 - compiler-errors:supertrait-item-shad…
Browse files Browse the repository at this point in the history
…owing, r=<try>

Implement RFC 3624 `supertrait_item_shadowing` (v2)

Implements RFC 3624 and the associated lint in the RFC.

Implements:
* Shadowing algorithm
* Lint for call-site shadowing (allow by default, gated)
* Lint for definition-site shadowing (allow by default, gated)

Tracking:
- rust-lang#89151

cc `@Amanieu` and rust-lang/rfcs#3624 and rust-lang#89151
  • Loading branch information
bors committed Jan 25, 2025
2 parents 203e6c1 + 8409aaa commit 2f0cace
Show file tree
Hide file tree
Showing 36 changed files with 1,056 additions and 1 deletion.
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,8 @@ declare_features! (
(unstable, strict_provenance_lints, "1.61.0", Some(130351)),
/// Allows string patterns to dereference values to match them.
(unstable, string_deref_patterns, "1.67.0", Some(87121)),
/// Allows subtrait items to shadow supertrait items.
(unstable, supertrait_item_shadowing, "CURRENT_RUSTC_VERSION", Some(89151)),
/// Allows the use of `#[target_feature]` on safe functions.
(unstable, target_feature_11, "1.45.0", Some(69098)),
/// Allows using `#[thread_local]` on `static` items.
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,12 @@ hir_analysis_specialization_trait = implementing `rustc_specialization_trait` tr
hir_analysis_static_specialize = cannot specialize on `'static` lifetime
hir_analysis_supertrait_item_multiple_shadowee = items from several supertraits are shadowed: {$traits}
hir_analysis_supertrait_item_shadowee = item from `{$supertrait}` is shadowed by a subtrait item
hir_analysis_supertrait_item_shadowing = trait item `{$item}` from `{$subtrait}` shadows identically named item from supertrait
hir_analysis_tait_forward_compat = item constrains opaque type that is not in its signature
.note = this item must mention the opaque type in its signature in order to be able to register hidden types
Expand Down
43 changes: 43 additions & 0 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_hir::lang_items::LangItem;
use rustc_hir::{AmbigArg, ItemKind};
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt};
use rustc_lint_defs::builtin::SUPERTRAIT_ITEM_SHADOWING_DEFINITION;
use rustc_macros::LintDiagnostic;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::query::Providers;
Expand Down Expand Up @@ -377,7 +378,10 @@ fn check_trait_item<'tcx>(
hir::TraitItemKind::Type(_bounds, Some(ty)) => (None, ty.span),
_ => (None, trait_item.span),
};

check_dyn_incompatible_self_trait_by_name(tcx, trait_item);
check_item_shadowed_by_supertrait(tcx, def_id);

let mut res = check_associated_item(tcx, def_id, span, method_sig);

if matches!(trait_item.kind, hir::TraitItemKind::Fn(..)) {
Expand Down Expand Up @@ -892,6 +896,45 @@ fn check_dyn_incompatible_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitI
}
}

fn check_item_shadowed_by_supertrait<'tcx>(tcx: TyCtxt<'tcx>, trait_item_def_id: LocalDefId) {
let item_name = tcx.item_name(trait_item_def_id.to_def_id());
let trait_def_id = tcx.local_parent(trait_item_def_id);

let shadowed: Vec<_> = traits::supertrait_def_ids(tcx, trait_def_id.to_def_id())
.skip(1)
.flat_map(|supertrait_def_id| {
tcx.associated_items(supertrait_def_id).filter_by_name_unhygienic(item_name)
})
.collect();
if !shadowed.is_empty() {
let shadowee = if let [shadowed] = shadowed[..] {
errors::SupertraitItemShadowee::Labeled {
span: tcx.def_span(shadowed.def_id),
supertrait: tcx.item_name(shadowed.trait_container(tcx).unwrap()),
}
} else {
let (traits, spans): (Vec<_>, Vec<_>) = shadowed
.iter()
.map(|item| {
(tcx.item_name(item.trait_container(tcx).unwrap()), tcx.def_span(item.def_id))
})
.unzip();
errors::SupertraitItemShadowee::Several { traits: traits.into(), spans: spans.into() }
};

tcx.emit_node_span_lint(
SUPERTRAIT_ITEM_SHADOWING_DEFINITION,
tcx.local_def_id_to_hir_id(trait_item_def_id),
tcx.def_span(trait_item_def_id),
errors::SupertraitItemShadowing {
item: item_name,
subtrait: tcx.item_name(trait_def_id.to_def_id()),
shadowee,
},
);
}
}

fn check_impl_item<'tcx>(
tcx: TyCtxt<'tcx>,
impl_item: &'tcx hir::ImplItem<'tcx>,
Expand Down
28 changes: 27 additions & 1 deletion compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level, MultiSpan,
Applicability, Diag, DiagCtxtHandle, DiagSymbolList, Diagnostic, EmissionGuarantee, Level,
MultiSpan,
};
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::Ty;
Expand Down Expand Up @@ -1702,3 +1703,28 @@ pub(crate) struct RegisterTypeUnstable<'a> {
pub span: Span,
pub ty: Ty<'a>,
}

#[derive(LintDiagnostic)]
#[diag(hir_analysis_supertrait_item_shadowing)]
pub(crate) struct SupertraitItemShadowing {
pub item: Symbol,
pub subtrait: Symbol,
#[subdiagnostic]
pub shadowee: SupertraitItemShadowee,
}

#[derive(Subdiagnostic)]
pub(crate) enum SupertraitItemShadowee {
#[note(hir_analysis_supertrait_item_shadowee)]
Labeled {
#[primary_span]
span: Span,
supertrait: Symbol,
},
#[note(hir_analysis_supertrait_item_multiple_shadowee)]
Several {
#[primary_span]
spans: MultiSpan,
traits: DiagSymbolList,
},
}
8 changes: 8 additions & 0 deletions compiler/rustc_hir_typeck/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,14 @@ hir_typeck_suggest_boxing_when_appropriate = store this in the heap by calling `
hir_typeck_suggest_ptr_null_mut = consider using `core::ptr::null_mut` instead
hir_typeck_supertrait_item_multiple_shadowee = items from several supertraits are shadowed: {$traits}
hir_typeck_supertrait_item_shadowee = item from `{$supertrait}` is shadowed by a subtrait item
hir_typeck_supertrait_item_shadower = item from `{$subtrait}` shadows a supertrait item
hir_typeck_supertrait_item_shadowing = trait item `{$item}` from `{$subtrait}` shadows identically named item from supertrait
hir_typeck_trivial_cast = trivial {$numeric ->
[true] numeric cast
*[false] cast
Expand Down
35 changes: 35 additions & 0 deletions compiler/rustc_hir_typeck/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -866,3 +866,38 @@ pub(crate) struct ReplaceCommaWithSemicolon {
pub comma_span: Span,
pub descr: &'static str,
}

#[derive(LintDiagnostic)]
#[diag(hir_typeck_supertrait_item_shadowing)]
pub(crate) struct SupertraitItemShadowing {
pub item: Symbol,
pub subtrait: Symbol,
#[subdiagnostic]
pub shadower: SupertraitItemShadower,
#[subdiagnostic]
pub shadowee: SupertraitItemShadowee,
}

#[derive(Subdiagnostic)]
#[note(hir_typeck_supertrait_item_shadower)]
pub(crate) struct SupertraitItemShadower {
pub subtrait: Symbol,
#[primary_span]
pub span: Span,
}

#[derive(Subdiagnostic)]
pub(crate) enum SupertraitItemShadowee {
#[note(hir_typeck_supertrait_item_shadowee)]
Labeled {
#[primary_span]
span: Span,
supertrait: Symbol,
},
#[note(hir_typeck_supertrait_item_multiple_shadowee)]
Several {
#[primary_span]
spans: MultiSpan,
traits: DiagSymbolList,
},
}
43 changes: 43 additions & 0 deletions compiler/rustc_hir_typeck/src/method/confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_hir_analysis::hir_ty_lowering::{
FeedConstTy, GenericArgsLowerer, HirTyLowerer, IsMethodCall, RegionInferReason,
};
use rustc_infer::infer::{self, DefineOpaqueTypes, InferOk};
use rustc_lint::builtin::SUPERTRAIT_ITEM_SHADOWING_USAGE;
use rustc_middle::traits::{ObligationCauseCode, UnifyReceiverContext};
use rustc_middle::ty::adjustment::{
Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCoercion,
Expand All @@ -24,6 +25,7 @@ use rustc_trait_selection::traits;
use tracing::debug;

use super::{MethodCallee, probe};
use crate::errors::{SupertraitItemShadowee, SupertraitItemShadower, SupertraitItemShadowing};
use crate::{FnCtxt, callee};

struct ConfirmContext<'a, 'tcx> {
Expand Down Expand Up @@ -143,6 +145,8 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
// Make sure nobody calls `drop()` explicitly.
self.enforce_illegal_method_limitations(pick);

self.enforce_shadowed_supertrait_items(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 @@ -665,6 +669,45 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
}
}

fn enforce_shadowed_supertrait_items(
&self,
pick: &probe::Pick<'_>,
segment: &hir::PathSegment<'tcx>,
) {
if pick.shadowed_candidates.is_empty() {
return;
}

let shadower_span = self.tcx.def_span(pick.item.def_id);
let subtrait = self.tcx.item_name(pick.item.trait_container(self.tcx).unwrap());
let shadower = SupertraitItemShadower { span: shadower_span, subtrait };

let shadowee = if let [shadowee] = &pick.shadowed_candidates[..] {
let shadowee_span = self.tcx.def_span(shadowee.def_id);
let supertrait = self.tcx.item_name(shadowee.trait_container(self.tcx).unwrap());
SupertraitItemShadowee::Labeled { span: shadowee_span, supertrait }
} else {
let (traits, spans): (Vec<_>, Vec<_>) = pick
.shadowed_candidates
.iter()
.map(|item| {
(
self.tcx.item_name(item.trait_container(self.tcx).unwrap()),
self.tcx.def_span(item.def_id),
)
})
.unzip();
SupertraitItemShadowee::Several { traits: traits.into(), spans: spans.into() }
};

self.tcx.emit_node_span_lint(
SUPERTRAIT_ITEM_SHADOWING_USAGE,
segment.hir_id,
segment.ident.span,
SupertraitItemShadowing { shadower, shadowee, item: segment.ident.name, subtrait },
);
}

fn upcast(
&mut self,
source_trait_ref: ty::PolyTraitRef<'tcx>,
Expand Down
95 changes: 95 additions & 0 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::cmp::max;
use std::ops::Deref;

use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::sso::SsoHashSet;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::HirId;
Expand Down Expand Up @@ -33,6 +34,7 @@ use rustc_trait_selection::traits::query::method_autoderef::{
CandidateStep, MethodAutoderefBadTy, MethodAutoderefStepsResult,
};
use rustc_trait_selection::traits::{self, ObligationCause, ObligationCtxt};
use rustc_type_ir::elaborate::supertrait_def_ids;
use smallvec::{SmallVec, smallvec};
use tracing::{debug, instrument};

Expand Down Expand Up @@ -224,6 +226,9 @@ pub(crate) struct Pick<'tcx> {
/// to identify this method. Used only for deshadowing errors.
/// Only applies for inherent impls.
pub receiver_steps: Option<usize>,

/// Candidates that were shadowed by supertraits.
pub shadowed_candidates: Vec<ty::AssocItem>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -1634,6 +1639,17 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
}

if applicable_candidates.len() > 1 {
// We collapse to a subtrait pick *after* filtering unstable candidates
// to make sure we don't prefer a unstable subtrait method over a stable
// supertrait method.
if self.tcx.features().supertrait_item_shadowing() {
if let Some(pick) =
self.collapse_candidates_to_subtrait_pick(self_ty, &applicable_candidates)
{
return Some(Ok(pick));
}
}

let sources = candidates.iter().map(|p| self.candidate_source(p, self_ty)).collect();
return Some(Err(MethodError::Ambiguity(sources)));
}
Expand Down Expand Up @@ -1672,6 +1688,7 @@ impl<'tcx> Pick<'tcx> {
self_ty,
unstable_candidates: _,
receiver_steps: _,
shadowed_candidates: _,
} = *self;
self_ty != other.self_ty || def_id != other.item.def_id
}
Expand Down Expand Up @@ -2081,6 +2098,83 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
self_ty,
unstable_candidates: vec![],
receiver_steps: None,
shadowed_candidates: vec![],
})
}

/// Much like `collapse_candidates_to_trait_pick`, this method allows us to collapse
/// multiple conflicting picks if there is one pick whose trait container is a subtrait
/// of the trait containers of all of the other picks.
///
/// This implements RFC #3624.
fn collapse_candidates_to_subtrait_pick(
&self,
self_ty: Ty<'tcx>,
probes: &[(&Candidate<'tcx>, ProbeResult)],
) -> Option<Pick<'tcx>> {
let mut child_candidate = probes[0].0;
let mut child_trait = child_candidate.item.trait_container(self.tcx)?;
let mut supertraits: SsoHashSet<_> = supertrait_def_ids(self.tcx, child_trait).collect();

let mut remaining_candidates: Vec<_> = probes[1..].iter().map(|&(p, _)| p).collect();
while !remaining_candidates.is_empty() {
let mut made_progress = false;
let mut next_round = vec![];

for remaining_candidate in remaining_candidates {
let remaining_trait = remaining_candidate.item.trait_container(self.tcx)?;
if supertraits.contains(&remaining_trait) {
made_progress = true;
continue;
}

// This pick is not a supertrait of the `child_pick`.
// Check if it's a subtrait of the `child_pick`, instead.
// If it is, then it must have been a subtrait of every
// other pick we've eliminated at this point. It will
// take over at this point.
let remaining_trait_supertraits: SsoHashSet<_> =
supertrait_def_ids(self.tcx, remaining_trait).collect();
if remaining_trait_supertraits.contains(&child_trait) {
child_candidate = remaining_candidate;
child_trait = remaining_trait;
supertraits = remaining_trait_supertraits;
made_progress = true;
continue;
}

// `child_pick` is not a supertrait of this pick.
// Don't bail here, since we may be comparing two supertraits
// of a common subtrait. These two supertraits won't be related
// at all, but we will pick them up next round when we find their
// child as we continue iterating in this round.
next_round.push(remaining_candidate);
}

if made_progress {
// If we've made progress, iterate again.
remaining_candidates = next_round;
} else {
// Otherwise, we must have at least two candidates which
// are not related to each other at all.
return None;
}
}

Some(Pick {
item: child_candidate.item,
kind: TraitPick,
import_ids: child_candidate.import_ids.clone(),
autoderefs: 0,
autoref_or_ptr_adjustment: None,
self_ty,
unstable_candidates: vec![],
shadowed_candidates: probes
.iter()
.map(|(c, _)| c.item)
.filter(|item| item.def_id != child_candidate.item.def_id)
.collect(),
receiver_steps: None,
})
}

Expand Down Expand Up @@ -2378,6 +2472,7 @@ impl<'tcx> Candidate<'tcx> {
InherentImplCandidate { receiver_steps, .. } => Some(receiver_steps),
_ => None,
},
shadowed_candidates: vec![],
}
}
}
Loading

0 comments on commit 2f0cace

Please sign in to comment.