Arbitrary self types v2: deshadow quicker

A previous commit added a search for certain types of "shadowing"
situation where one method (in an outer smart pointer type, typically)
might hide or shadow the method in the pointee.

Performance investigation showed that the naïve approach is too slow -
this commit speeds it up, while being functionally the same.

This still does not actually cause the deshadowing check to emit any
errors; that comes in a subsequent commit which is where all the tests
live.

Author: adetaylor

compiler/rustc_hir_typeck/src/method/probe.rs

@@ -85,6 +85,11 @@ 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> {
@@ -99,6 +104,7 @@ 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)]
@@ -171,6 +177,30 @@ struct PickDiagHints<'a, 'tcx> {
     )>,
 }
 
+/// Criteria to apply when searching for a given Pick. This is used during
+/// the search for potentially shadowed methods to ensure we don't search
+/// more candidates than strictly necessary.
+#[derive(Debug)]
+struct PickConstraintsForShadowed {
+    autoderefs: usize,
+    receiver_trait_derefs: usize,
+    def_id: DefId,
+}
+
+impl PickConstraintsForShadowed {
+    fn may_shadow_based_on_autoderefs(&self, autoderefs: usize) -> bool {
+        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_defid(&self, def_id: DefId) -> bool {
+        def_id != self.def_id
+    }
+}
+
 #[derive(Debug, Clone)]
 pub(crate) struct Pick<'tcx> {
     pub item: ty::AssocItem,
@@ -190,6 +220,10 @@ pub(crate) struct Pick<'tcx> {
 
     /// Unstable candidates alongside the stable ones.
     unstable_candidates: Vec<(Candidate<'tcx>, Symbol)>,
+
+    /// Number of jumps along the `Receiver::Target` chain we followed
+    /// to identify this method. Used only for deshadowing errors.
+    pub receiver_trait_derefs: usize,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -674,6 +708,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             static_candidates: RefCell::new(Vec::new()),
             scope_expr_id,
             is_suggestion,
+            receiver_trait_derefs: 0usize,
         }
     }
 
@@ -706,7 +741,8 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         import_ids: SmallVec<[LocalDefId; 1]>,
         is_inherent: bool,
     ) {
-        let candidate = Candidate { item, kind, import_ids };
+        let candidate =
+            Candidate { item, kind, import_ids, receiver_trait_derefs: self.receiver_trait_derefs };
         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);
@@ -729,6 +765,7 @@ 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);
         }
     }
@@ -1196,8 +1233,13 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                     return Some(by_value_pick);
                 }
 
-                let autoref_pick =
-                    self.pick_autorefd_method(step, self_ty, hir::Mutability::Not, pick_diag_hints);
+                let autoref_pick = self.pick_autorefd_method(
+                    step,
+                    self_ty,
+                    hir::Mutability::Not,
+                    pick_diag_hints,
+                    None,
+                );
                 // Check for shadowing of a by-mut-ref method by a by-reference method (see comments on check_for_shadowing)
                 if let Some(autoref_pick) = autoref_pick {
                     if let Ok(autoref_pick) = autoref_pick.as_ref() {
@@ -1238,9 +1280,15 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                 // in the pointee. In practice, methods taking raw pointers
                 // are rare, and it seems that it should be easily possible
                 // to avoid such compatibility breaks.
-                self.pick_autorefd_method(step, self_ty, hir::Mutability::Mut, pick_diag_hints)
-                    .or_else(|| self.pick_const_ptr_method(step, self_ty, pick_diag_hints))
-                    .or_else(|| self.pick_reborrow_pin_method(step, self_ty, pick_diag_hints))
+                self.pick_autorefd_method(
+                    step,
+                    self_ty,
+                    hir::Mutability::Mut,
+                    pick_diag_hints,
+                    None,
+                )
+                .or_else(|| self.pick_const_ptr_method(step, self_ty, pick_diag_hints))
+                .or_else(|| self.pick_reborrow_pin_method(step, self_ty, pick_diag_hints))
             })
     }
 
@@ -1261,7 +1309,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
     /// Report an error in this case.
     fn check_for_shadowed_autorefd_method(
         &self,
-        _possible_shadower: &Pick<'tcx>,
+        possible_shadower: &Pick<'tcx>,
         step: &CandidateStep<'tcx>,
         self_ty: Ty<'tcx>,
         mutbl: hir::Mutability,
@@ -1275,8 +1323,54 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             unstable_candidates: if track_unstable_candidates { Some(Vec::new()) } else { None },
             unsatisfied_predicates: &mut Vec::new(),
         };
-        let _potentially_shadowed_pick =
-            self.pick_autorefd_method(step, self_ty, mutbl, &mut pick_diag_hints);
+        // Set criteria for how we find methods possibly shadowed by 'possible_shadower'
+        let pick_constraints = PickConstraintsForShadowed {
+            // It's the same `self` type...
+            autoderefs: possible_shadower.autoderefs,
+            // ... but the method was found in an impl block determined
+            // by searching further along the Receiver chain than the other,
+            // showing that it's a smart pointer type causing the problem...
+            receiver_trait_derefs: possible_shadower.receiver_trait_derefs,
+            // ... and they don't end up pointing to the same item in the
+            // first place (could happen with things like blanket impls for T)
+            def_id: possible_shadower.item.def_id,
+        };
+        // A note on the autoderefs above. Within pick_by_value_method, an extra
+        // autoderef may be applied in order to reborrow a reference with
+        // a different lifetime. That seems as though it would break the
+        // logic of these constraints, since the number of autoderefs could
+        // no longer be used to identify the fundamental type of the receiver.
+        // However, this extra autoderef is applied only to by-value calls
+        // where the receiver is already a reference. So this situation would
+        // only occur in cases where the shadowing looks like this:
+        // ```
+        // struct A;
+        // impl A {
+        //   fn foo(self: &&NonNull<A>) {}
+        //      // note this is by DOUBLE reference
+        // }
+        // ```
+        // then we've come along and added this method to `NonNull`:
+        // ```
+        //   fn foo(&self)  // note this is by single reference
+        // ```
+        // and the call is:
+        // ```
+        // let bar = NonNull<Foo>;
+        // let bar = &foo;
+        // bar.foo();
+        // ```
+        // In these circumstances, the logic is wrong, and we wouldn't spot
+        // the shadowing, because the autoderef-based maths wouldn't line up.
+        // This is a niche case and we can live without generating an error
+        // in the case of such shadowing.
+        let _potentially_shadowed_pick = self.pick_autorefd_method(
+            step,
+            self_ty,
+            mutbl,
+            &mut pick_diag_hints,
+            Some(&pick_constraints),
+        );
 
         // At the moment, this function does no checks. A future
         // commit will fill out the body here.
@@ -1299,7 +1393,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             return None;
         }
 
