rust-lang/rust — pull request #19434 by japaric (2014-12-01 05:06:30+00:00)
Title: [WIP] Don't rely on the type of `lhs` when type checking `lhs OP rhs`

Tweaks the binary operator dispatch algorithm to not rely on the full resolution of the type of the LHS.

Closes #8280
Closes #19035

---

This is an overview of the current type checking algorithm vs the new algorithm introduced in this PR:

### Current

- Check `lhs`
* (i.e. `check_expr(lhs)`)
- Fully resolve the type of `lhs` (**compiler error** if can't fully resolve)
* (i.e. `lhs_ty = structurally_resolved_type(lhs)`)
* This is the cause of #8280
- If `lhs_ty` is integer and `OP` is bit-shift
- Enforce that `rhs_ty` is a subtype of `uint`
- Categorize `lhs_ty` and `OP`, if `lhs_ty OP lhs_ty` is a built-in operation
* (i.e. `is_binopable(lhs_ty, op)` returns true)
* This is the cause of #19035
- Watch out for SIMD comparison, make `rhs_ty` a subtype of `lhs_ty` and "write" the type of `lhs OP rhs` expression
- Raise compile error if OP is `&&` or `||`
- Otherwise operator is overloaded, proceed with trait dispatch

### New (this PR)

- Check *both* `lhs` and `rhs`
* (i.e. `check_expr(lhs|rhs) `)
- *Try* to fully resolve `lhs` *and* `rhs`
- If both got fully resolved
* (i.e. iff both `try_structurally_resolved_type(lhs|rhs)` return `Some`)
- If *both* `lhs_ty` and `rhs_ty` are integers and `OP` is bit-shift
- Enforce that `rhs_ty` is a subtype of `uint`
- Categorize `lhs_ty`, `rhs_ty` and `OP`, if `lhs_ty` *and* `rhs_ty` have the same category and `lhs OP rhs` is a built-in operation
* (i.e. `is_builtin_binop(cx, lhs_ty, op)` returns true)
- Watch out for SIMD comparison and write type of `lhs OP rhs` expression
- Raise compile error if OP is `&&` or `||`
- Otherwise operator is overloaded, proceed with trait dispatch

---

This PR passes `make check` as it is, but it comes with two new issues:

- The "reborrow adjustment" trick we use to make `&'a T == &'b T` work even though only `&'a T == &'a T` (*note* same lifetime `'a`) should work ... well, it no longer works (or it doesn't work in most cases). However, this is not a real problem, because with #19167 (overloaded `==` operator), we can now `impl PartialEq<&'b T> for &'a T` to take care references of with different lifetimes.

- The most aggravating issue is an increase in the verbosity and decrease in the quality of the error messages related to binary operators. Consider the following code:

``` rust
struct Foo;

#[deriving(PartialEq)]
struct Bar(Foo);

fn main() {}
```

Which now generates the following error message:

``` rust
deriving.rs:4:12: 4:16 error: binary operation `==` cannot be applied to type `Foo`
deriving.rs:4 struct Bar(Foo);
^~~~
note: in expansion of #[deriving]
deriving.rs:3:1: 3:23 note: expansion site
deriving.rs:4:12: 4:16 error: binary operation `&&` cannot be applied to type `bool`
deriving.rs:4 struct Bar(Foo);
^~~~
note: in expansion of #[deriving]
deriving.rs:3:1: 3:23 note: expansion site
deriving.rs:4:12: 4:16 error: binary operation `==` cannot be applied to type `Foo`
deriving.rs:4 struct Bar(Foo);
^~~~
note: in expansion of #[deriving]
deriving.rs:3:1: 3:23 note: expansion site
deriving.rs:4:12: 4:16 error: binary operation `!=` cannot be applied to type `Foo`
deriving.rs:4 struct Bar(Foo);
^~~~
note: in expansion of #[deriving]
deriving.rs:3:1: 3:23 note: expansion site
deriving.rs:4:12: 4:16 "
deriving.rs:4 struct Bar(Foo);
^~~~
note: in expansion of #[deriving]
deriving.rs:3:1: 3:23 note: expansion site
deriving.rs:4:12: 4:16 error: binary operation `!=` cannot be applied to type `Foo`
deriving.rs:4 struct Bar(Foo);
^~~~
note: in expansion of #[deriving]
deriving.rs:3:1: 3:23 note: expansion site
error: aborting due to 6 previous errors
```

- The error message "binary operation `==` cannot be applied to type `Foo`" (and its `!=` brother) should appear only once.
- The error message "binary operation `==` cannot be applied to type `Foo`", although it sounds absurd, it's actually a real error, just that the message it's bad. That message it's actually referring to the operation `bool && [Type Error]` where `[Type Error]` is the type of `Foo == Foo` (which can't be resolved, since `Foo` doesn't implement `PartialEq`).

@nikomatsakis Thoughts? I think I can fix/improve the error messages, but I'd like to know what you think about the approach and whether the "reborrow adjustment" can/should be fixed or not.