about summary refs log tree commit diff
path: root/compiler
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-06-05 18:21:11 +0200
committerGitHub <noreply@github.com>2024-06-05 18:21:11 +0200
commit9abf8b105e22dd876834bf5094c77b0f2702f974 (patch)
tree8aaedcb59eb93d128b29d3d8596d1f67cc97855a /compiler
parent69a8c139f1d7c97dc8b9bc0086e042fa6c79a389 (diff)
parentffb1b2c14809ae5d15a078a9cce5299865d7dd73 (diff)
downloadrust-9abf8b105e22dd876834bf5094c77b0f2702f974.tar.gz
rust-9abf8b105e22dd876834bf5094c77b0f2702f974.zip
Rollup merge of #125622 - oli-obk:define_opaque_types15, r=compiler-errors
Winnow private method candidates instead of assuming any candidate of the right name will apply

partially reverts https://github.com/rust-lang/rust/pull/60721

My original motivation was just to avoid the `delay_span_bug` (by attempting to thread the `ErrorGuaranteed` through to here). But then I realized that the error message is wrong. It refers to the `Foo<A>::foo` instead of `Foo<B>::foo`. This is almost invisible, because both functions are the same, but on different lines, so `-Zui-testing` makes it so the test is the same no matter which of these two functions is referenced.

But there's a much more obvious bug: If `Foo<B>` does not have a `foo` method at all, but `Foo<A>` has a private `foo` method, then we'll refer to that one. This has now been fixed, and we report a normal `method not found` error.

The way this is done is by creating a list of all possible private functions (just like we create a list of the public functions that can actually be called), and then winnowing it by analyzing where bounds and `Self` types to see if any of the found methods can actually apply (again, just like with the list of public functions).

I wonder if there is room for doing the same thing with unstable functions instead of running all of method resolution twice.

r? ``@compiler-errors`` for method resolution stuff
Diffstat (limited to 'compiler')
-rw-r--r--compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs8
-rw-r--r--compiler/rustc_hir_typeck/src/method/probe.rs28
2 files changed, 25 insertions, 11 deletions
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
index 7d7324a2b64..0a668074cf8 100644
--- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
+++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
@@ -1066,7 +1066,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     ty::ImplContainer => {
                         if segments.len() == 1 {
                             // `<T>::assoc` will end up here, and so
-                            // can `T::assoc`. It this came from an
+                            // can `T::assoc`. If this came from an
                             // inherent impl, we need to record the
                             // `T` for posterity (see `UserSelfTy` for
                             // details).
@@ -1410,11 +1410,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             ) {
                 Ok(ok) => self.register_infer_ok_obligations(ok),
                 Err(_) => {
-                    self.dcx().span_delayed_bug(
+                    self.dcx().span_bug(
                         span,
                         format!(
-                        "instantiate_value_path: (UFCS) {self_ty:?} was a subtype of {impl_ty:?} but now is not?",
-                    ),
+                            "instantiate_value_path: (UFCS) {self_ty:?} was a subtype of {impl_ty:?} but now is not?",
+                        ),
                     );
                 }
             }
diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs
index 12ced49f92f..ab0f16bd87d 100644
--- a/compiler/rustc_hir_typeck/src/method/probe.rs
+++ b/compiler/rustc_hir_typeck/src/method/probe.rs
@@ -41,6 +41,7 @@ use rustc_trait_selection::traits::query::method_autoderef::{
 use rustc_trait_selection::traits::query::CanonicalTyGoal;
 use rustc_trait_selection::traits::ObligationCtxt;
 use rustc_trait_selection::traits::{self, ObligationCause};
+use std::cell::Cell;
 use std::cell::RefCell;
 use std::cmp::max;
 use std::iter;
@@ -76,8 +77,12 @@ pub(crate) struct ProbeContext<'a, 'tcx> {
     /// requested name (by edit distance)
     allow_similar_names: bool,
 
+    /// List of potential private candidates. Will be trimmed to ones that
+    /// actually apply and then the result inserted into `private_candidate`
+    private_candidates: Vec<Candidate<'tcx>>,
+
     /// Some(candidate) if there is a private candidate
-    private_candidate: Option<(DefKind, DefId)>,
+    private_candidate: Cell<Option<(DefKind, DefId)>>,
 
     /// Collects near misses when the candidate functions are missing a `self` keyword and is only
     /// used for error reporting
@@ -581,7 +586,8 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             orig_steps_var_values,
             steps,
             allow_similar_names: false,
-            private_candidate: None,
+            private_candidates: Vec::new(),
+            private_candidate: Cell::new(None),
             static_candidates: RefCell::new(Vec::new()),
             unsatisfied_predicates: RefCell::new(Vec::new()),
             scope_expr_id,
@@ -593,7 +599,8 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         self.inherent_candidates.clear();
         self.extension_candidates.clear();
         self.impl_dups.clear();
-        self.private_candidate = None;
+        self.private_candidates.clear();
+        self.private_candidate.set(None);
         self.static_candidates.borrow_mut().clear();
         self.unsatisfied_predicates.borrow_mut().clear();
     }
@@ -617,9 +624,8 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             } else {
                 self.extension_candidates.push(candidate);
             }
-        } else if self.private_candidate.is_none() {
-            self.private_candidate =
-                Some((candidate.item.kind.as_def_kind(), candidate.item.def_id));
+        } else {
+            self.private_candidates.push(candidate);
         }
     }
 
@@ -1171,7 +1177,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         let mut possibly_unsatisfied_predicates = Vec::new();
 
         for (kind, candidates) in
-            &[("inherent", &self.inherent_candidates), ("extension", &self.extension_candidates)]
+            [("inherent", &self.inherent_candidates), ("extension", &self.extension_candidates)]
         {
             debug!("searching {} candidates", kind);
             let res = self.consider_candidates(
@@ -1185,6 +1191,14 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             }
         }
 
+        if self.private_candidate.get().is_none() {
+            if let Some(Ok(pick)) =
+                self.consider_candidates(self_ty, &self.private_candidates, &mut vec![], None)
+            {
+                self.private_candidate.set(Some((pick.item.kind.as_def_kind(), pick.item.def_id)));
+            }
+        }
+
         // `pick_method` may be called twice for the same self_ty if no stable methods
         // match. Only extend once.
         if unstable_candidates.is_some() {