Skip to content

Normalization issue with operators or Deref #9971

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

Closed
lnicola opened this issue Aug 21, 2021 · 7 comments · Fixed by #9988
Closed

Normalization issue with operators or Deref #9971

lnicola opened this issue Aug 21, 2021 · 7 comments · Fixed by #9988
Assignees
Labels
A-ty type system / type inference / traits / method resolution S-actionable Someone could pick this issue up and work on it right now

Comments

@lnicola
Copy link
Member

lnicola commented Aug 21, 2021

CC #8961

let s = String::new() + &String::new(); // s is <String as Add<String>>::Output
@lnicola lnicola added A-ty type system / type inference / traits / method resolution S-actionable Someone could pick this issue up and work on it right now labels Aug 21, 2021
@lnicola
Copy link
Member Author

lnicola commented Aug 21, 2021

This might be related to Deref or something else. The following snipped works as expected:

use std::ops::Add;

struct S;

impl Add<&Self> for S {
    type Output = S;

    fn add(self, _rhs: &Self) -> Self::Output {
        todo!()
    }
}

fn main() {
    let s = S + &S;
}

@lnicola lnicola changed the title Normalization issue with operators Normalization issue with operators or Deref Aug 21, 2021
@flodiebold
Copy link
Member

flodiebold commented Aug 21, 2021

Actually, String::new() + String::new() is a type error, isn't it?

Edit: let s = String::new() + &String::new() is valid and also a problem for us, though.

So this might actually be the same as #9968.

@flodiebold
Copy link
Member

flodiebold commented Aug 21, 2021

Yeah, I think the problem is that we don't do coercions on the right hand side in binary ops (and assignment is a binary op, so that's #9968).

@lnicola
Copy link
Member Author

lnicola commented Aug 21, 2021

Actually, String::new() + String::new() is a type error, isn't it?

Well, this is awkward...

@lnicola
Copy link
Member Author

lnicola commented Aug 21, 2021

Should we also coerce on the LHS?

@flodiebold
Copy link
Member

There's nothing to really coerce to at that point. (rustc does coerce to a fresh type variable, but just to avoid too strict lifetime constraints.)

@lnicola
Copy link
Member Author

lnicola commented Aug 21, 2021

The problem is that we only do the trait resolution at the end, but we need to do it before checking the right-hand side (with a variable in place of the RHS type). For String, that will tell us that the RHS type has to be &str.

@flodiebold flodiebold self-assigned this Aug 22, 2021
bors bot added a commit that referenced this issue Aug 22, 2021
9988: fix: Refactor & improve handling of overloaded binary operators r=flodiebold a=flodiebold

Fixes #9971. Also records them as method resolutions, which we could use later.

Co-authored-by: Florian Diebold <[email protected]>
@bors bors bot closed this as completed in 424dda8 Aug 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ty type system / type inference / traits / method resolution S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants