Auto merge of #90329 - nbdd0121:typeck, r=nagisa

Try all stable method candidates first before trying unstable ones

Currently we try methods in this order in each step:
* Stable by value
* Unstable by value
* Stable autoref
* Unstable autoref
* ...

This PR changes it to first try pick methods without any unstable candidates, and if none is found, try again to pick unstable ones.

Fix #90320
CC #88971, hopefully would allow us to rename the "unstable_*" methods for integer impls back.

`@rustbot` label T-compiler T-libs-api

Author: bors

compiler/rustc_interface/src/tests.rs

@@ -752,6 +752,7 @@ fn test_debugging_options_tracking_hash() {
     tracked!(panic_abort_tests, true);
     tracked!(panic_in_drop, PanicStrategy::Abort);
     tracked!(partially_uninit_const_threshold, Some(123));
+    tracked!(pick_stable_methods_before_any_unstable, false);
     tracked!(plt, Some(true));
     tracked!(polonius, true);
     tracked!(precise_enum_drop_elaboration, false);

compiler/rustc_session/src/options.rs

@@ -1240,6 +1240,8 @@ options! {
         and set the maximum total size of a const allocation for which this is allowed (default: never)"),
     perf_stats: bool = (false, parse_bool, [UNTRACKED],
         "print some performance-related statistics (default: no)"),
+    pick_stable_methods_before_any_unstable: bool = (true, parse_bool, [TRACKED],
+        "try to pick stable methods first before picking any unstable methods (default: yes)"),
     plt: Option<bool> = (None, parse_opt_bool, [TRACKED],
         "whether to use the PLT when calling into shared libraries;
         only has effect for PIC code on systems with ELF binaries

compiler/rustc_typeck/src/check/method/probe.rs

@@ -92,7 +92,7 @@ impl<'a, 'tcx> Deref for ProbeContext<'a, 'tcx> {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 struct Candidate<'tcx> {
     // Candidates are (I'm not quite sure, but they are mostly) basically
     // some metadata on top of a `ty::AssocItem` (without substs).
@@ -132,7 +132,7 @@ struct Candidate<'tcx> {
     import_ids: SmallVec<[LocalDefId; 1]>,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 enum CandidateKind<'tcx> {
     InherentImplCandidate(
         SubstsRef<'tcx>,
@@ -204,6 +204,7 @@ pub struct Pick<'tcx> {
     /// Indicates that we want to add an autoref (and maybe also unsize it), or if the receiver is
     /// `*mut T`, convert it to `*const T`.
     pub autoref_or_ptr_adjustment: Option<AutorefOrPtrAdjustment<'tcx>>,
+    pub self_ty: Ty<'tcx>,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -1101,13 +1102,37 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
     }
 
     fn pick_core(&mut self) -> Option<PickResult<'tcx>> {
-        let steps = self.steps.clone();
+        let mut unstable_candidates = Vec::new();
+        let pick = self.pick_all_method(Some(&mut unstable_candidates));
+
+        // In this case unstable picking is done by `pick_method`.
+        if !self.tcx.sess.opts.debugging_opts.pick_stable_methods_before_any_unstable {
+            return pick;
+        }
 
-        // find the first step that works
+        match pick {
+            // Emit a lint if there are unstable candidates alongside the stable ones.
+            //
+            // We suppress warning if we're picking the method only because it is a
+            // suggestion.
+            Some(Ok(ref p)) if !self.is_suggestion.0 && !unstable_candidates.is_empty() => {
+                self.emit_unstable_name_collision_hint(p, &unstable_candidates);
+                pick
+            }
+            Some(_) => pick,
+            None => self.pick_all_method(None),
+        }
+    }
+
+    fn pick_all_method(
+        &mut self,
+        mut unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
+    ) -> Option<PickResult<'tcx>> {
+        let steps = self.steps.clone();
         steps
             .iter()
             .filter(|step| {
-                debug!("pick_core: step={:?}", step);
+                debug!("pick_all_method: step={:?}", step);
                 // skip types that are from a type error or that would require dereferencing
                 // a raw pointer
                 !step.self_ty.references_error() && !step.from_unsafe_deref
@@ -1123,11 +1148,30 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                     .unwrap_or_else(|_| {
                         span_bug!(self.span, "{:?} was applicable but now isn't?", step.self_ty)
                     });
-                self.pick_by_value_method(step, self_ty).or_else(|| {
-                    self.pick_autorefd_method(step, self_ty, hir::Mutability::Not)
-                        .or_else(|| self.pick_autorefd_method(step, self_ty, hir::Mutability::Mut))
-                        .or_else(|| self.pick_const_ptr_method(step, self_ty))
-                })
+                self.pick_by_value_method(step, self_ty, unstable_candidates.as_deref_mut())
+                    .or_else(|| {
+                        self.pick_autorefd_method(
+                            step,
+                            self_ty,
+                            hir::Mutability::Not,
+                            unstable_candidates.as_deref_mut(),
+                        )
+                        .or_else(|| {
+                            self.pick_autorefd_method(
+                                step,
+                                self_ty,
+                                hir::Mutability::Mut,
+                                unstable_candidates.as_deref_mut(),
+                            )
+                        })
+                        .or_else(|| {
+                            self.pick_const_ptr_method(
+                                step,
+                                self_ty,
+                                unstable_candidates.as_deref_mut(),
+                            )
+                        })
+                    })
             })
             .next()
     }
@@ -1142,12 +1186,13 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         &mut self,
         step: &CandidateStep<'tcx>,
         self_ty: Ty<'tcx>,
+        unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
     ) -> Option<PickResult<'tcx>> {
         if step.unsize {
             return None;
         }
 
-        self.pick_method(self_ty).map(|r| {
+        self.pick_method(self_ty, unstable_candidates).map(|r| {
             r.map(|mut pick| {
                 pick.autoderefs = step.autoderefs;
 
@@ -1170,14 +1215,15 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         step: &CandidateStep<'tcx>,
         self_ty: Ty<'tcx>,
         mutbl: hir::Mutability,
+        unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
     ) -> Option<PickResult<'tcx>> {
         let tcx = self.tcx;
 
         // In general, during probing we erase regions.
         let region = tcx.lifetimes.re_erased;
 
         let autoref_ty = tcx.mk_ref(region, ty::TypeAndMut { ty: self_ty, mutbl });
-        self.pick_method(autoref_ty).map(|r| {
+        self.pick_method(autoref_ty, unstable_candidates).map(|r| {
             r.map(|mut pick| {
                 pick.autoderefs = step.autoderefs;
                 pick.autoref_or_ptr_adjustment = Some(AutorefOrPtrAdjustment::Autoref {
@@ -1196,6 +1242,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         &mut self,
         step: &CandidateStep<'tcx>,
         self_ty: Ty<'tcx>,
+        unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
     ) -> Option<PickResult<'tcx>> {
         // Don't convert an unsized reference to ptr
         if step.unsize {
@@ -1209,7 +1256,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
 
         let const_self_ty = ty::TypeAndMut { ty, mutbl: hir::Mutability::Not };
         let const_ptr_ty = self.tcx.mk_ptr(const_self_ty);
-        self.pick_method(const_ptr_ty).map(|r| {
+        self.pick_method(const_ptr_ty, unstable_candidates).map(|r| {
             r.map(|mut pick| {
                 pick.autoderefs = step.autoderefs;
                 pick.autoref_or_ptr_adjustment = Some(AutorefOrPtrAdjustment::ToConstPtr);
@@ -1218,8 +1265,8 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         })
     }
 
-    fn pick_method(&mut self, self_ty: Ty<'tcx>) -> Option<PickResult<'tcx>> {
-        debug!("pick_method(self_ty={})", self.ty_to_string(self_ty));
+    fn pick_method_with_unstable(&mut self, self_ty: Ty<'tcx>) -> Option<PickResult<'tcx>> {
+        debug!("pick_method_with_unstable(self_ty={})", self.ty_to_string(self_ty));
 
         let mut possibly_unsatisfied_predicates = Vec::new();
         let mut unstable_candidates = Vec::new();
@@ -1241,7 +1288,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                         //
                         // We suppress warning if we're picking the method only because it is a
                         // suggestion.
-                        self.emit_unstable_name_collision_hint(p, &unstable_candidates, self_ty);
+                        self.emit_unstable_name_collision_hint(p, &unstable_candidates);
                     }
                 }
                 return Some(pick);
@@ -1251,7 +1298,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         debug!("searching unstable candidates");
         let res = self.consider_candidates(
             self_ty,
-            unstable_candidates.into_iter().map(|(c, _)| c),
+            unstable_candidates.iter().map(|(c, _)| c),
             &mut possibly_unsatisfied_predicates,
             None,
         );
@@ -1261,6 +1308,42 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         res
     }
 
+    fn pick_method(
+        &mut self,
+        self_ty: Ty<'tcx>,
+        mut unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
+    ) -> Option<PickResult<'tcx>> {
+        if !self.tcx.sess.opts.debugging_opts.pick_stable_methods_before_any_unstable {
+            return self.pick_method_with_unstable(self_ty);
+        }
+
+        debug!("pick_method(self_ty={})", self.ty_to_string(self_ty));
+
+        let mut possibly_unsatisfied_predicates = Vec::new();
+
+        for (kind, candidates) in
+            &[("inherent", &self.inherent_candidates), ("extension", &self.extension_candidates)]
+        {
+            debug!("searching {} candidates", kind);
+            let res = self.consider_candidates(
+                self_ty,
+                candidates.iter(),
+                &mut possibly_unsatisfied_predicates,
+                unstable_candidates.as_deref_mut(),
+            );
+            if let Some(pick) = res {
+                return Some(pick);
+            }
+        }
+
+        // `pick_method` may be called twice for the same self_ty if no stable methods
+        // match. Only extend once.
+        if unstable_candidates.is_some() {
+            self.unsatisfied_predicates.extend(possibly_unsatisfied_predicates);
+        }
+        None
+    }
+
     fn consider_candidates<'b, ProbesIter>(
         &self,
         self_ty: Ty<'tcx>,
@@ -1269,10 +1352,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             ty::Predicate<'tcx>,
             Option<ty::Predicate<'tcx>>,
         )>,
-        unstable_candidates: Option<&mut Vec<(&'b Candidate<'tcx>, Symbol)>>,
+        unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
     ) -> Option<PickResult<'tcx>>
     where
         ProbesIter: Iterator<Item = &'b Candidate<'tcx>> + Clone,
+        'tcx: 'b,
     {
         let mut applicable_candidates: Vec<_> = probes
             .clone()
@@ -1285,7 +1369,9 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         debug!("applicable_candidates: {:?}", applicable_candidates);
 
         if applicable_candidates.len() > 1 {
-            if let Some(pick) = self.collapse_candidates_to_trait_pick(&applicable_candidates[..]) {
+            if let Some(pick) =
+                self.collapse_candidates_to_trait_pick(self_ty, &applicable_candidates[..])
+            {
                 return Some(Ok(pick));
             }
         }
@@ -1295,7 +1381,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                 if let stability::EvalResult::Deny { feature, .. } =
                     self.tcx.eval_stability(p.item.def_id, None, self.span, None)
                 {
-                    uc.push((p, feature));
+                    uc.push((p.clone(), feature));
                     return false;
                 }
                 true
@@ -1309,7 +1395,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
 
         applicable_candidates.pop().map(|(probe, status)| {
             if status == ProbeResult::Match {
-                Ok(probe.to_unadjusted_pick())
+                Ok(probe.to_unadjusted_pick(self_ty))
             } else {
                 Err(MethodError::BadReturnType)
             }
@@ -1319,8 +1405,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
     fn emit_unstable_name_collision_hint(
         &self,
         stable_pick: &Pick<'_>,
-        unstable_candidates: &[(&Candidate<'tcx>, Symbol)],
-        self_ty: Ty<'tcx>,
+        unstable_candidates: &[(Candidate<'tcx>, Symbol)],
     ) {
         self.tcx.struct_span_lint_hir(
             lint::builtin::UNSTABLE_NAME_COLLISIONS,
@@ -1351,7 +1436,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                             "use the fully qualified path to the associated const",
                             format!(
                                 "<{} as {}>::{}",
-                                self_ty,
+                                stable_pick.self_ty,
                                 self.tcx.def_path_str(def_id),
                                 stable_pick.item.ident
                             ),
@@ -1591,6 +1676,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
     /// use, so it's ok to just commit to "using the method from the trait Foo".
     fn collapse_candidates_to_trait_pick(
         &self,
+        self_ty: Ty<'tcx>,
         probes: &[(&Candidate<'tcx>, ProbeResult)],
     ) -> Option<Pick<'tcx>> {
         // Do all probes correspond to the same trait?
@@ -1610,6 +1696,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             import_ids: probes[0].0.import_ids.clone(),
             autoderefs: 0,
             autoref_or_ptr_adjustment: None,
+            self_ty,
         })
     }
 
@@ -1828,7 +1915,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
 }
 
 impl<'tcx> Candidate<'tcx> {
-    fn to_unadjusted_pick(&self) -> Pick<'tcx> {
+    fn to_unadjusted_pick(&self, self_ty: Ty<'tcx>) -> Pick<'tcx> {
         Pick {
             item: self.item,
             kind: match self.kind {
@@ -1852,6 +1939,7 @@ impl<'tcx> Candidate<'tcx> {
             import_ids: self.import_ids.clone(),
             autoderefs: 0,
             autoref_or_ptr_adjustment: None,
+            self_ty,
         }
     }
 }

src/test/ui/inference/auxiliary/inference_unstable_iterator.rs

@@ -1,4 +1,5 @@
 #![feature(staged_api)]
+#![feature(arbitrary_self_types)]
 
 #![stable(feature = "ipu_iterator", since = "1.0.0")]
 
@@ -8,6 +9,22 @@ pub trait IpuIterator {
     fn ipu_flatten(&self) -> u32 {
         0
     }
+
+    #[unstable(feature = "ipu_flatten", issue = "99999")]
+    fn ipu_by_value_vs_by_ref(self) -> u32 where Self: Sized {
+        0
+    }
+
+    #[unstable(feature = "ipu_flatten", issue = "99999")]
+    fn ipu_by_ref_vs_by_ref_mut(&self) -> u32 {
+        0
+    }
+
+    #[unstable(feature = "ipu_flatten", issue = "99999")]
+    fn ipu_by_mut_ptr_vs_by_const_ptr(self: *mut Self) -> u32 {
+        0
+    }
+
     #[unstable(feature = "assoc_const_ipu_iter", issue = "99999")]
     const C: i32;
 }

src/test/ui/inference/auxiliary/inference_unstable_itertools.rs

@@ -1,8 +1,22 @@
+#![feature(arbitrary_self_types)]
+
 pub trait IpuItertools {
     fn ipu_flatten(&self) -> u32 {
         1
     }
 
+    fn ipu_by_value_vs_by_ref(&self) -> u32 {
+        1
+    }
+
+    fn ipu_by_ref_vs_by_ref_mut(&mut self) -> u32 {
+        1
+    }
+
+    fn ipu_by_mut_ptr_vs_by_const_ptr(self: *const Self) -> u32 {
+        1
+    }
+
     const C: i32;
 }