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

Surprising non-idempotent behavior on {integer} inference + method resolution #121453

Closed
slanterns opened this issue Feb 22, 2024 · 11 comments
Closed
Labels
A-inference Area: Type inference C-discussion Category: Discussion or questions that doesn't represent real issues. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@slanterns
Copy link
Contributor

slanterns commented Feb 22, 2024

Excerpt from Zulip (originally https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/Question.20from.20t-spec/near/422487076):

trait Trait {
    fn abs(self) -> Self;
}

impl Trait for i64 {
    fn abs(self) -> Self {
        2 * self
    }
}

fn main() {
    let x = 42;
    println!("{}", x.abs());
    println!("{}", x.abs());
}

Output (playground):

84
42

I find and repost the above code snippets from Zulip to a discussion group, and someone think it to be a counterintuitive oddity which deserves more discussion. Open the issue to see if there is anything can be improved (like adding a warning).

@slanterns slanterns added the C-bug Category: This is a bug. label Feb 22, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 22, 2024
@slanterns

This comment was marked as resolved.

@rustbot rustbot added C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed C-bug Category: This is a bug. labels Feb 22, 2024
@darkyzhou
Copy link

darkyzhou commented Feb 22, 2024

I observed more wired behaviors and wondered if it is actually a feature.

  1. It seems the first call is resolved to Trait::abs but the second one to core::num::<impl i64>::abs(which is a const fn). According to Method-call expressions there should be a compilation error.

https://godbolt.org/z/f4hn9K9cn

  1. It seems only the first call in a block will be resolved to Trait::abs and all the calls after in the same block to the latter.
截屏2024-02-22 22 37 35 截屏2024-02-22 22 37 41

@DarkHighness
Copy link

If let x : i64 = 42; then both calls will be resolved to core::num::<impl i64>::abs

https://godbolt.org/z/Mn4TEseMj

@chikaku
Copy link

chikaku commented Feb 22, 2024

If let x : i64 = 42; then both calls will be resolved to core::num::<impl i64>::abs

https://godbolt.org/z/Mn4TEseMj

I guess the type infer cause first x.abs() call search abs method and Trait::abs is found. Once the type of x is inferred to be i64, then it will use core::num::abs.

@slanterns
Copy link
Contributor Author

slanterns commented Feb 22, 2024

In the Zulip link from the description, errs has already given a clear explanation for the cause (see also someone on X.) The only question is whether such behavior is optimal.

@slanterns

This comment was marked as resolved.

@rustbot rustbot added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Feb 22, 2024
@compiler-errors
Copy link
Member

I don't believe this is possible to fix with a lot of fallout. See CI failures on #121459, we can't even bootstrap core for example.

@slanterns
Copy link
Contributor Author

I don't expect much real-world code to include this pattern, so it may not be worthwhile to make changes to inference. Will it be a good idea to have a warn-by-default lint about such trait impl on numeric types with ambiguous names with inherent methods?

@Jules-Bertholet
Copy link
Contributor

Could this be changed over an edition?

@jieyouxu jieyouxu added A-inference Area: Type inference S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 26, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Aug 22, 2024

It just occurred to me this is a duplicate of #99405 (the code example and output are identical, lol), and related to #72498.

@jieyouxu
Copy link
Member

Closing this as a duplicate of #99405, please continue discussion in that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inference Area: Type inference C-discussion Category: Discussion or questions that doesn't represent real issues. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants