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

8 files changed, 72 insertions, 31 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() {
diff --git a/src/tools/tidy/src/issues.txt b/src/tools/tidy/src/issues.txt
index 398a6fd0fba..a6ba8959f0c 100644
--- a/src/tools/tidy/src/issues.txt
+++ b/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
diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs
index cce0fb2c1a2..f2eeda339d8 100644
--- a/src/tools/tidy/src/ui_tests.rs
+++ b/src/tools/tidy/src/ui_tests.rs
@@ -15,7 +15,7 @@ use std::path::{Path, PathBuf};
 const ENTRY_LIMIT: u32 = 900;
 // FIXME: The following limits should be reduced eventually.
 
-const ISSUES_ENTRY_LIMIT: u32 = 1674;
+const ISSUES_ENTRY_LIMIT: u32 = 1672;
 
 const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
     "rs",     // test source files
diff --git a/tests/ui/issues/issue-53498.rs b/tests/ui/issues/issue-53498.rs
deleted file mode 100644
index 9e0437c46f4..00000000000
--- a/tests/ui/issues/issue-53498.rs
+++ /dev/null
@@ -1,17 +0,0 @@
-pub mod test {
-    pub struct A;
-    pub struct B;
-    pub struct Foo<T>(T);
-
-    impl Foo<A> {
-        fn foo() {}
-    }
-
-    impl Foo<B> {
-        fn foo() {}
-    }
-}
-
-fn main() {
-    test::Foo::<test::B>::foo(); //~ ERROR associated function `foo` is private
-}
diff --git a/tests/ui/privacy/ufc-method-call.different_name.stderr b/tests/ui/privacy/ufc-method-call.different_name.stderr
new file mode 100644
index 00000000000..16496c480dd
--- /dev/null
+++ b/tests/ui/privacy/ufc-method-call.different_name.stderr
@@ -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`.
diff --git a/tests/ui/privacy/ufc-method-call.rs b/tests/ui/privacy/ufc-method-call.rs
new file mode 100644
index 00000000000..525d9a9eee9
--- /dev/null
+++ b/tests/ui/privacy/ufc-method-call.rs
@@ -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>`
+}
diff --git a/tests/ui/issues/issue-53498.stderr b/tests/ui/privacy/ufc-method-call.same_name.stderr
index 61a1aedf508..194ba42cbf9 100644
--- a/tests/ui/issues/issue-53498.stderr
+++ b/tests/ui/privacy/ufc-method-call.same_name.stderr
@@ -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