-        self.pick_method(self_ty, pick_diag_hints).map(|r| {
+        self.pick_method(self_ty, pick_diag_hints, None).map(|r| {
             r.map(|mut pick| {
                 pick.autoderefs = step.autoderefs;
 
@@ -1338,14 +1432,21 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         self_ty: Ty<'tcx>,
         mutbl: hir::Mutability,
         pick_diag_hints: &mut PickDiagHints<'b, 'tcx>,
+        pick_constraints: Option<&PickConstraintsForShadowed>,
     ) -> Option<PickResult<'tcx>> {
         let tcx = self.tcx;
 
+        if let Some(pick_constraints) = pick_constraints {
+            if !pick_constraints.may_shadow_based_on_autoderefs(step.autoderefs) {
+                return None;
+            }
+        }
+
         // In general, during probing we erase regions.
         let region = tcx.lifetimes.re_erased;
 
         let autoref_ty = Ty::new_ref(tcx, region, self_ty, mutbl);
-        self.pick_method(autoref_ty, pick_diag_hints).map(|r| {
+        self.pick_method(autoref_ty, pick_diag_hints, pick_constraints).map(|r| {
             r.map(|mut pick| {
                 pick.autoderefs = step.autoderefs;
                 pick.autoref_or_ptr_adjustment =
@@ -1382,7 +1483,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
 
         let region = self.tcx.lifetimes.re_erased;
         let autopin_ty = Ty::new_pinned_ref(self.tcx, region, inner_ty, hir::Mutability::Not);
-        self.pick_method(autopin_ty, pick_diag_hints).map(|r| {
+        self.pick_method(autopin_ty, pick_diag_hints, None).map(|r| {
             r.map(|mut pick| {
                 pick.autoderefs = step.autoderefs;
                 pick.autoref_or_ptr_adjustment =
@@ -1411,7 +1512,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         };
 
         let const_ptr_ty = Ty::new_imm_ptr(self.tcx, ty);
-        self.pick_method(const_ptr_ty, pick_diag_hints).map(|r| {
+        self.pick_method(const_ptr_ty, pick_diag_hints, None).map(|r| {
             r.map(|mut pick| {
                 pick.autoderefs = step.autoderefs;
                 pick.autoref_or_ptr_adjustment = Some(AutorefOrPtrAdjustment::ToConstPtr);
@@ -1424,22 +1525,24 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         &self,
         self_ty: Ty<'tcx>,
         pick_diag_hints: &mut PickDiagHints<'b, 'tcx>,
+        pick_constraints: Option<&PickConstraintsForShadowed>,
     ) -> Option<PickResult<'tcx>> {
         debug!("pick_method(self_ty={})", self.ty_to_string(self_ty));
 
         for (kind, candidates) in
             [("inherent", &self.inherent_candidates), ("extension", &self.extension_candidates)]
         {
             debug!("searching {} candidates", kind);
-            let res = self.consider_candidates(self_ty, candidates, pick_diag_hints);
+            let res =
+                self.consider_candidates(self_ty, candidates, pick_diag_hints, pick_constraints);
             if let Some(pick) = res {
                 return Some(pick);
             }
         }
 
         if self.private_candidate.get().is_none() {
             if let Some(Ok(pick)) =
-                self.consider_candidates(self_ty, &self.private_candidates, pick_diag_hints)
+                self.consider_candidates(self_ty, &self.private_candidates, pick_diag_hints, None)
             {
                 self.private_candidate.set(Some((pick.item.kind.as_def_kind(), pick.item.def_id)));
             }
@@ -1452,9 +1555,20 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         self_ty: Ty<'tcx>,
         candidates: &[Candidate<'tcx>],
         pick_diag_hints: &mut PickDiagHints<'b, 'tcx>,
+        pick_constraints: Option<&PickConstraintsForShadowed>,
     ) -> Option<PickResult<'tcx>> {
         let mut applicable_candidates: Vec<_> = candidates
             .iter()
+            .filter(|candidate| {
+                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,
+                            )
+                    })
+                    .unwrap_or(true)
+            })
             .map(|probe| {
                 (
                     probe,
@@ -1528,6 +1642,7 @@ impl<'tcx> Pick<'tcx> {
             autoref_or_ptr_adjustment: _,
             self_ty,
             unstable_candidates: _,
+            receiver_trait_derefs: _,
         } = *self;
         self_ty != other.self_ty || def_id != other.item.def_id
     }
@@ -1900,10 +2015,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             item: probes[0].0.item,
             kind: TraitPick,
             import_ids: probes[0].0.import_ids.clone(),
-            autoderefs: 0,
+            autoderefs: probes[0].0.receiver_trait_derefs,
             autoref_or_ptr_adjustment: None,
             self_ty,
             unstable_candidates: vec![],
+            receiver_trait_derefs: 0,
         })
     }
 
@@ -2197,6 +2313,7 @@ impl<'tcx> Candidate<'tcx> {
             autoref_or_ptr_adjustment: None,
             self_ty,
             unstable_candidates,
+            receiver_trait_derefs: self.receiver_trait_derefs,
         }
     }
 }