From 03b0d995564b26a2ca9ccf0ded8a6fd375dd9412 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 13 Apr 2017 21:27:35 +0300 Subject: [PATCH] rustc_typeck: consolidate adjustment composition Fixes #41213. --- src/librustc/ty/adjustment.rs | 2 +- src/librustc_typeck/check/callee.rs | 2 +- src/librustc_typeck/check/coercion.rs | 38 +++++--------- src/librustc_typeck/check/method/confirm.rs | 8 +-- src/librustc_typeck/check/method/mod.rs | 2 +- src/librustc_typeck/check/mod.rs | 55 +++++++++++++++------ src/test/run-pass/issue-41213.rs | 32 ++++++++++++ 7 files changed, 92 insertions(+), 47 deletions(-) create mode 100644 src/test/run-pass/issue-41213.rs diff --git a/src/librustc/ty/adjustment.rs b/src/librustc/ty/adjustment.rs index d8ca30477205c..1d7100a7a4e44 100644 --- a/src/librustc/ty/adjustment.rs +++ b/src/librustc/ty/adjustment.rs @@ -33,7 +33,7 @@ pub enum Adjust<'tcx> { /// Go from a safe fn pointer to an unsafe fn pointer. UnsafeFnPointer, - // Go from a non-capturing closure to an fn pointer. + /// Go from a non-capturing closure to an fn pointer. ClosureFnPointer, /// Go from a mut raw pointer to a const raw pointer. diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index f9bc947a97358..9c5870c12aad4 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -100,7 +100,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // If the callee is a bare function or a closure, then we're all set. match self.structurally_resolved_type(callee_expr.span, adjusted_ty).sty { ty::TyFnDef(..) | ty::TyFnPtr(_) => { - self.write_autoderef_adjustment(callee_expr.id, autoderefs, adjusted_ty); + self.apply_autoderef_adjustment(callee_expr.id, autoderefs, adjusted_ty); return Some(CallStep::Builtin); } diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index a5acd0c7e5300..de7a579ba10ed 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -712,13 +712,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.commit_if_ok(|_| { let ok = coerce.coerce(&[expr], source, target)?; let adjustment = self.register_infer_ok_obligations(ok); - if !adjustment.is_identity() { - debug!("Success, coerced with {:?}", adjustment); - if self.tables.borrow().adjustments.get(&expr.id).is_some() { - bug!("expr already has an adjustment on it!"); - } - self.write_adjustment(expr.id, adjustment); - } + self.apply_adjustment(expr.id, adjustment); // We should now have added sufficient adjustments etc to // ensure that the type of expression, post-adjustment, is @@ -780,9 +774,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // Reify both sides and return the reified fn pointer type. let fn_ptr = self.tcx.mk_fn_ptr(fty); for expr in exprs.iter().map(|e| e.as_coercion_site()).chain(Some(new)) { - // No adjustments can produce a fn item, so this should never trip. - assert!(!self.tables.borrow().adjustments.contains_key(&expr.id)); - self.write_adjustment(expr.id, Adjustment { + // The only adjustment that can produce an fn item is + // `NeverToAny`, so this should always be valid. + self.apply_adjustment(expr.id, Adjustment { kind: Adjust::ReifyFnPointer, target: fn_ptr }); @@ -803,9 +797,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { match result { Ok(ok) => { let adjustment = self.register_infer_ok_obligations(ok); - if !adjustment.is_identity() { - self.write_adjustment(new.id, adjustment); - } + self.apply_adjustment(new.id, adjustment); return Ok(adjustment.target); } Err(e) => first_error = Some(e), @@ -825,7 +817,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }) => { match self.node_ty(expr.id).sty { ty::TyRef(_, mt_orig) => { - // Reborrow that we can safely ignore. + // Reborrow that we can safely ignore, because + // the next adjustment can only be a DerefRef + // which will be merged into it. mutbl_adj == mt_orig.mutbl } _ => false, @@ -858,19 +852,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } Ok(ok) => { let adjustment = self.register_infer_ok_obligations(ok); - if !adjustment.is_identity() { - let mut tables = self.tables.borrow_mut(); - for expr in exprs { - let expr = expr.as_coercion_site(); - if let Some(&mut Adjustment { - kind: Adjust::NeverToAny, - ref mut target - }) = tables.adjustments.get_mut(&expr.id) { - *target = adjustment.target; - continue; - } - tables.adjustments.insert(expr.id, adjustment); - } + for expr in exprs { + let expr = expr.as_coercion_site(); + self.apply_adjustment(expr.id, adjustment); } Ok(adjustment.target) } diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs index 73f6cd76290aa..28ac335cf195a 100644 --- a/src/librustc_typeck/check/method/confirm.rs +++ b/src/librustc_typeck/check/method/confirm.rs @@ -143,7 +143,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { let target = target.adjust_for_autoref(self.tcx, autoref); // Write out the final adjustment. - self.write_adjustment(self.self_expr.id, Adjustment { + self.apply_adjustment(self.self_expr.id, Adjustment { kind: Adjust::DerefRef { autoderefs: pick.autoderefs, autoref: autoref, @@ -433,7 +433,8 @@ 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. + // 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, .. }, .. }) => { @@ -464,7 +465,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { // 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().adjustments.get(&base_expr.id).cloned(); + 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 }, @@ -537,6 +538,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { // 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), diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 4085a171bbef5..9ecf0ffa71ebd 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -299,7 +299,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } }; - self.write_adjustment(self_expr.id, Adjustment { + self.apply_adjustment(self_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 c995b7e92843d..dc221a93f1342 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -95,7 +95,7 @@ use rustc::ty::{ParamTy, ParameterEnvironment}; use rustc::ty::{LvaluePreference, NoPreference, PreferMutLvalue}; use rustc::ty::{self, Ty, TyCtxt, Visibility}; use rustc::ty::{MethodCall, MethodCallee}; -use rustc::ty::adjustment; +use rustc::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; use rustc::ty::fold::{BottomUpFolder, TypeFoldable}; use rustc::ty::maps::Providers; use rustc::ty::util::{Representability, IntTypeExt}; @@ -108,6 +108,7 @@ use util::common::{ErrorReported, indenter}; use util::nodemap::{DefIdMap, FxHashMap, FxHashSet, NodeMap}; use std::cell::{Cell, RefCell}; +use std::collections::hash_map::Entry; use std::cmp; use std::mem::replace; use std::ops::{self, Deref}; @@ -1637,12 +1638,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } - pub fn write_autoderef_adjustment(&self, + pub fn apply_autoderef_adjustment(&self, node_id: ast::NodeId, derefs: usize, adjusted_ty: Ty<'tcx>) { - self.write_adjustment(node_id, adjustment::Adjustment { - kind: adjustment::Adjust::DerefRef { + self.apply_adjustment(node_id, Adjustment { + kind: Adjust::DerefRef { autoderefs: derefs, autoref: None, unsize: false @@ -1651,16 +1652,42 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }); } - pub fn write_adjustment(&self, - node_id: ast::NodeId, - adj: adjustment::Adjustment<'tcx>) { - debug!("write_adjustment(node_id={}, adj={:?})", node_id, adj); + pub fn apply_adjustment(&self, node_id: ast::NodeId, adj: Adjustment<'tcx>) { + debug!("apply_adjustment(node_id={}, adj={:?})", node_id, adj); if adj.is_identity() { return; } - self.tables.borrow_mut().adjustments.insert(node_id, adj); + match self.tables.borrow_mut().adjustments.entry(node_id) { + Entry::Vacant(entry) => { entry.insert(adj); }, + Entry::Occupied(mut entry) => { + debug!(" - composing on top of {:?}", entry.get()); + let composed_kind = match (entry.get().kind, adj.kind) { + // Applying any adjustment on top of a NeverToAny + // is a valid NeverToAny adjustment, because it can't + // be reached. + (Adjust::NeverToAny, _) => Adjust::NeverToAny, + (Adjust::DerefRef { + autoderefs: 1, + autoref: Some(AutoBorrow::Ref(..)), + unsize: false + }, Adjust::DerefRef { autoderefs, .. }) if autoderefs > 0 => { + // A reborrow has no effect before a dereference. + adj.kind + } + // FIXME: currently we never try to compose autoderefs + // and ReifyFnPointer/UnsafeFnPointer, but we could. + _ => + bug!("while adjusting {}, can't compose {:?} and {:?}", + node_id, entry.get(), adj) + }; + *entry.get_mut() = Adjustment { + kind: composed_kind, + target: adj.target + }; + } + } } /// Basically whenever we are converting from a type scheme into @@ -2302,7 +2329,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { 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.write_autoderef_adjustment(base_expr.id, autoderefs, adjusted_ty); + self.apply_autoderef_adjustment(base_expr.id, autoderefs, adjusted_ty); return Some((tcx.types.usize, ty)); } _ => {} @@ -2685,8 +2712,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { "expression with never type wound up being adjusted"); let adj_ty = self.next_diverging_ty_var( TypeVariableOrigin::AdjustmentType(expr.span)); - self.write_adjustment(expr.id, adjustment::Adjustment { - kind: adjustment::Adjust::NeverToAny, + self.apply_adjustment(expr.id, Adjustment { + kind: Adjust::NeverToAny, target: adj_ty }); ty = adj_ty; @@ -2917,7 +2944,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let field_ty = self.field_ty(expr.span, field, substs); if self.tcx.vis_is_accessible_from(field.vis, self.body_id) { autoderef.finalize(lvalue_pref, &[base]); - self.write_autoderef_adjustment(base.id, autoderefs, base_t); + self.apply_autoderef_adjustment(base.id, autoderefs, base_t); self.tcx.check_stability(field.did, expr.id, expr.span); @@ -3041,7 +3068,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(field_ty) = field { autoderef.finalize(lvalue_pref, &[base]); - self.write_autoderef_adjustment(base.id, autoderefs, base_t); + self.apply_autoderef_adjustment(base.id, autoderefs, base_t); return field_ty; } } diff --git a/src/test/run-pass/issue-41213.rs b/src/test/run-pass/issue-41213.rs new file mode 100644 index 0000000000000..d4755020fef22 --- /dev/null +++ b/src/test/run-pass/issue-41213.rs @@ -0,0 +1,32 @@ +// 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. + +enum A { + A1, + A2, + A3, +} + +enum B { + B1(String, String), + B2(String, String), +} + +fn main() { + let a = A::A1; + loop { + let _ctor = match a { + A::A3 => break, + A::A1 => B::B1, + A::A2 => B::B2, + }; + break; + } +}