Skip to content

Commit

Permalink
Unrolled build for rust-lang#125622
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rust-timer authored Jun 5, 2024
2 parents 7ebd2bd + ffb1b2c commit 33c42bd
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 31 deletions.
8 changes: 4 additions & 4 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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?",
),
);
}
}
Expand Down
28 changes: 21 additions & 7 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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();
}
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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(
Expand All @@ -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() {
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/issues.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 0 additions & 17 deletions tests/ui/issues/issue-53498.rs

This file was deleted.

15 changes: 15 additions & 0 deletions tests/ui/privacy/ufc-method-call.different_name.stderr
Original file line number Diff line number Diff line change
@@ -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`.
30 changes: 30 additions & 0 deletions tests/ui/privacy/ufc-method-call.rs
Original file line number Diff line number Diff line change
@@ -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>`
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down

0 comments on commit 33c42bd

Please sign in to comment.