Skip to content

Commit

Permalink
Rollup merge of #72280 - nbdd0121:typeck, r=nikomatsakis
Browse files Browse the repository at this point in the history
Fix up autoderef when reborrowing

Currently `(f)()` and `f.call_mut()` behaves differently if expression `f` contains autoderef in it. This causes a weird error in #72225.

When `f` is type checked, `Deref` is used (this is expected as we can't yet determine if we should use `Fn` or `FnMut`). When subsequently we determine the actual trait to be used, when using the `f.call_mut()` syntax the `Deref` is patched to `DerefMut`, while for the `(f)()` syntax case it is not.

This PR replicates the fixup for the first case.

Fixes #72225
Fixes #68590
  • Loading branch information
RalfJung committed Jun 19, 2020
2 parents 63b441a + 2b7d858 commit 70622db
Show file tree
Hide file tree
Showing 10 changed files with 467 additions and 392 deletions.
12 changes: 5 additions & 7 deletions src/librustc_typeck/check/autoderef.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::method::MethodCallee;
use super::{FnCtxt, Needs, PlaceOp};
use super::{FnCtxt, PlaceOp};

use rustc_errors::struct_span_err;
use rustc_hir as hir;
Expand Down Expand Up @@ -170,14 +170,13 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
}

/// Returns the adjustment steps.
pub fn adjust_steps(&self, fcx: &FnCtxt<'a, 'tcx>, needs: Needs) -> Vec<Adjustment<'tcx>> {
fcx.register_infer_ok_obligations(self.adjust_steps_as_infer_ok(fcx, needs))
pub fn adjust_steps(&self, fcx: &FnCtxt<'a, 'tcx>) -> Vec<Adjustment<'tcx>> {
fcx.register_infer_ok_obligations(self.adjust_steps_as_infer_ok(fcx))
}

