Skip to content

Commit

Permalink
Arbitrary self types v2: no deshadowing on traits
Browse files Browse the repository at this point in the history
This is a hacky temporary "fix" for problems encountered where our
deshadowing algorithm is blocking legitimate code. This will need to be
reworked, but pushing to see if we get through CI.

The solution here involves only applying the deshadowing algorithm in
cases where both methods are inherent rather than in traits. That's
probably what we want to do? - but it needs discussion, and the code
needs a bit of restructuring.
  • Loading branch information
adetaylor committed Nov 20, 2024
1 parent 49acfb4 commit c0e8581
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 41 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
[candidate] => format!(
"the method of the same name on {} `{}`",
match candidate.kind {
probe::CandidateKind::InherentImplCandidate(_) => "the inherent impl for",
probe::CandidateKind::InherentImplCandidate(..) => "the inherent impl for",
_ => "trait",
},
self.tcx.def_path_str(candidate.item.container_id(self.tcx))
Expand Down
106 changes: 66 additions & 40 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,6 @@ pub(crate) struct ProbeContext<'a, 'tcx> {
/// machinery, since we don't particularly care about, for example, similarly named
/// candidates if we're *reporting* similarly named candidates.
is_suggestion: IsSuggestion,

/// Number of times we've hopped along the chain of `Receiver::Target`.
/// Used to spot cases where an "outer" method in a smart pointer might
/// "shadow" a pre-existing method in the pointee.
receiver_trait_derefs: usize,
}

impl<'a, 'tcx> Deref for ProbeContext<'a, 'tcx> {
Expand All @@ -104,12 +99,11 @@ pub(crate) struct Candidate<'tcx> {
pub(crate) item: ty::AssocItem,
pub(crate) kind: CandidateKind<'tcx>,
pub(crate) import_ids: SmallVec<[LocalDefId; 1]>,
receiver_trait_derefs: usize,
}

#[derive(Debug, Clone)]
pub(crate) enum CandidateKind<'tcx> {
InherentImplCandidate(DefId),
InherentImplCandidate(DefId, usize),
ObjectCandidate(ty::PolyTraitRef<'tcx>),
TraitCandidate(ty::PolyTraitRef<'tcx>),
WhereClauseCandidate(ty::PolyTraitRef<'tcx>),
Expand Down Expand Up @@ -183,7 +177,7 @@ struct PickDiagHints<'a, 'tcx> {
#[derive(Debug)]
struct PickConstraintsForShadowed {
autoderefs: usize,
receiver_trait_derefs: usize,
receiver_trait_derefs: Option<usize>,
def_id: DefId,
}

Expand All @@ -192,8 +186,17 @@ impl PickConstraintsForShadowed {
autoderefs == self.autoderefs
}

fn may_shadow_based_on_receiver_trait_derefs(&self, receiver_trait_derefs: usize) -> bool {
receiver_trait_derefs != self.receiver_trait_derefs
fn may_shadow_based_on_kind(&self, kind: &CandidateKind<'_>) -> bool {
if let Some(receiver_trait_derefs) = self.receiver_trait_derefs {
match kind {
CandidateKind::InherentImplCandidate(_, other_derefs) => {
*other_derefs != receiver_trait_derefs
}
_ => false,
}
} else {
false
}
}

fn may_shadow_based_on_defid(&self, def_id: DefId) -> bool {
Expand Down Expand Up @@ -223,7 +226,8 @@ pub(crate) struct Pick<'tcx> {

/// Number of jumps along the `Receiver::Target` chain we followed
/// to identify this method. Used only for deshadowing errors.
pub receiver_trait_derefs: usize,
/// Only applies for inherent impls.
pub receiver_trait_derefs: Option<usize>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -708,7 +712,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
static_candidates: RefCell::new(Vec::new()),
scope_expr_id,
is_suggestion,
receiver_trait_derefs: 0usize,
}
}

Expand Down Expand Up @@ -741,8 +744,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
import_ids: SmallVec<[LocalDefId; 1]>,
is_inherent: bool,
) {
let candidate =
Candidate { item, kind, import_ids, receiver_trait_derefs: self.receiver_trait_derefs };
let candidate = Candidate { item, kind, import_ids };
let is_accessible = if let Some(name) = self.method_name {
let item = candidate.item;
let hir_id = self.tcx.local_def_id_to_hir_id(self.body_id);
Expand All @@ -765,13 +767,16 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {

fn assemble_inherent_candidates(&mut self) {
for step in self.steps.iter() {
self.receiver_trait_derefs = step.autoderefs;
self.assemble_probe(&step.self_ty);
self.assemble_probe(&step.self_ty, step.autoderefs);
}
}

#[instrument(level = "debug", skip(self))]
fn assemble_probe(&mut self, self_ty: &Canonical<'tcx, QueryResponse<'tcx, Ty<'tcx>>>) {
fn assemble_probe(
&mut self,
self_ty: &Canonical<'tcx, QueryResponse<'tcx, Ty<'tcx>>>,
receiver_steps: usize,
) {
let raw_self_ty = self_ty.value.value;
match *raw_self_ty.kind() {
ty::Dynamic(data, ..) if let Some(p) = data.principal() => {
Expand All @@ -796,22 +801,31 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
self.fcx.instantiate_canonical(self.span, self_ty);

self.assemble_inherent_candidates_from_object(generalized_self_ty);
self.assemble_inherent_impl_candidates_for_type(p.def_id());
self.assemble_inherent_impl_candidates_for_type(p.def_id(), receiver_steps);
if self.tcx.has_attr(p.def_id(), sym::rustc_has_incoherent_inherent_impls) {
self.assemble_inherent_candidates_for_incoherent_ty(raw_self_ty);
self.assemble_inherent_candidates_for_incoherent_ty(
raw_self_ty,
receiver_steps,
);
}
}
ty::Adt(def, _) => {
let def_id = def.did();
self.assemble_inherent_impl_candidates_for_type(def_id);
self.assemble_inherent_impl_candidates_for_type(def_id, receiver_steps);
if self.tcx.has_attr(def_id, sym::rustc_has_incoherent_inherent_impls) {
self.assemble_inherent_candidates_for_incoherent_ty(raw_self_ty);
self.assemble_inherent_candidates_for_incoherent_ty(
raw_self_ty,
receiver_steps,
);
}
}
ty::Foreign(did) => {
self.assemble_inherent_impl_candidates_for_type(did);
self.assemble_inherent_impl_candidates_for_type(did, receiver_steps);
if self.tcx.has_attr(did, sym::rustc_has_incoherent_inherent_impls) {
self.assemble_inherent_candidates_for_incoherent_ty(raw_self_ty);
self.assemble_inherent_candidates_for_incoherent_ty(
raw_self_ty,
receiver_steps,
);
}
}
ty::Param(p) => {
Expand All @@ -828,29 +842,35 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
| ty::RawPtr(_, _)
| ty::Ref(..)
| ty::Never
| ty::Tuple(..) => self.assemble_inherent_candidates_for_incoherent_ty(raw_self_ty),
| ty::Tuple(..) => {
self.assemble_inherent_candidates_for_incoherent_ty(raw_self_ty, receiver_steps)
}
_ => {}
}
}

fn assemble_inherent_candidates_for_incoherent_ty(&mut self, self_ty: Ty<'tcx>) {
fn assemble_inherent_candidates_for_incoherent_ty(
&mut self,
self_ty: Ty<'tcx>,
receiver_steps: usize,
) {
let Some(simp) = simplify_type(self.tcx, self_ty, TreatParams::InstantiateWithInfer) else {
bug!("unexpected incoherent type: {:?}", self_ty)
};
for &impl_def_id in self.tcx.incoherent_impls(simp).into_iter() {
self.assemble_inherent_impl_probe(impl_def_id);
self.assemble_inherent_impl_probe(impl_def_id, receiver_steps);
}
}

fn assemble_inherent_impl_candidates_for_type(&mut self, def_id: DefId) {
fn assemble_inherent_impl_candidates_for_type(&mut self, def_id: DefId, receiver_steps: usize) {
let impl_def_ids = self.tcx.at(self.span).inherent_impls(def_id).into_iter();
for &impl_def_id in impl_def_ids {
self.assemble_inherent_impl_probe(impl_def_id);
self.assemble_inherent_impl_probe(impl_def_id, receiver_steps);
}
}

#[instrument(level = "debug", skip(self))]
fn assemble_inherent_impl_probe(&mut self, impl_def_id: DefId) {
fn assemble_inherent_impl_probe(&mut self, impl_def_id: DefId, receiver_steps: usize) {
if !self.impl_dups.insert(impl_def_id) {
return; // already visited
}
Expand All @@ -861,7 +881,12 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
self.record_static_candidate(CandidateSource::Impl(impl_def_id));
continue;
}
self.push_candidate(item, InherentImplCandidate(impl_def_id), smallvec![], true);
self.push_candidate(
item,
InherentImplCandidate(impl_def_id, receiver_steps),
smallvec![],
true,
);
}
}

Expand Down Expand Up @@ -1569,9 +1594,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
pick_constraints
.map(|pick_constraints| {
pick_constraints.may_shadow_based_on_defid(candidate.item.def_id)
&& pick_constraints.may_shadow_based_on_receiver_trait_derefs(
candidate.receiver_trait_derefs,
)
&& pick_constraints.may_shadow_based_on_kind(&candidate.kind)
})
.unwrap_or(true)
})
Expand Down Expand Up @@ -1724,7 +1747,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
/// so do not use to make a decision that may lead to a successful compilation.
fn candidate_source(&self, candidate: &Candidate<'tcx>, self_ty: Ty<'tcx>) -> CandidateSource {
match candidate.kind {
InherentImplCandidate(_) => {
InherentImplCandidate(..) => {
CandidateSource::Impl(candidate.item.container_id(self.tcx))
}
ObjectCandidate(_) | WhereClauseCandidate(_) => {
Expand Down Expand Up @@ -1787,7 +1810,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
let (mut xform_self_ty, mut xform_ret_ty);

match probe.kind {
InherentImplCandidate(impl_def_id) => {
InherentImplCandidate(impl_def_id, _) => {
let impl_args = self.fresh_args_for_item(self.span, impl_def_id);
let impl_ty = self.tcx.type_of(impl_def_id).instantiate(self.tcx, impl_args);
(xform_self_ty, xform_ret_ty) =
Expand Down Expand Up @@ -1956,7 +1979,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
// We don't normalize the other candidates for perf/backwards-compat reasons...
// but `self.return_type` is only set on the diagnostic-path, so we
// should be okay doing it here.
if !matches!(probe.kind, InherentImplCandidate(_)) {
if !matches!(probe.kind, InherentImplCandidate(..)) {
xform_ret_ty = ocx.normalize(&cause, self.param_env, xform_ret_ty);
}

Expand Down Expand Up @@ -2030,11 +2053,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
item: probes[0].0.item,
kind: TraitPick,
import_ids: probes[0].0.import_ids.clone(),
autoderefs: probes[0].0.receiver_trait_derefs,
autoderefs: 0,
autoref_or_ptr_adjustment: None,
self_ty,
unstable_candidates: vec![],
receiver_trait_derefs: 0,
receiver_trait_derefs: None,
})
}

Expand Down Expand Up @@ -2306,7 +2329,7 @@ impl<'tcx> Candidate<'tcx> {
Pick {
item: self.item,
kind: match self.kind {
InherentImplCandidate(_) => InherentImplPick,
InherentImplCandidate(..) => InherentImplPick,
ObjectCandidate(_) => ObjectPick,
TraitCandidate(_) => TraitPick,
WhereClauseCandidate(trait_ref) => {
Expand All @@ -2328,7 +2351,10 @@ impl<'tcx> Candidate<'tcx> {
autoref_or_ptr_adjustment: None,
self_ty,
unstable_candidates,
receiver_trait_derefs: self.receiver_trait_derefs,
receiver_trait_derefs: match self.kind {
InherentImplCandidate(_, receiver_steps) => Some(receiver_steps),
_ => None,
},
}
}
}

0 comments on commit c0e8581

Please sign in to comment.