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

Binary operator LHS type is fixed instead of being subtyped #45425

Closed
shepmaster opened this issue Oct 21, 2017 · 1 comment
Closed

Binary operator LHS type is fixed instead of being subtyped #45425

shepmaster opened this issue Oct 21, 2017 · 1 comment
Assignees

Comments

@shepmaster
Copy link
Member

shepmaster commented Oct 21, 2017

This started with a Stack Overflow question which boiled down to:

use std::ops::Rem;

fn powm<T>(fbase: &T, modulus: &T)
where
    for<'x> &'x T: Rem<&'x T, Output = T>,
{
    fbase % modulus;
}

fn main() {}
Complete error
error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
 --> src/main.rs:7:11
  |
7 |     fbase % modulus;
  |           ^
  |
note: first, the lifetime cannot outlive the lifetime 'b as defined on the function body at 3:1...
 --> src/main.rs:3:1
  |
3 | / fn powm<'a, 'b, T>(fbase: &'a T, modulus: &'b T)
4 | | where
5 | |     for<'x> &'x T: Rem<&'x T, Output = T>,
6 | | {
7 | |     fbase % modulus;
8 | | }
  | |_^
note: ...so that reference does not outlive borrowed content
 --> src/main.rs:7:13
  |
7 |     fbase % modulus;
  |             ^^^^^^^
note: but, the lifetime must be valid for the lifetime 'a as defined on the function body at 3:1...
 --> src/main.rs:3:1
  |
3 | / fn powm<'a, 'b, T>(fbase: &'a T, modulus: &'b T)
4 | | where
5 | |     for<'x> &'x T: Rem<&'x T, Output = T>,
6 | | {
7 | |     fbase % modulus;
8 | | }
  | |_^
note: ...so that types are compatible (expected std::ops::Rem, found std::ops::Rem<&T>)
 --> src/main.rs:7:11
  |
7 |     fbase % modulus;
  |           ^

Some poking around found some workarounds:

  • If we say that the input references can be unified to the same
    lifetime, it works:

    fn powm<'a, T>(fbase: &'a T, modulus: &'a T)
  • If we say that 'b outlives 'a, it works:

    fn powm<'a, 'b: 'a, T>(fbase: &'a T, modulus: &'b T)
  • If we say that we can have two different lifetimes in the operator, it works:

    for<'x, 'y> &'x T: Rem<&'y T, Output = T>,
  • If we directly call the Rem::rem method, it works:

    Rem::rem(fbase, modulus);
  • If we dereference and re-reference, it works:

    &*fbase % &*modulus;

I poked @eddyb for further explanation why the original case didn't work. Our tidied IRC conversation:

@eddyb
The answer is Rem::rem(fbase, modulus); doesn't do unification, it does a reborrow coercion.
That is, it's equivalent to &*fbase % &*modulus
You can figure this out by having an operator implemented with &str on the RHS and passing &String or other stuff like that.
If it works with &String, then this is generally approved

@shepmaster
But what's the difference of a % b and Rem::rem(a, b) — I thought that was straight desugaring?

@eddyb
There is no such thing as a straight desugaring; the first is Rem::rem(a, b) the second is (Rem::rem)(coerce a, coerce b)
IDK, I think we should coerce, but we can't coerce the LHS, because......... what type to coerce to?
It has to be asymmetrical. "has to"
Wait, no, why is this a... I might be wrong; yeah no I am wrong.

@eddyb
This is about traits... so Rem::rem(a, b) is <_ as Rem<_>>::rem(a, b) and a % b is <typeof a as Rem<typeof b>>::rem(a, b)
So we do coercions on the RHS, but I think we need to also subtype the LHS, which we're not doing right now.

@shepmaster
There's some piece of "this doesn't work because the lifetimes are disjoint, even though they could be unified at least as much as the lifetime of the function call"

@eddyb
It's not really disjointness; hint: an asymmetrical 'b: 'a or whatever you added only helps because the RHS is getting coerced. The RHS can be a subtype but the LHS is fixed.
Lemme rephrase :D
<typeof a as Rem<_>>::rem(a, b) whereas you want _ extends (typeof a) or something as the Self for Rem

@eddyb
We already use forced subtyping to have more flexibility around unsizing coercions because the trait matching of T: Unsize<U> is invariant.
It wasn't invariant for a while, which was a soundness bug..... introduced by yours truly in the original implementation.

@eddyb
This shows lhs_ty being used for Self and rhs_ty_var for the trait parameters so the LHS type is fixed instead of being subtyped.

@eddyb
This is where we do the right thing, going through an inference variable & extra subtyping (or LUB but that doesn't apply here) to make something more flexible than what the trait system allows.

@eddyb eddyb changed the title LHS type is fixed instead of being subtyped Binary operator LHS type is fixed instead of being subtyped Oct 21, 2017
@eddyb eddyb self-assigned this Oct 21, 2017
@bluss
Copy link
Member

bluss commented Oct 21, 2017

Old issue that duplicates this one #32008

bors added a commit that referenced this issue Nov 1, 2017
rustc_typeck: use subtyping on the LHS of binops.

Fixes #45425.

r? @nikomatsakis
jturner314 added a commit to jturner314/ndarray that referenced this issue May 10, 2018
Rust 1.23 fixed the original issue (rust-ndarray#103) that required `'b` to be
added. (See rust-lang/rust#32008, rust-lang/rust#45425, and
rust-lang/rust#45435 for the relevant Rust issues/PRs.)
jturner314 added a commit to jturner314/ndarray that referenced this issue Jun 13, 2018
Rust 1.23 fixed the original issue (rust-ndarray#103) that required `'b` to be
added. (See rust-lang/rust#32008, rust-lang/rust#45425, and
rust-lang/rust#45435 for the relevant Rust issues/PRs.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants