Skip to content
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

FnPtr blanket impls cause "multiple applicable items in scope" for unrelated types. #109892

Closed
eddyb opened this issue Apr 3, 2023 · 1 comment · Fixed by #109896
Closed

FnPtr blanket impls cause "multiple applicable items in scope" for unrelated types. #109892

eddyb opened this issue Apr 3, 2023 · 1 comment · Fixed by #109896
Assignees
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Apr 3, 2023

Note: the below reduction originally came from a use num_traits::Float; + 0.0.max(x) in the wild.

I haven't found a way to demonstrate this outside of the "float inference vars + Ord method" combo, but:

trait MyCmp {
    fn cmp(&self) {}
}
impl MyCmp for f32 {}

fn main() {
    0.0.cmp();
}

works on stable (1.68.2) and beta (1.69.0-beta.6), and errors on nightly with:

error[E0034]: multiple applicable items in scope
 --> src/main.rs:7:9
  |
7 |     0.0.cmp();
  |         ^^^ multiple `cmp` found
  |
note: candidate #1 is defined in an impl of the trait `MyCmp` for the type `f32`
 --> src/main.rs:2:5
  |
2 |     fn cmp(&self) {}
  |     ^^^^^^^^^^^^^
  = note: candidate #2 is defined in an impl of the trait `Ord` for the type `F`
note: candidate #3 is defined in the trait `Iterator`
 --> /rustc/3a8a131e9509c478ece1c58fe0ea2d49463d2300/library/core/src/iter/traits/iterator.rs:3533:5
help: disambiguate the method for candidate #1
  |
7 |     MyCmp::cmp(&0.0);
  |     ~~~~~~~~~~~~~~~~
help: disambiguate the method for candidate #2
  |
7 |     Ord::cmp(&0.0);
  |     ~~~~~~~~~~~~~~
help: disambiguate the method for candidate #3
  |
7 |     Iterator::cmp(0.0);
  |     ~~~~~~~~~~~~~~~~~~

Even though it doesn't show the source for candidate #2, I'm almost certain it's this:

#[stable(feature = "fnptr_impls", since = "1.4.0")]
impl<F: FnPtr> Ord for F {
#[inline]
fn cmp(&self, other: &Self) -> Ordering {
self.addr().cmp(&other.addr())
}
}

from (cc @lcnr @oli-obk):

On the original PR I left comments like #99531 (comment), trying to rectify the issues with the original approach, but they didn't go anywhere and the version that one in the end is the (arguably) more fragile original one, that uses blanket impls and works around their unwanted interactions.

It's possible in this case all it takes is checking for "float inference vars" and making sure they can never be considered to implement FnPtr, but I'm worried there might be other edge cases lurking in the shadows.

Also, did both of those PRs skip crater? I feel like this shouldn't be that hard to hit in the wild.

@eddyb eddyb added A-trait-system Area: Trait system regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. C-bug Category: This is a bug. labels Apr 3, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Apr 3, 2023
@apiraino
Copy link
Contributor

apiraino commented Apr 3, 2023

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Apr 3, 2023
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 3, 2023
@Noratrieb Noratrieb self-assigned this Apr 3, 2023
@bors bors closed this as completed in 7d3207b Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants