From 5e8df95dbb445b2891610a9e46ce31c2b615bc09 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 27 May 2024 13:01:16 +0000 Subject: [PATCH 1/6] Manual rustfmt --- compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 58eb0c2817987..5f73b3b618f5f 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -1419,8 +1419,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.dcx().span_delayed_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?", + ), ); } } From 14f9c63759b67ad725dbecb003bcb7752fc92d69 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 28 May 2024 06:32:35 +0000 Subject: [PATCH 2/6] Show that it will pick up the entirely wrong function as a private candidate --- src/tools/tidy/src/ui_tests.rs | 2 +- ...3498.stderr => issue-53498.different_name.stderr} | 2 +- tests/ui/issues/issue-53498.rs | 9 ++++++++- tests/ui/issues/issue-53498.same_name.stderr | 12 ++++++++++++ 4 files changed, 22 insertions(+), 3 deletions(-) rename tests/ui/issues/{issue-53498.stderr => issue-53498.different_name.stderr} (91%) create mode 100644 tests/ui/issues/issue-53498.same_name.stderr diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index cce0fb2c1a25b..37ce335c9a3dd 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 = 1675; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files diff --git a/tests/ui/issues/issue-53498.stderr b/tests/ui/issues/issue-53498.different_name.stderr similarity index 91% rename from tests/ui/issues/issue-53498.stderr rename to tests/ui/issues/issue-53498.different_name.stderr index 61a1aedf50821..45ee858ac9215 100644 --- a/tests/ui/issues/issue-53498.stderr +++ b/tests/ui/issues/issue-53498.different_name.stderr @@ -1,5 +1,5 @@ error[E0624]: associated function `foo` is private - --> $DIR/issue-53498.rs:16:27 + --> $DIR/issue-53498.rs:21:27 | LL | fn foo() {} | -------- private associated function defined here diff --git a/tests/ui/issues/issue-53498.rs b/tests/ui/issues/issue-53498.rs index 9e0437c46f4bd..816d3f9e448fa 100644 --- a/tests/ui/issues/issue-53498.rs +++ b/tests/ui/issues/issue-53498.rs @@ -1,3 +1,5 @@ +//@ revisions: same_name different_name + pub mod test { pub struct A; pub struct B; @@ -8,10 +10,15 @@ pub mod test { } impl Foo { + #[cfg(same_name)] fn foo() {} + #[cfg(different_name)] + fn bar() {} } } fn main() { - test::Foo::::foo(); //~ ERROR associated function `foo` is private + test::Foo::::foo(); + //[same_name]~^ ERROR associated function `foo` is private + //[different_name]~^^ ERROR associated function `foo` is private } diff --git a/tests/ui/issues/issue-53498.same_name.stderr b/tests/ui/issues/issue-53498.same_name.stderr new file mode 100644 index 0000000000000..45ee858ac9215 --- /dev/null +++ b/tests/ui/issues/issue-53498.same_name.stderr @@ -0,0 +1,12 @@ +error[E0624]: associated function `foo` is private + --> $DIR/issue-53498.rs:21:27 + | +LL | fn foo() {} + | -------- private associated function defined here +... +LL | test::Foo::::foo(); + | ^^^ private associated function + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0624`. From 7d151fa3b015ad1dbac18933c58db3413a964534 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 27 May 2024 15:51:21 +0000 Subject: [PATCH 3/6] Turn a delayed bug back into a normal bug by winnowing private method candidates instead of assuming any candidate of the right name will apply. --- .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 4 +-- compiler/rustc_hir_typeck/src/method/probe.rs | 28 ++++++++++++++----- .../issues/issue-53498.different_name.stderr | 13 +++++---- tests/ui/issues/issue-53498.rs | 2 +- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 5f73b3b618f5f..4bd73dc6c9425 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/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 { // `::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,7 +1416,7 @@ 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?", diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index 12ced49f92ff7..ab0f16bd87d07 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>, + /// Some(candidate) if there is a private candidate - private_candidate: Option<(DefKind, DefId)>, + private_candidate: Cell>, /// 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/tests/ui/issues/issue-53498.different_name.stderr b/tests/ui/issues/issue-53498.different_name.stderr index 45ee858ac9215..fc765f2e75d3f 100644 --- a/tests/ui/issues/issue-53498.different_name.stderr +++ b/tests/ui/issues/issue-53498.different_name.stderr @@ -1,12 +1,15 @@ -error[E0624]: associated function `foo` is private +error[E0599]: no function or associated item named `foo` found for struct `Foo` in the current scope --> $DIR/issue-53498.rs:21:27 | -LL | fn foo() {} - | -------- private associated function defined here +LL | pub struct Foo(T); + | ----------------- function or associated item `foo` not found for this struct ... LL | test::Foo::::foo(); - | ^^^ private associated function + | ^^^ function or associated item not found in `Foo` + | + = note: the function or associated item was found for + - `Foo` error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0624`. +For more information about this error, try `rustc --explain E0599`. diff --git a/tests/ui/issues/issue-53498.rs b/tests/ui/issues/issue-53498.rs index 816d3f9e448fa..64eb3a0c5fd4e 100644 --- a/tests/ui/issues/issue-53498.rs +++ b/tests/ui/issues/issue-53498.rs @@ -20,5 +20,5 @@ pub mod test { fn main() { test::Foo::::foo(); //[same_name]~^ ERROR associated function `foo` is private - //[different_name]~^^ ERROR associated function `foo` is private + //[different_name]~^^ ERROR no function or associated item named `foo` found for struct `Foo` } From 7894a1148371e4d0fa665c183615a76f9e2ec656 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 28 May 2024 09:26:40 +0000 Subject: [PATCH 4/6] Move tests to a more appropriate directory --- src/tools/tidy/src/ui_tests.rs | 2 +- tests/ui/{issues => privacy}/issue-53498.different_name.stderr | 0 tests/ui/{issues => privacy}/issue-53498.rs | 0 tests/ui/{issues => privacy}/issue-53498.same_name.stderr | 0 4 files changed, 1 insertion(+), 1 deletion(-) rename tests/ui/{issues => privacy}/issue-53498.different_name.stderr (100%) rename tests/ui/{issues => privacy}/issue-53498.rs (100%) rename tests/ui/{issues => privacy}/issue-53498.same_name.stderr (100%) diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 37ce335c9a3dd..f2eeda339d80e 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 = 1675; +const ISSUES_ENTRY_LIMIT: u32 = 1672; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files diff --git a/tests/ui/issues/issue-53498.different_name.stderr b/tests/ui/privacy/issue-53498.different_name.stderr similarity index 100% rename from tests/ui/issues/issue-53498.different_name.stderr rename to tests/ui/privacy/issue-53498.different_name.stderr diff --git a/tests/ui/issues/issue-53498.rs b/tests/ui/privacy/issue-53498.rs similarity index 100% rename from tests/ui/issues/issue-53498.rs rename to tests/ui/privacy/issue-53498.rs diff --git a/tests/ui/issues/issue-53498.same_name.stderr b/tests/ui/privacy/issue-53498.same_name.stderr similarity index 100% rename from tests/ui/issues/issue-53498.same_name.stderr rename to tests/ui/privacy/issue-53498.same_name.stderr From 81895065bb541b9e79f402d2446c1c83243b37d0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 28 May 2024 09:32:53 +0000 Subject: [PATCH 5/6] Give test a more useful name --- src/tools/tidy/src/issues.txt | 1 - ...ferent_name.stderr => ufc-method-call.different_name.stderr} | 2 +- tests/ui/privacy/{issue-53498.rs => ufc-method-call.rs} | 0 ...-53498.same_name.stderr => ufc-method-call.same_name.stderr} | 2 +- 4 files changed, 2 insertions(+), 3 deletions(-) rename tests/ui/privacy/{issue-53498.different_name.stderr => ufc-method-call.different_name.stderr} (93%) rename tests/ui/privacy/{issue-53498.rs => ufc-method-call.rs} (100%) rename tests/ui/privacy/{issue-53498.same_name.stderr => ufc-method-call.same_name.stderr} (90%) diff --git a/src/tools/tidy/src/issues.txt b/src/tools/tidy/src/issues.txt index 398a6fd0fbaaa..a6ba8959f0c5d 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/tests/ui/privacy/issue-53498.different_name.stderr b/tests/ui/privacy/ufc-method-call.different_name.stderr similarity index 93% rename from tests/ui/privacy/issue-53498.different_name.stderr rename to tests/ui/privacy/ufc-method-call.different_name.stderr index fc765f2e75d3f..80321b1c11cb4 100644 --- a/tests/ui/privacy/issue-53498.different_name.stderr +++ b/tests/ui/privacy/ufc-method-call.different_name.stderr @@ -1,5 +1,5 @@ error[E0599]: no function or associated item named `foo` found for struct `Foo` in the current scope - --> $DIR/issue-53498.rs:21:27 + --> $DIR/ufc-method-call.rs:21:27 | LL | pub struct Foo(T); | ----------------- function or associated item `foo` not found for this struct diff --git a/tests/ui/privacy/issue-53498.rs b/tests/ui/privacy/ufc-method-call.rs similarity index 100% rename from tests/ui/privacy/issue-53498.rs rename to tests/ui/privacy/ufc-method-call.rs diff --git a/tests/ui/privacy/issue-53498.same_name.stderr b/tests/ui/privacy/ufc-method-call.same_name.stderr similarity index 90% rename from tests/ui/privacy/issue-53498.same_name.stderr rename to tests/ui/privacy/ufc-method-call.same_name.stderr index 45ee858ac9215..c66ae0a6b2527 100644 --- a/tests/ui/privacy/issue-53498.same_name.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:21:27 + --> $DIR/ufc-method-call.rs:21:27 | LL | fn foo() {} | -------- private associated function defined here From ffb1b2c14809ae5d15a078a9cce5299865d7dd73 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 28 May 2024 09:34:16 +0000 Subject: [PATCH 6/6] Add test description --- tests/ui/privacy/ufc-method-call.different_name.stderr | 2 +- tests/ui/privacy/ufc-method-call.rs | 6 ++++++ tests/ui/privacy/ufc-method-call.same_name.stderr | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/ui/privacy/ufc-method-call.different_name.stderr b/tests/ui/privacy/ufc-method-call.different_name.stderr index 80321b1c11cb4..16496c480dd12 100644 --- a/tests/ui/privacy/ufc-method-call.different_name.stderr +++ b/tests/ui/privacy/ufc-method-call.different_name.stderr @@ -1,5 +1,5 @@ error[E0599]: no function or associated item named `foo` found for struct `Foo` in the current scope - --> $DIR/ufc-method-call.rs:21:27 + --> $DIR/ufc-method-call.rs:27:27 | LL | pub struct Foo(T); | ----------------- function or associated item `foo` not found for this struct diff --git a/tests/ui/privacy/ufc-method-call.rs b/tests/ui/privacy/ufc-method-call.rs index 64eb3a0c5fd4e..525d9a9eee904 100644 --- a/tests/ui/privacy/ufc-method-call.rs +++ b/tests/ui/privacy/ufc-method-call.rs @@ -1,3 +1,9 @@ +//! This test used to report that the method call cannot +//! call the private method `Foo::foo`, even though the user +//! explicitly selected `Foo::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 { diff --git a/tests/ui/privacy/ufc-method-call.same_name.stderr b/tests/ui/privacy/ufc-method-call.same_name.stderr index c66ae0a6b2527..194ba42cbf985 100644 --- a/tests/ui/privacy/ufc-method-call.same_name.stderr +++ b/tests/ui/privacy/ufc-method-call.same_name.stderr @@ -1,5 +1,5 @@ error[E0624]: associated function `foo` is private - --> $DIR/ufc-method-call.rs:21:27 + --> $DIR/ufc-method-call.rs:27:27 | LL | fn foo() {} | -------- private associated function defined here