pub fn adjust_steps_as_infer_ok(
&self,
fcx: &FnCtxt<'a, 'tcx>,
needs: Needs,
) -> InferOk<'tcx, Vec<Adjustment<'tcx>>> {
let mut obligations = vec![];
let targets = self.steps.iter().skip(1).map(|&(ty, _)| ty).chain(iter::once(self.cur_ty));
Expand All @@ -186,7 +185,7 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
.iter()
.map(|&(source, kind)| {
if let AutoderefKind::Overloaded = kind {
fcx.try_overloaded_deref(self.span, source, needs).and_then(
fcx.try_overloaded_deref(self.span, source).and_then(
|InferOk { value: method, obligations: o }| {
obligations.extend(o);
if let ty::Ref(region, _, mutbl) = method.sig.output().kind {
Expand Down Expand Up @@ -266,8 +265,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
span: Span,
base_ty: Ty<'tcx>,
needs: Needs,
) -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
self.try_overloaded_place_op(span, base_ty, &[], needs, PlaceOp::Deref)
self.try_overloaded_place_op(span, base_ty, &[], PlaceOp::Deref)
}
}
45 changes: 26 additions & 19 deletions src/librustc_typeck/check/callee.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::autoderef::Autoderef;
use super::method::MethodCallee;
use super::{Expectation, FnCtxt, Needs, TupleArgumentsFlag};
use super::{Expectation, FnCtxt, TupleArgumentsFlag};
use crate::type_error_struct;

use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
Expand Down Expand Up @@ -115,7 +115,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// If the callee is a bare function or a closure, then we're all set.
match adjusted_ty.kind {
ty::FnDef(..) | ty::FnPtr(_) => {
let adjustments = autoderef.adjust_steps(self, Needs::None);
let adjustments = autoderef.adjust_steps(self);
self.apply_adjustments(callee_expr, adjustments);
return Some(CallStep::Builtin(adjusted_ty));
}
Expand All @@ -135,7 +135,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&closure_sig,
)
.0;
let adjustments = autoderef.adjust_steps(self, Needs::None);
let adjustments = autoderef.adjust_steps(self);
self.record_deferred_call_resolution(
def_id,
DeferredCallResolution {
Expand Down Expand Up @@ -176,7 +176,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.try_overloaded_call_traits(call_expr, adjusted_ty, Some(arg_exprs))
.or_else(|| self.try_overloaded_call_traits(call_expr, adjusted_ty, None))
.map(|(autoref, method)| {
let mut adjustments = autoderef.adjust_steps(self, Needs::None);
let mut adjustments = autoderef.adjust_steps(self);
adjustments.extend(autoref);
self.apply_adjustments(callee_expr, adjustments);
CallStep::Overloaded(method)
Expand Down Expand Up @@ -220,21 +220,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let method = self.register_infer_ok_obligations(ok);
let mut autoref = None;
if borrow {
if let ty::Ref(region, _, mutbl) = method.sig.inputs()[0].kind {
let mutbl = match mutbl {
hir::Mutability::Not => AutoBorrowMutability::Not,
hir::Mutability::Mut => AutoBorrowMutability::Mut {
// For initial two-phase borrow
// deployment, conservatively omit
// overloaded function call ops.
allow_two_phase_borrow: AllowTwoPhase::No,
},
};
autoref = Some(Adjustment {
kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)),
target: method.sig.inputs()[0],
});
}
// Check for &self vs &mut self in the method signature. Since this is either
// the Fn or FnMut trait, it should be one of those.
let (region, mutbl) = if let ty::Ref(r, _, mutbl) = method.sig.inputs()[0].kind
{
(r, mutbl)
} else {
span_bug!(call_expr.span, "input to call/call_mut is not a ref?");
};

let mutbl = match mutbl {
hir::Mutability::Not => AutoBorrowMutability::Not,
hir::Mutability::Mut => AutoBorrowMutability::Mut {
// For initial two-phase borrow
// deployment, conservatively omit
// overloaded function call ops.
allow_two_phase_borrow: AllowTwoPhase::No,
},
};
autoref = Some(Adjustment {
kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)),
target: method.sig.inputs()[0],
});
}
return Some((autoref, method));
}
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
//! we may want to adjust precisely when coercions occur.

use crate::astconv::AstConv;
use crate::check::{FnCtxt, Needs};
use crate::check::FnCtxt;
use rustc_errors::{struct_span_err, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
Expand Down Expand Up @@ -421,9 +421,8 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
return success(vec![], ty, obligations);
}

let needs = Needs::maybe_mut_place(mutbl_b);
let InferOk { value: mut adjustments, obligations: o } =
autoderef.adjust_steps_as_infer_ok(self, needs);
autoderef.adjust_steps_as_infer_ok(self);
obligations.extend(o);
obligations.extend(autoderef.into_obligations());

Expand Down
87 changes: 30 additions & 57 deletions src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ use rustc_hir::{ExprKind, QPath};
use rustc_infer::infer;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_middle::ty;
use rustc_middle::ty::adjustment::{
Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability,
};
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AllowTwoPhase};
use rustc_middle::ty::Ty;
use rustc_middle::ty::TypeFoldable;
use rustc_middle::ty::{AdtKind, Visibility};
Expand Down Expand Up @@ -113,12 +111,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.check_expr_with_expectation(expr, ExpectHasType(expected))
}

pub(super) fn check_expr_with_expectation(
fn check_expr_with_expectation_and_needs(
&self,
expr: &'tcx hir::Expr<'tcx>,
expected: Expectation<'tcx>,
needs: Needs,
) -> Ty<'tcx> {
self.check_expr_with_expectation_and_needs(expr, expected, Needs::None)
let ty = self.check_expr_with_expectation(expr, expected);

// If the expression is used in a place whether mutable place is required
// e.g. LHS of assignment, perform the conversion.
if let Needs::MutPlace = needs {
self.convert_place_derefs_to_mutable(expr);
}

ty
}

pub(super) fn check_expr(&self, expr: &'tcx hir::Expr<'tcx>) -> Ty<'tcx> {
Expand All @@ -143,11 +150,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Note that inspecting a type's structure *directly* may expose the fact
/// that there are actually multiple representations for `Error`, so avoid
/// that when err needs to be handled differently.
fn check_expr_with_expectation_and_needs(
pub(super) fn check_expr_with_expectation(
&self,
expr: &'tcx hir::Expr<'tcx>,
expected: Expectation<'tcx>,
needs: Needs,
) -> Ty<'tcx> {
debug!(">> type-checking: expr={:?} expected={:?}", expr, expected);

Expand All @@ -171,7 +177,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let old_diverges = self.diverges.replace(Diverges::Maybe);
let old_has_errors = self.has_errors.replace(false);

let ty = self.check_expr_kind(expr, expected, needs);
let ty = self.check_expr_kind(expr, expected);

// Warn for non-block expressions with diverging children.
match expr.kind {
Expand Down Expand Up @@ -213,9 +219,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
expr: &'tcx hir::Expr<'tcx>,
expected: Expectation<'tcx>,
needs: Needs,
) -> Ty<'tcx> {
debug!("check_expr_kind(expr={:?}, expected={:?}, needs={:?})", expr, expected, needs,);
debug!("check_expr_kind(expr={:?}, expected={:?})", expr, expected);

let tcx = self.tcx;
match expr.kind {
Expand All @@ -226,9 +231,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.check_expr_assign(expr, expected, lhs, rhs, span)
}
ExprKind::AssignOp(op, ref lhs, ref rhs) => self.check_binop_assign(expr, op, lhs, rhs),
ExprKind::Unary(unop, ref oprnd) => {
self.check_expr_unary(unop, oprnd, expected, needs, expr)
}
ExprKind::Unary(unop, ref oprnd) => self.check_expr_unary(unop, oprnd, expected, expr),
ExprKind::AddrOf(kind, mutbl, ref oprnd) => {
self.check_expr_addr_of(kind, mutbl, oprnd, expected, expr)
}
Expand Down Expand Up @@ -264,7 +267,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ExprKind::Block(ref body, _) => self.check_block_with_expected(&body, expected),
ExprKind::Call(ref callee, ref args) => self.check_call(expr, &callee, args, expected),
ExprKind::MethodCall(ref segment, span, ref args, _) => {
self.check_method_call(expr, segment, span, args, expected, needs)
self.check_method_call(expr, segment, span, args, expected)
}
ExprKind::Cast(ref e, ref t) => self.check_expr_cast(e, t, expr),
ExprKind::Type(ref e, ref t) => {
Expand All @@ -281,8 +284,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ExprKind::Struct(ref qpath, fields, ref base_expr) => {
self.check_expr_struct(expr, expected, qpath, fields, base_expr)
}
ExprKind::Field(ref base, field) => self.check_field(expr, needs, &base, field),
ExprKind::Index(ref base, ref idx) => self.check_expr_index(base, idx, needs, expr),
ExprKind::Field(ref base, field) => self.check_field(expr, &base, field),
ExprKind::Index(ref base, ref idx) => self.check_expr_index(base, idx, expr),
ExprKind::Yield(ref value, ref src) => self.check_expr_yield(value, expr, src),
hir::ExprKind::Err => tcx.ty_error(),
}
Expand All @@ -302,48 +305,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
unop: hir::UnOp,
oprnd: &'tcx hir::Expr<'tcx>,
expected: Expectation<'tcx>,
needs: Needs,
expr: &'tcx hir::Expr<'tcx>,
) -> Ty<'tcx> {
let tcx = self.tcx;
let expected_inner = match unop {
hir::UnOp::UnNot | hir::UnOp::UnNeg => expected,
hir::UnOp::UnDeref => NoExpectation,
};
let needs = match unop {
hir::UnOp::UnDeref => needs,
_ => Needs::None,
};
let mut oprnd_t = self.check_expr_with_expectation_and_needs(&oprnd, expected_inner, needs);
let mut oprnd_t = self.check_expr_with_expectation(&oprnd, expected_inner);

if !oprnd_t.references_error() {
oprnd_t = self.structurally_resolved_type(expr.span, oprnd_t);
match unop {
hir::UnOp::UnDeref => {
if let Some(mt) = oprnd_t.builtin_deref(true) {
oprnd_t = mt.ty;
} else if let Some(ok) = self.try_overloaded_deref(expr.span, oprnd_t, needs) {
let method = self.register_infer_ok_obligations(ok);
if let ty::Ref(region, _, mutbl) = method.sig.inputs()[0].kind {
let mutbl = match mutbl {
hir::Mutability::Not => AutoBorrowMutability::Not,
hir::Mutability::Mut => AutoBorrowMutability::Mut {
// (It shouldn't actually matter for unary ops whether
// we enable two-phase borrows or not, since a unary
// op has no additional operands.)
allow_two_phase_borrow: AllowTwoPhase::No,
},
};
self.apply_adjustments(
oprnd,
vec![Adjustment {
kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)),
target: method.sig.inputs()[0],
}],
);
}
oprnd_t = self.make_overloaded_place_return_type(method).ty;
self.write_method_call(expr.hir_id, method);
if let Some(ty) = self.lookup_derefing(expr, oprnd, oprnd_t) {
oprnd_t = ty;
} else {
let mut err = type_error_struct!(
tcx.sess,
Expand Down Expand Up @@ -405,8 +381,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => NoExpectation,
}
});
let needs = Needs::maybe_mut_place(mutbl);
let ty = self.check_expr_with_expectation_and_needs(&oprnd, hint, needs);
let ty =
self.check_expr_with_expectation_and_needs(&oprnd, hint, Needs::maybe_mut_place(mutbl));

let tm = ty::TypeAndMut { ty, mutbl };
match kind {
Expand Down Expand Up @@ -861,10 +837,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span: Span,
args: &'tcx [hir::Expr<'tcx>],
expected: Expectation<'tcx>,
needs: Needs,
) -> Ty<'tcx> {
let rcvr = &args[0];
let rcvr_t = self.check_expr_with_needs(&rcvr, needs);
let rcvr_t = self.check_expr(&rcvr);
// no need to check for bot/err -- callee does that
let rcvr_t = self.structurally_resolved_type(args[0].span, rcvr_t);

Expand Down Expand Up @@ -1443,11 +1418,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn check_field(
&self,
expr: &'tcx hir::Expr<'tcx>,
needs: Needs,
base: &'tcx hir::Expr<'tcx>,
field: Ident,
) -> Ty<'tcx> {
let expr_t = self.check_expr_with_needs(base, needs);
let expr_t = self.check_expr(base);
let expr_t = self.structurally_resolved_type(base.span, expr_t);
let mut private_candidate = None;
let mut autoderef = self.autoderef(expr.span, expr_t);
Expand All @@ -1467,7 +1441,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// of error recovery.
self.write_field_index(expr.hir_id, index);
if field.vis.is_accessible_from(def_scope, self.tcx) {
let adjustments = autoderef.adjust_steps(self, needs);
let adjustments = autoderef.adjust_steps(self);
self.apply_adjustments(base, adjustments);
autoderef.finalize(self);

Expand All @@ -1482,7 +1456,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Ok(index) = fstr.parse::<usize>() {
if fstr == index.to_string() {
if let Some(field_ty) = tys.get(index) {
let adjustments = autoderef.adjust_steps(self, needs);
let adjustments = autoderef.adjust_steps(self);
self.apply_adjustments(base, adjustments);
autoderef.finalize(self);

Expand Down Expand Up @@ -1721,10 +1695,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
base: &'tcx hir::Expr<'tcx>,
idx: &'tcx hir::Expr<'tcx>,
needs: Needs,
expr: &'tcx hir::Expr<'tcx>,
) -> Ty<'tcx> {
let base_t = self.check_expr_with_needs(&base, needs);
let base_t = self.check_expr(&base);
let idx_t = self.check_expr(&idx);

if base_t.references_error() {
Expand All @@ -1733,7 +1706,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
idx_t
} else {
let base_t = self.structurally_resolved_type(base.span, base_t);
match self.lookup_indexing(expr, base, base_t, idx_t, needs) {
match self.lookup_indexing(expr, base, base_t, idx_t) {
Some((index_ty, element_ty)) => {
// two-phase not needed because index_ty is never mutable
self.demand_coerce(idx, idx_t, index_ty, None, AllowTwoPhase::No);
Expand Down
Loading

0 comments on commit 70622db

Please sign in to comment.