-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Winnow private method candidates instead of assuming any candidate of the right name will apply #125622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Winnow private method candidates instead of assuming any candidate of the right name will apply #125622
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5e8df95
Manual rustfmt
oli-obk 14f9c63
Show that it will pick up the entirely wrong function as a private ca...
oli-obk 7d151fa
Turn a delayed bug back into a normal bug by winnowing private method...
oli-obk 7894a11
Move tests to a more appropriate directory
oli-obk 8189506
Give test a more useful name
oli-obk ffb1b2c
Add test description
oli-obk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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)>>, | ||
|
Comment on lines
+80
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the difference between these lol There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or actually, better question: Why do we need two fields here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need the Of course we could just eagerly report (not return) an error from method selection and return that no methods were found, which will subsequently return an error containing the list of traits that should be imported. But then method selection is reporting errors, which is also not be desirable (and probably wrong in various cases where we'll return a better error later). |
||
|
|
||
| /// 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() { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 0 additions & 17 deletions
tests/ui/issues/issue-53498.rs
Oops, something went wrong.
15 changes: 15 additions & 0 deletions
tests/ui/privacy/ufc-method-call.different_name.stderr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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>` | ||
| } |
2 changes: 1 addition & 1 deletion
tests/ui/issues/issue-53498.stderr → .../privacy/ufc-method-call.same_name.stderr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.