Rollup merge of rust-lang#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 rust-lang#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

Author: compiler-errors

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -1072,7 +1072,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).
@@ -1416,11 +1416,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?",
+                        ),
                     );
                 }
             }

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() {

src/tools/tidy/src/issues.txt

@@ -2445,7 +2445,6 @@ ui/issues/issue-53300.rs
 ui/issues/issue-53333.rs
 ui/issues/issue-53348.rs
 ui/issues/issue-53419.rs
-ui/issues/issue-53498.rs
 ui/issues/issue-53568.rs
 ui/issues/issue-5358-1.rs
 ui/issues/issue-53728.rs

tests/ui/issues/issue-53498.rs

@@ -0,0 +1,15 @@
+error[E0599]: no function or associated item named `foo` found for struct `Foo<B>` in the current scope
+  --> $DIR/ufc-method-call.rs:27:27
+   |
+LL |     pub struct Foo<T>(T);
+   |     ----------------- function or associated item `foo` not found for this struct
+...
+LL |     test::Foo::<test::B>::foo();
+   |                           ^^^ function or associated item not found in `Foo<B>`
+   |
+   = note: the function or associated item was found for
+           - `Foo<A>`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0599`.

tests/ui/privacy/ufc-method-call.different_name.stderr

@@ -0,0 +1,30 @@
+//! This test used to report that the method call cannot
+//! call the private method `Foo<A>::foo`, even though the user
+//! explicitly selected `Foo<B>::foo`. This is because we only
+//! looked for methods of the right name, without properly checking
+//! the `Self` type
+
+//@ revisions: same_name different_name
+
+pub mod test {
+    pub struct A;
+    pub struct B;
+    pub struct Foo<T>(T);
+
+    impl Foo<A> {
+        fn foo() {}
+    }
+
+    impl Foo<B> {
+        #[cfg(same_name)]
+        fn foo() {}
+        #[cfg(different_name)]
+        fn bar() {}
+    }
+}
+
+fn main() {
+    test::Foo::<test::B>::foo();
+    //[same_name]~^ ERROR associated function `foo` is private
+    //[different_name]~^^ ERROR no function or associated item named `foo` found for struct `Foo<B>`
+}

tests/ui/privacy/ufc-method-call.rs

@@ -1,5 +1,5 @@
 error[E0624]: associated function `foo` is private
-  --> $DIR/issue-53498.rs:16:27
+  --> $DIR/ufc-method-call.rs:27:27
    |
 LL |         fn foo() {}
    |         -------- private associated function defined here