Skip to content

Commit

Permalink
Auto merge of #45435 - eddyb:binop-subtype-lhs, r=nikomatsakis
Browse files Browse the repository at this point in the history
rustc_typeck: use subtyping on the LHS of binops.

Fixes #45425.

r? @nikomatsakis
  • Loading branch information
bors committed Nov 1, 2017
2 parents 7402866 + 1a7fb7d commit 2f581cf
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 74 deletions.
105 changes: 57 additions & 48 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
}

pub fn demand_coerce(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>, expected: Ty<'tcx>) {
if let Some(mut err) = self.demand_coerce_diag(expr, checked_ty, expected) {
pub fn demand_coerce(&self,
expr: &hir::Expr,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>)
-> Ty<'tcx> {
let (ty, err) = self.demand_coerce_diag(expr, checked_ty, expected);
if let Some(mut err) = err {
err.emit();
}
ty
}

// Checks that the type of `expr` can be coerced to `expected`.
Expand All @@ -88,61 +94,64 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
pub fn demand_coerce_diag(&self,
expr: &hir::Expr,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>) -> Option<DiagnosticBuilder<'tcx>> {
expected: Ty<'tcx>)
-> (Ty<'tcx>, Option<DiagnosticBuilder<'tcx>>) {
let expected = self.resolve_type_vars_with_obligations(expected);

if let Err(e) = self.try_coerce(expr, checked_ty, self.diverges.get(), expected) {
let cause = self.misc(expr.span);
let expr_ty = self.resolve_type_vars_with_obligations(checked_ty);
let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e);
let e = match self.try_coerce(expr, checked_ty, self.diverges.get(), expected) {
Ok(ty) => return (ty, None),
Err(e) => e
};

// If the expected type is an enum with any variants whose sole
// field is of the found type, suggest such variants. See Issue
// #42764.
if let ty::TyAdt(expected_adt, substs) = expected.sty {
let mut compatible_variants = vec![];
for variant in &expected_adt.variants {
if variant.fields.len() == 1 {
let sole_field = &variant.fields[0];
let sole_field_ty = sole_field.ty(self.tcx, substs);
if self.can_coerce(expr_ty, sole_field_ty) {
let mut variant_path = self.tcx.item_path_str(variant.did);
variant_path = variant_path.trim_left_matches("std::prelude::v1::")
.to_string();
compatible_variants.push(variant_path);
}
let cause = self.misc(expr.span);
let expr_ty = self.resolve_type_vars_with_obligations(checked_ty);
let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e);

// If the expected type is an enum with any variants whose sole
// field is of the found type, suggest such variants. See Issue
// #42764.
if let ty::TyAdt(expected_adt, substs) = expected.sty {
let mut compatible_variants = vec![];
for variant in &expected_adt.variants {
if variant.fields.len() == 1 {
let sole_field = &variant.fields[0];
let sole_field_ty = sole_field.ty(self.tcx, substs);
if self.can_coerce(expr_ty, sole_field_ty) {
let mut variant_path = self.tcx.item_path_str(variant.did);
variant_path = variant_path.trim_left_matches("std::prelude::v1::")
.to_string();
compatible_variants.push(variant_path);
}
}
if !compatible_variants.is_empty() {
let expr_text = print::to_string(print::NO_ANN, |s| s.print_expr(expr));
let suggestions = compatible_variants.iter()
.map(|v| format!("{}({})", v, expr_text)).collect::<Vec<_>>();
err.span_suggestions(expr.span,
"try using a variant of the expected type",
suggestions);
}
}
if !compatible_variants.is_empty() {
let expr_text = print::to_string(print::NO_ANN, |s| s.print_expr(expr));
let suggestions = compatible_variants.iter()
.map(|v| format!("{}({})", v, expr_text)).collect::<Vec<_>>();
err.span_suggestions(expr.span,
"try using a variant of the expected type",
suggestions);
}
}

if let Some(suggestion) = self.check_ref(expr,
checked_ty,
expected) {
err.help(&suggestion);
} else {
let mode = probe::Mode::MethodCall;
let suggestions = self.probe_for_return_type(syntax_pos::DUMMY_SP,
mode,
expected,
checked_ty,
ast::DUMMY_NODE_ID);
if suggestions.len() > 0 {
err.help(&format!("here are some functions which \
might fulfill your needs:\n{}",
self.get_best_match(&suggestions).join("\n")));
}
if let Some(suggestion) = self.check_ref(expr,
checked_ty,
expected) {
err.help(&suggestion);
} else {
let mode = probe::Mode::MethodCall;
let suggestions = self.probe_for_return_type(syntax_pos::DUMMY_SP,
mode,
expected,
checked_ty,
ast::DUMMY_NODE_ID);
if suggestions.len() > 0 {
err.help(&format!("here are some functions which \
might fulfill your needs:\n{}",
self.get_best_match(&suggestions).join("\n")));
}
return Some(err);
}
None
(expected, Some(err))
}

fn format_method_suggestion(&self, method: &AssociatedItem) -> String {
Expand Down
16 changes: 13 additions & 3 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2755,9 +2755,19 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
fn check_expr_coercable_to_type(&self,
expr: &'gcx hir::Expr,
expected: Ty<'tcx>) -> Ty<'tcx> {
let ty = self.check_expr_with_hint(expr, expected);
self.demand_coerce(expr, ty, expected);
ty
self.check_expr_coercable_to_type_with_lvalue_pref(expr, expected, NoPreference)
}

fn check_expr_coercable_to_type_with_lvalue_pref(&self,
expr: &'gcx hir::Expr,
expected: Ty<'tcx>,
lvalue_pref: LvaluePreference)
-> Ty<'tcx> {
let ty = self.check_expr_with_expectation_and_lvalue_pref(
expr,
ExpectHasType(expected),
lvalue_pref);
self.demand_coerce(expr, ty, expected)
}

fn check_expr_with_hint(&self, expr: &'gcx hir::Expr,
Expand Down
48 changes: 27 additions & 21 deletions src/librustc_typeck/check/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

use super::FnCtxt;
use super::method::MethodCallee;
use rustc::ty::{self, Ty, TypeFoldable, PreferMutLvalue, TypeVariants};
use rustc::ty::{self, Ty, TypeFoldable, NoPreference, PreferMutLvalue, TypeVariants};
use rustc::ty::TypeVariants::{TyStr, TyRef};
use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow};
use rustc::infer::type_variable::TypeVariableOrigin;
Expand All @@ -29,12 +29,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
lhs_expr: &'gcx hir::Expr,
rhs_expr: &'gcx hir::Expr) -> Ty<'tcx>
{
let lhs_ty = self.check_expr_with_lvalue_pref(lhs_expr, PreferMutLvalue);

let lhs_ty = self.resolve_type_vars_with_obligations(lhs_ty);
let (rhs_ty, return_ty) =
self.check_overloaded_binop(expr, lhs_expr, lhs_ty, rhs_expr, op, IsAssign::Yes);
let rhs_ty = self.resolve_type_vars_with_obligations(rhs_ty);
let (lhs_ty, rhs_ty, return_ty) =
self.check_overloaded_binop(expr, lhs_expr, rhs_expr, op, IsAssign::Yes);

let ty = if !lhs_ty.is_ty_var() && !rhs_ty.is_ty_var()
&& is_builtin_binop(lhs_ty, rhs_ty, op) {
Expand Down Expand Up @@ -73,27 +69,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
lhs_expr,
rhs_expr);

let lhs_ty = self.check_expr(lhs_expr);
let lhs_ty = self.resolve_type_vars_with_obligations(lhs_ty);

match BinOpCategory::from(op) {
BinOpCategory::Shortcircuit => {
// && and || are a simple case.
self.check_expr_coercable_to_type(lhs_expr, tcx.types.bool);
let lhs_diverges = self.diverges.get();
self.demand_suptype(lhs_expr.span, tcx.mk_bool(), lhs_ty);
self.check_expr_coercable_to_type(rhs_expr, tcx.mk_bool());
self.check_expr_coercable_to_type(rhs_expr, tcx.types.bool);

// Depending on the LHS' value, the RHS can never execute.
self.diverges.set(lhs_diverges);

tcx.mk_bool()
tcx.types.bool
}
_ => {
// Otherwise, we always treat operators as if they are
// overloaded. This is the way to be most flexible w/r/t
// types that get inferred.
let (rhs_ty, return_ty) =
self.check_overloaded_binop(expr, lhs_expr, lhs_ty,
let (lhs_ty, rhs_ty, return_ty) =
self.check_overloaded_binop(expr, lhs_expr,
rhs_expr, op, IsAssign::No);

// Supply type inference hints if relevant. Probably these
Expand All @@ -108,7 +101,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
// deduce that the result type should be `u32`, even
// though we don't know yet what type 2 has and hence
// can't pin this down to a specific impl.
let rhs_ty = self.resolve_type_vars_with_obligations(rhs_ty);
if
!lhs_ty.is_ty_var() && !rhs_ty.is_ty_var() &&
is_builtin_binop(lhs_ty, rhs_ty, op)
Expand Down Expand Up @@ -164,17 +156,30 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
fn check_overloaded_binop(&self,
expr: &'gcx hir::Expr,
lhs_expr: &'gcx hir::Expr,
lhs_ty: Ty<'tcx>,
rhs_expr: &'gcx hir::Expr,
op: hir::BinOp,
is_assign: IsAssign)
-> (Ty<'tcx>, Ty<'tcx>)
-> (Ty<'tcx>, Ty<'tcx>, Ty<'tcx>)
{
debug!("check_overloaded_binop(expr.id={}, lhs_ty={:?}, is_assign={:?})",
debug!("check_overloaded_binop(expr.id={}, op={:?}, is_assign={:?})",
expr.id,
lhs_ty,
op,
is_assign);

let lhs_pref = match is_assign {
IsAssign::Yes => PreferMutLvalue,
IsAssign::No => NoPreference
};
// Find a suitable supertype of the LHS expression's type, by coercing to
// a type variable, to pass as the `Self` to the trait, avoiding invariant
// trait matching creating lifetime constraints that are too strict.
// E.g. adding `&'a T` and `&'b T`, given `&'x T: Add<&'x T>`, will result
// in `&'a T <: &'x T` and `&'b T <: &'x T`, instead of `'a = 'b = 'x`.
let lhs_ty = self.check_expr_coercable_to_type_with_lvalue_pref(lhs_expr,
self.next_ty_var(TypeVariableOrigin::MiscVariable(lhs_expr.span)),
lhs_pref);
let lhs_ty = self.resolve_type_vars_with_obligations(lhs_ty);

// NB: As we have not yet type-checked the RHS, we don't have the
// type at hand. Make a variable to represent it. The whole reason
// for this indirection is so that, below, we can check the expr
Expand All @@ -187,6 +192,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {

// see `NB` above
let rhs_ty = self.check_expr_coercable_to_type(rhs_expr, rhs_ty_var);
let rhs_ty = self.resolve_type_vars_with_obligations(rhs_ty);

let return_ty = match result {
Ok(method) => {
Expand Down Expand Up @@ -296,7 +302,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
};

(rhs_ty_var, return_ty)
(lhs_ty, rhs_ty, return_ty)
}

fn check_str_addition(&self,
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-41394.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

enum Foo {
A = "" + 1
//~^ ERROR binary operation `+` cannot be applied to type `&'static str`
//~^ ERROR binary operation `+` cannot be applied to type `&str`
}

enum Bar {
Expand Down
2 changes: 2 additions & 0 deletions src/test/run-fail/binop-fail-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
fn foo() -> ! {
panic!("quux");
}

#[allow(resolve_trait_on_defaulted_unit)]
fn main() {
foo() == foo(); // these types wind up being defaulted to ()
}
35 changes: 35 additions & 0 deletions src/test/run-pass/issue-32008.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Tests that binary operators allow subtyping on both the LHS and RHS,
// and as such do not introduce unnecesarily strict lifetime constraints.

use std::ops::Add;

struct Foo;

impl<'a> Add<&'a Foo> for &'a Foo {
type Output = ();
fn add(self, rhs: &'a Foo) {}
}

fn try_to_add(input: &Foo) {
let local = Foo;

// Manual reborrow worked even with invariant trait search.
&*input + &local;

// Direct use of the reference on the LHS requires additional
// subtyping before searching (invariantly) for `LHS: Add<RHS>`.
input + &local;
}

fn main() {
}
20 changes: 20 additions & 0 deletions src/test/run-pass/issue-45425.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::ops::Add;

fn ref_add<T>(a: &T, b: &T) -> T
where
for<'x> &'x T: Add<&'x T, Output = T>,
{
a + b
}

fn main() {}
2 changes: 1 addition & 1 deletion src/test/ui/span/issue-39018.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0369]: binary operation `+` cannot be applied to type `&'static str`
error[E0369]: binary operation `+` cannot be applied to type `&str`
--> $DIR/issue-39018.rs:12:13
|
12 | let x = "Hello " + "World!";
Expand Down

0 comments on commit 2f581cf

Please sign in to comment.