From 48096583030582a73fe5f0dfafbf66f5a0b91807 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 27 Apr 2017 19:25:13 +0300 Subject: [PATCH] refactor the handling of lvalue ops Fixes #41604. --- src/librustc_typeck/check/autoderef.rs | 41 +---- src/librustc_typeck/check/callee.rs | 8 +- src/librustc_typeck/check/method/confirm.rs | 178 +++++++------------- src/librustc_typeck/check/method/mod.rs | 34 +--- src/librustc_typeck/check/mod.rs | 138 ++++++++++----- src/librustc_typeck/check/op.rs | 15 +- src/test/run-pass/issue-41604.rs | 20 +++ 7 files changed, 204 insertions(+), 230 deletions(-) create mode 100644 src/test/run-pass/issue-41604.rs diff --git a/src/librustc_typeck/check/autoderef.rs b/src/librustc_typeck/check/autoderef.rs index 92fb02c6379dc..c9584f1d9e1e0 100644 --- a/src/librustc_typeck/check/autoderef.rs +++ b/src/librustc_typeck/check/autoderef.rs @@ -10,7 +10,7 @@ use astconv::AstConv; -use super::FnCtxt; +use super::{FnCtxt, LvalueOp}; use check::coercion::AsCoercionSite; use rustc::infer::InferOk; @@ -18,7 +18,7 @@ use rustc::traits; use rustc::ty::{self, Ty, TraitRef}; use rustc::ty::{ToPredicate, TypeFoldable}; use rustc::ty::{MethodCall, MethodCallee}; -use rustc::ty::{LvaluePreference, NoPreference, PreferMutLvalue}; +use rustc::ty::{LvaluePreference, NoPreference}; use rustc::hir; use syntax_pos::Span; @@ -213,39 +213,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { span: Span, base_expr: Option<&hir::Expr>, base_ty: Ty<'tcx>, - lvalue_pref: LvaluePreference) + pref: LvaluePreference) -> Option>> { - debug!("try_overloaded_deref({:?},{:?},{:?},{:?})", - span, - base_expr, - base_ty, - lvalue_pref); - // Try DerefMut first, if preferred. - let method = match (lvalue_pref, self.tcx.lang_items.deref_mut_trait()) { - (PreferMutLvalue, Some(trait_did)) => { - self.lookup_method_in_trait(span, - base_expr, - Symbol::intern("deref_mut"), - trait_did, - base_ty, - None) - } - _ => None, - }; - - // Otherwise, fall back to Deref. - let method = match (method, self.tcx.lang_items.deref_trait()) { - (None, Some(trait_did)) => { - self.lookup_method_in_trait(span, - base_expr, - Symbol::intern("deref"), - trait_did, - base_ty, - None) - } - (method, _) => method, - }; + let rcvr = base_expr.map(|base_expr| super::AdjustedRcvr { + rcvr_expr: base_expr, autoderefs: 0, unsize: false + }); - method + self.try_overloaded_lvalue_op(span, rcvr, base_ty, &[], pref, LvalueOp::Deref) } } diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index 32f130aca1cb9..7a49309e005a1 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -165,11 +165,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }; match self.lookup_method_in_trait_adjusted(call_expr.span, - Some(&callee_expr), + Some(super::AdjustedRcvr { + rcvr_expr: callee_expr, + autoderefs, + unsize: false + }), method_name, trait_def_id, - autoderefs, - false, adjusted_ty, None) { None => continue, diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs index fb608b2494558..ddec54cc0926d 100644 --- a/src/librustc_typeck/check/method/confirm.rs +++ b/src/librustc_typeck/check/method/confirm.rs @@ -10,7 +10,7 @@ use super::probe; -use check::{FnCtxt, callee}; +use check::{FnCtxt, LvalueOp, callee}; use hir::def_id::DefId; use rustc::ty::subst::Substs; use rustc::traits; @@ -433,137 +433,81 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { for (i, &expr) in exprs.iter().rev().enumerate() { debug!("convert_lvalue_derefs_to_mutable: i={} expr={:?}", i, expr); - // Count autoderefs. We don't need to fix up the autoref - the parent - // expression will fix them up for us. - let adjustment = self.tables.borrow().adjustments.get(&expr.id).cloned(); - match adjustment { - Some(Adjustment { kind: Adjust::DerefRef { autoderefs, .. }, .. }) => { - if autoderefs > 0 { - let mut autoderef = self.autoderef(expr.span, self.node_ty(expr.id)); - autoderef.nth(autoderefs).unwrap_or_else(|| { - span_bug!(expr.span, - "expr was deref-able {} times but now isn't?", - autoderefs); - }); - autoderef.finalize(PreferMutLvalue, expr); + // Fix up the adjustment. + let autoderefs = match self.tables.borrow_mut().adjustments.get_mut(&expr.id) { + Some(&mut Adjustment { + kind: Adjust::DerefRef { autoderefs, ref mut autoref, .. }, ref mut target + }) => { + if let &mut Some(AutoBorrow::Ref(_, ref mut mutbl)) = autoref { + *mutbl = hir::Mutability::MutMutable; + *target = match target.sty { + ty::TyRef(r, ty::TypeAndMut { ty, .. }) => + self.tcx.mk_ref(r, ty::TypeAndMut { ty, mutbl: *mutbl }), + _ => span_bug!(expr.span, "AutoBorrow::Ref resulted in non-ref {:?}", + target) + }; } + autoderefs } - Some(_) | None => {} + Some(_) | None => 0 + }; + + if autoderefs > 0 { + let mut autoderef = self.autoderef(expr.span, self.node_ty(expr.id)); + autoderef.nth(autoderefs).unwrap_or_else(|| { + span_bug!(expr.span, + "expr was deref-able {} times but now isn't?", + autoderefs); + }); + autoderef.finalize(PreferMutLvalue, expr); } - // Don't retry the first one or we might infinite loop! - if i == 0 { - continue; - } match expr.node { hir::ExprIndex(ref base_expr, ref index_expr) => { - // If this is an overloaded index, the - // adjustment will include an extra layer of - // autoref because the method is an &self/&mut - // self method. We have to peel it off to get - // the raw adjustment that `try_index_step` - // expects. This is annoying and horrible. We - // ought to recode this routine so it doesn't - // (ab)use the normal type checking paths. - let adj = self.tables.borrow_mut().adjustments.remove(&base_expr.id); - let (autoderefs, unsize, adjusted_base_ty) = match adj { - Some(Adjustment { - kind: Adjust::DerefRef { autoderefs, autoref, unsize }, - target - }) => { - match autoref { - None => { - assert!(!unsize); - } - Some(AutoBorrow::Ref(..)) => {} - Some(_) => { - span_bug!(base_expr.span, - "unexpected adjustment autoref {:?}", - adj); - } - } - - (autoderefs, unsize, if unsize { - target.builtin_deref(false, NoPreference) - .expect("fixup: AutoBorrow::Ref is not &T") - .ty - } else { - let ty = self.node_ty(base_expr.id); - let mut ty = self.shallow_resolve(ty); - let mut method_type = |method_call: ty::MethodCall| { - self.tables.borrow().method_map.get(&method_call).map(|m| { - self.resolve_type_vars_if_possible(&m.ty) - }) - }; - - if !ty.references_error() { - for i in 0..autoderefs { - ty = ty.adjust_for_autoderef(self.tcx, - base_expr.id, - base_expr.span, - i as u32, - &mut method_type); - } - } - - ty - }) - } - None => (0, false, self.node_ty(base_expr.id)), - Some(_) => { - span_bug!(base_expr.span, "unexpected adjustment type"); - } - }; - let index_expr_ty = self.node_ty(index_expr.id); - let adjusted_base_ty = self.resolve_type_vars_if_possible(&adjusted_base_ty); - let index_expr_ty = self.resolve_type_vars_if_possible(&index_expr_ty); - - let result = self.try_index_step(ty::MethodCall::expr(expr.id), - expr, - &base_expr, - adjusted_base_ty, - autoderefs, - unsize, - PreferMutLvalue, - index_expr_ty); - - if let Some((input_ty, return_ty)) = result { - self.demand_suptype(index_expr.span, input_ty, index_expr_ty); - - let expr_ty = self.node_ty(expr.id); - self.demand_suptype(expr.span, expr_ty, return_ty); - } else { - // We could not perform a mutable index. Re-apply the - // immutable index adjustments - borrowck will detect - // this as an error. - if let Some(adjustment) = adjustment { - self.apply_adjustment(expr.id, adjustment); - } - self.tcx.sess.delay_span_bug( - expr.span, "convert_lvalue_derefs_to_mutable failed"); - } + self.convert_lvalue_op_to_mutable( + LvalueOp::Index, expr, base_expr, &[index_expr_ty]); } hir::ExprUnary(hir::UnDeref, ref base_expr) => { - // if this is an overloaded deref, then re-evaluate with - // a preference for mut - let method_call = ty::MethodCall::expr(expr.id); - if self.tables.borrow().method_map.contains_key(&method_call) { - self.tables.borrow_mut().adjustments.remove(&base_expr.id); - let method = self.try_overloaded_deref(expr.span, - Some(&base_expr), - self.node_ty(base_expr.id), - PreferMutLvalue); - let ok = method.expect("re-trying deref failed"); - let method = self.register_infer_ok_obligations(ok); - self.tables.borrow_mut().method_map.insert(method_call, method); - } + self.convert_lvalue_op_to_mutable( + LvalueOp::Deref, expr, base_expr, &[]); } _ => {} } } } + fn convert_lvalue_op_to_mutable(&self, + op: LvalueOp, + expr: &hir::Expr, + base_expr: &hir::Expr, + arg_tys: &[Ty<'tcx>]) + { + debug!("convert_lvalue_op_to_mutable({:?}, {:?}, {:?}, {:?})", + op, expr, base_expr, arg_tys); + let method_call = ty::MethodCall::expr(expr.id); + if !self.tables.borrow().method_map.contains_key(&method_call) { + debug!("convert_lvalue_op_to_mutable - builtin, nothing to do"); + return + } + + let base_ty = self.tables.borrow().adjustments.get(&base_expr.id) + .map_or_else(|| self.node_ty(expr.id), |adj| adj.target); + let base_ty = self.resolve_type_vars_if_possible(&base_ty); + + // Need to deref because overloaded lvalue ops take self by-reference. + let base_ty = base_ty.builtin_deref(false, NoPreference) + .expect("lvalue op takes something that is not a ref") + .ty; + + let method = self.try_overloaded_lvalue_op( + expr.span, None, base_ty, arg_tys, PreferMutLvalue, op); + let ok = method.expect("re-trying op failed"); + let method = self.register_infer_ok_obligations(ok); + debug!("convert_lvalue_op_to_mutable: method={:?}", method); + self.tables.borrow_mut().method_map.insert(method_call, method); + } + /////////////////////////////////////////////////////////////////////////// // MISCELLANY diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 7dd2699a6eaf0..ed04cabf3bd37 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -10,7 +10,7 @@ //! Method lookup: the secret sauce of Rust. See `README.md`. -use check::FnCtxt; +use check::{FnCtxt, AdjustedRcvr}; use hir::def::Def; use hir::def_id::DefId; use rustc::ty::subst::Substs; @@ -153,24 +153,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { supplied_method_types)) } - pub fn lookup_method_in_trait(&self, - span: Span, - self_expr: Option<&hir::Expr>, - m_name: ast::Name, - trait_def_id: DefId, - self_ty: ty::Ty<'tcx>, - opt_input_types: Option>>) - -> Option>> { - self.lookup_method_in_trait_adjusted(span, - self_expr, - m_name, - trait_def_id, - 0, - false, - self_ty, - opt_input_types) - } - /// `lookup_in_trait_adjusted` is used for overloaded operators. /// It does a very narrow slice of what the normal probe/confirm path does. /// In particular, it doesn't really do any probing: it simply constructs @@ -184,18 +166,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { /// this method is basically the same as confirmation. pub fn lookup_method_in_trait_adjusted(&self, span: Span, - self_expr: Option<&hir::Expr>, + self_info: Option, m_name: ast::Name, trait_def_id: DefId, - autoderefs: usize, - unsize: bool, self_ty: ty::Ty<'tcx>, opt_input_types: Option>>) -> Option>> { - debug!("lookup_in_trait_adjusted(self_ty={:?}, self_expr={:?}, \ + debug!("lookup_in_trait_adjusted(self_ty={:?}, self_info={:?}, \ m_name={}, trait_def_id={:?})", self_ty, - self_expr, + self_info, m_name, trait_def_id); @@ -288,10 +268,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { obligations.push(traits::Obligation::new(cause, ty::Predicate::WellFormed(method_ty))); // Insert any adjustments needed (always an autoref of some mutability). - if let Some(self_expr) = self_expr { + if let Some(AdjustedRcvr { rcvr_expr, autoderefs, unsize }) = self_info { debug!("lookup_in_trait_adjusted: inserting adjustment if needed \ (self-id={}, autoderefs={}, unsize={}, fty={:?})", - self_expr.id, autoderefs, unsize, original_method_ty); + rcvr_expr.id, autoderefs, unsize, original_method_ty); let original_sig = original_method_ty.fn_sig(); let autoref = match (&original_sig.input(0).skip_binder().sty, @@ -308,7 +288,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } }; - self.apply_adjustment(self_expr.id, Adjustment { + self.apply_adjustment(rcvr_expr.id, Adjustment { kind: Adjust::DerefRef { autoderefs: autoderefs, autoref: autoref, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 098e8c53a52c1..4c9ed7adceeae 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -361,6 +361,19 @@ impl UnsafetyState { } } +#[derive(Debug, Copy, Clone)] +pub enum LvalueOp { + Deref, + Index +} + +#[derive(Copy, Clone, Debug)] +pub struct AdjustedRcvr<'a> { + pub rcvr_expr: &'a hir::Expr, + pub autoderefs: usize, + pub unsize: bool +} + /// Tracks whether executing a node may exit normally (versus /// return/break/panic, which "diverge", leaving dead code in their /// wake). Tracked semi-automatically (through type variables marked @@ -2129,9 +2142,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { while let Some((adj_ty, autoderefs)) = autoderef.next() { if let Some(final_mt) = self.try_index_step( - MethodCall::expr(expr.id), - expr, base_expr, adj_ty, autoderefs, - false, lvalue_pref, idx_ty) + MethodCall::expr(expr.id), expr, Some(AdjustedRcvr { + rcvr_expr: base_expr, + autoderefs, + unsize: false + }), base_expr.span, adj_ty, lvalue_pref, idx_ty) { autoderef.finalize(lvalue_pref, base_expr); return Some(final_mt); @@ -2139,10 +2154,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let ty::TyArray(element_ty, _) = adj_ty.sty { autoderef.finalize(lvalue_pref, base_expr); - let adjusted_ty = self.tcx.mk_slice(element_ty); + let adj_ty = self.tcx.mk_slice(element_ty); return self.try_index_step( - MethodCall::expr(expr.id), expr, base_expr, - adjusted_ty, autoderefs, true, lvalue_pref, idx_ty); + MethodCall::expr(expr.id), expr, Some(AdjustedRcvr { + rcvr_expr: base_expr, + autoderefs, + unsize: true + }), base_expr.span, adj_ty, lvalue_pref, idx_ty) } } autoderef.unambiguous_final_ty(); @@ -2157,77 +2175,111 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { fn try_index_step(&self, method_call: MethodCall, expr: &hir::Expr, - base_expr: &'gcx hir::Expr, + base_expr: Option, + base_span: Span, adjusted_ty: Ty<'tcx>, - autoderefs: usize, - unsize: bool, lvalue_pref: LvaluePreference, index_ty: Ty<'tcx>) -> Option<(/*index type*/ Ty<'tcx>, /*element type*/ Ty<'tcx>)> { let tcx = self.tcx; - debug!("try_index_step(expr={:?}, base_expr.id={:?}, adjusted_ty={:?}, \ - autoderefs={}, unsize={}, index_ty={:?})", + debug!("try_index_step(expr={:?}, base_expr={:?}, adjusted_ty={:?}, \ + index_ty={:?})", expr, base_expr, adjusted_ty, - autoderefs, - unsize, index_ty); - let input_ty = self.next_ty_var(TypeVariableOrigin::AutoDeref(base_expr.span)); + let input_ty = self.next_ty_var(TypeVariableOrigin::AutoDeref(base_span)); // First, try built-in indexing. match (adjusted_ty.builtin_index(), &index_ty.sty) { (Some(ty), &ty::TyUint(ast::UintTy::Us)) | (Some(ty), &ty::TyInfer(ty::IntVar(_))) => { debug!("try_index_step: success, using built-in indexing"); // If we had `[T; N]`, we should've caught it before unsizing to `[T]`. - assert!(!unsize); - self.apply_autoderef_adjustment(base_expr.id, autoderefs, adjusted_ty); + if let Some(base_expr) = base_expr { + assert!(!base_expr.unsize); + self.apply_autoderef_adjustment( + base_expr.rcvr_expr.id, base_expr.autoderefs, adjusted_ty); + } return Some((tcx.types.usize, ty)); } _ => {} } - // Try `IndexMut` first, if preferred. - let method = match (lvalue_pref, tcx.lang_items.index_mut_trait()) { + // If some lookup succeeds, write callee into table and extract index/element + // type from the method signature. + // If some lookup succeeded, install method in table + let method = self.try_overloaded_lvalue_op( + expr.span, base_expr, adjusted_ty, &[input_ty], lvalue_pref, LvalueOp::Index); + + method.map(|ok| { + debug!("try_index_step: success, using overloaded indexing"); + let method = self.register_infer_ok_obligations(ok); + self.tables.borrow_mut().method_map.insert(method_call, method); + (input_ty, self.make_overloaded_lvalue_return_type(method).ty) + }) + } + + fn resolve_lvalue_op(&self, op: LvalueOp, is_mut: bool) -> (Option, Symbol) { + let (tr, name) = match (op, is_mut) { + (LvalueOp::Deref, false) => + (self.tcx.lang_items.deref_trait(), "deref"), + (LvalueOp::Deref, true) => + (self.tcx.lang_items.deref_mut_trait(), "deref_mut"), + (LvalueOp::Index, false) => + (self.tcx.lang_items.index_trait(), "index"), + (LvalueOp::Index, true) => + (self.tcx.lang_items.index_mut_trait(), "index_mut"), + }; + (tr, Symbol::intern(name)) + } + + fn try_overloaded_lvalue_op(&self, + span: Span, + base_expr: Option, + base_ty: Ty<'tcx>, + arg_tys: &[Ty<'tcx>], + lvalue_pref: LvaluePreference, + op: LvalueOp) + -> Option>> + { + debug!("try_overloaded_lvalue_op({:?},{:?},{:?},{:?},{:?})", + span, + base_expr, + base_ty, + lvalue_pref, + op); + + // Try Mut first, if preferred. + let (mut_tr, mut_op) = self.resolve_lvalue_op(op, true); + let method = match (lvalue_pref, mut_tr) { (PreferMutLvalue, Some(trait_did)) => { - self.lookup_method_in_trait_adjusted(expr.span, - Some(&base_expr), - Symbol::intern("index_mut"), + self.lookup_method_in_trait_adjusted(span, + base_expr, + mut_op, trait_did, - autoderefs, - unsize, - adjusted_ty, - Some(vec![input_ty])) + base_ty, + Some(arg_tys.to_owned())) } _ => None, }; - // Otherwise, fall back to `Index`. - let method = match (method, tcx.lang_items.index_trait()) { + // Otherwise, fall back to the immutable version. + let (imm_tr, imm_op) = self.resolve_lvalue_op(op, false); + let method = match (method, imm_tr) { (None, Some(trait_did)) => { - self.lookup_method_in_trait_adjusted(expr.span, - Some(&base_expr), - Symbol::intern("index"), + self.lookup_method_in_trait_adjusted(span, + base_expr, + imm_op, trait_did, - autoderefs, - unsize, - adjusted_ty, - Some(vec![input_ty])) + base_ty, + Some(arg_tys.to_owned())) } (method, _) => method, }; - // If some lookup succeeds, write callee into table and extract index/element - // type from the method signature. - // If some lookup succeeded, install method in table - method.map(|ok| { - debug!("try_index_step: success, using overloaded indexing"); - let method = self.register_infer_ok_obligations(ok); - self.tables.borrow_mut().method_map.insert(method_call, method); - (input_ty, self.make_overloaded_lvalue_return_type(method).ty) - }) + method } fn check_method_argument_types(&self, diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 42296006b79d1..4fd0e984c9f4d 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -398,12 +398,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let method = match trait_did { Some(trait_did) => { - self.lookup_method_in_trait(expr.span, - Some(lhs_expr), - opname, - trait_did, - lhs_ty, - Some(other_tys)) + let lhs_expr = Some(super::AdjustedRcvr { + rcvr_expr: lhs_expr, autoderefs: 0, unsize: false + }); + self.lookup_method_in_trait_adjusted(expr.span, + lhs_expr, + opname, + trait_did, + lhs_ty, + Some(other_tys)) } None => None }; diff --git a/src/test/run-pass/issue-41604.rs b/src/test/run-pass/issue-41604.rs new file mode 100644 index 0000000000000..20fb8c7e7d6b0 --- /dev/null +++ b/src/test/run-pass/issue-41604.rs @@ -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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct B; + +impl B { + fn init(&mut self) {} +} + +fn main() { + let mut b = [B]; + b[1-1].init(); +}