rustc_typeck: consolidate adjustment composition

Fixes rust-lang#41213.

Author: arielb1

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.

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);
             }
 

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)
             }

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),

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,

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;
             }
         }

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 <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.
+
+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;
+    }
+}