Auto merge of #49714 - nikomatsakis:issue-49631, r=eddyb

mem-categorization, coherence fix

make mem-categorization use adjusted type for patterns: Fixes #49631

do not propagate `Err` when determing causal info: Fixes #48728

r? @eddyb

Author: bors

src/librustc/middle/mem_categorization.rs

@@ -503,8 +503,37 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
         self.resolve_type_vars_or_error(expr.hir_id, self.tables.expr_ty_adjusted_opt(expr))
     }
 
+    /// Returns the type of value that this pattern matches against.
+    /// Some non-obvious cases:
+    ///
+    /// - a `ref x` binding matches against a value of type `T` and gives
+    ///   `x` the type `&T`; we return `T`.
+    /// - a pattern with implicit derefs (thanks to default binding
+    ///   modes #42640) may look like `Some(x)` but in fact have
+    ///   implicit deref patterns attached (e.g., it is really
+    ///   `&Some(x)`). In that case, we return the "outermost" type
+    ///   (e.g., `&Option<T>).
     fn pat_ty(&self, pat: &hir::Pat) -> McResult<Ty<'tcx>> {
+        // Check for implicit `&` types wrapping the pattern; note
+        // that these are never attached to binding patterns, so
+        // actually this is somewhat "disjoint" from the code below
+        // that aims to account for `ref x`.
+        if let Some(vec) = self.tables.pat_adjustments().get(pat.hir_id) {
+            if let Some(first_ty) = vec.first() {
+                debug!("pat_ty(pat={:?}) found adjusted ty `{:?}`", pat, first_ty);
+                return Ok(first_ty);
+            }
+        }
+
+        self.pat_ty_unadjusted(pat)
+    }
+
+
+    /// Like `pat_ty`, but ignores implicit `&` patterns.
+    fn pat_ty_unadjusted(&self, pat: &hir::Pat) -> McResult<Ty<'tcx>> {
         let base_ty = self.node_ty(pat.hir_id)?;
+        debug!("pat_ty(pat={:?}) base_ty={:?}", pat, base_ty);
+
         // This code detects whether we are looking at a `ref x`,
         // and if so, figures out what the type *being borrowed* is.
         let ret_ty = match pat.node {
@@ -531,8 +560,8 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
             }
             _ => base_ty,
         };
-        debug!("pat_ty(pat={:?}) base_ty={:?} ret_ty={:?}",
-               pat, base_ty, ret_ty);
+        debug!("pat_ty(pat={:?}) ret_ty={:?}", pat, ret_ty);
+
         Ok(ret_ty)
     }
 
@@ -1246,7 +1275,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
                      self.tcx.adt_def(enum_def).variant_with_id(def_id).fields.len())
                 }
                 Def::StructCtor(_, CtorKind::Fn) => {
-                    match self.pat_ty(&pat)?.sty {
+                    match self.pat_ty_unadjusted(&pat)?.sty {
                         ty::TyAdt(adt_def, _) => {
                             (cmt, adt_def.non_enum_variant().fields.len())
                         }
@@ -1297,7 +1326,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
 
           PatKind::Tuple(ref subpats, ddpos) => {
             // (p1, ..., pN)
-            let expected_len = match self.pat_ty(&pat)?.sty {
+            let expected_len = match self.pat_ty_unadjusted(&pat)?.sty {
                 ty::TyTuple(ref tys) => tys.len(),
                 ref ty => span_bug!(pat.span, "tuple pattern unexpected type {:?}", ty),
             };

src/librustc/traits/select.rs

@@ -961,11 +961,21 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         if self.can_use_global_caches(param_env) {
             let mut cache = self.tcx().evaluation_cache.hashmap.borrow_mut();
             if let Some(trait_ref) = self.tcx().lift_to_global(&trait_ref) {
+                debug!(
+                    "insert_evaluation_cache(trait_ref={:?}, candidate={:?}) global",
+                    trait_ref,
+                    result,
+                );
                 cache.insert(trait_ref, WithDepNode::new(dep_node, result));
                 return;
             }
         }
 
+        debug!(
+            "insert_evaluation_cache(trait_ref={:?}, candidate={:?})",
+            trait_ref,
+            result,
+        );
         self.infcx.evaluation_cache.hashmap
                                    .borrow_mut()
                                    .insert(trait_ref, WithDepNode::new(dep_node, result));
@@ -1069,25 +1079,29 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                 if self.intercrate_ambiguity_causes.is_some() {
                     debug!("evaluate_stack: intercrate_ambiguity_causes is some");
                     // Heuristics: show the diagnostics when there are no candidates in crate.
-                    let candidate_set = self.assemble_candidates(stack)?;
-                    if !candidate_set.ambiguous && candidate_set.vec.iter().all(|c| {
-                        !self.evaluate_candidate(stack, &c).may_apply()
-                    }) {
-                        let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
-                        let self_ty = trait_ref.self_ty();
-                        let trait_desc = trait_ref.to_string();
-                        let self_desc = if self_ty.has_concrete_skeleton() {
-                            Some(self_ty.to_string())
-                        } else {
-                            None
-                        };
-                        let cause = if let Conflict::Upstream = conflict {
-                            IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc }
-                        } else {
-                            IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc }
-                        };
-                        debug!("evaluate_stack: pushing cause = {:?}", cause);
-                        self.intercrate_ambiguity_causes.as_mut().unwrap().push(cause);
+                    if let Ok(candidate_set) = self.assemble_candidates(stack) {
+                        if !candidate_set.ambiguous && candidate_set.vec.iter().all(|c| {
+                            !self.evaluate_candidate(stack, &c).may_apply()
+                        }) {
+                            let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
+                            let self_ty = trait_ref.self_ty();
+                            let trait_desc = trait_ref.to_string();
+                            let self_desc = if self_ty.has_concrete_skeleton() {
+                                Some(self_ty.to_string())
+                            } else {
+                                None
+                            };
+                            let cause = if let Conflict::Upstream = conflict {
+                                IntercrateAmbiguityCause::UpstreamCrateUpdate {
+                                    trait_desc,
+                                    self_desc,
+                                }
+                            } else {
+                                IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc }
+                            };
+                            debug!("evaluate_stack: pushing cause = {:?}", cause);
+                            self.intercrate_ambiguity_causes.as_mut().unwrap().push(cause);
+                        }
                     }
                 }
                 return Ok(None);
@@ -1285,12 +1299,22 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             let mut cache = tcx.selection_cache.hashmap.borrow_mut();
             if let Some(trait_ref) = tcx.lift_to_global(&trait_ref) {
                 if let Some(candidate) = tcx.lift_to_global(&candidate) {
+                    debug!(
+                        "insert_candidate_cache(trait_ref={:?}, candidate={:?}) global",
+                        trait_ref,
+                        candidate,
+                    );
                     cache.insert(trait_ref, WithDepNode::new(dep_node, candidate));
                     return;
                 }
             }
         }
 
+        debug!(
+            "insert_candidate_cache(trait_ref={:?}, candidate={:?}) local",
+            trait_ref,
+            candidate,
+        );
         self.infcx.selection_cache.hashmap
                                   .borrow_mut()
                                   .insert(trait_ref, WithDepNode::new(dep_node, candidate));

src/test/ui/issue-48728.rs

@@ -0,0 +1,23 @@
+// Copyright 2015 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.
+
+// Regression test for #48728, an ICE that occurred computing
+// coherence "help" information.
+
+#[derive(Clone)] //~ ERROR conflicting implementations of trait `std::clone::Clone`
+struct Node<T: ?Sized>(Box<T>);
+
+impl<T: Clone + ?Sized> Clone for Node<[T]> {
+    fn clone(&self) -> Self {
+        Node(Box::clone(&self.0))
+    }
+}
+
+fn main() {}

src/test/ui/issue-48728.stderr

@@ -0,0 +1,12 @@
+error[E0119]: conflicting implementations of trait `std::clone::Clone` for type `Node<[_]>`:
+  --> $DIR/issue-48728.rs:14:10
+   |
+LL | #[derive(Clone)] //~ ERROR conflicting implementations of trait `std::clone::Clone`
+   |          ^^^^^ conflicting implementation for `Node<[_]>`
+...
+LL | impl<T: Clone + ?Sized> Clone for Node<[T]> {
+   | ------------------------------------------- first implementation here
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0119`.

src/test/ui/rfc-2005-default-binding-mode/borrowck-issue-49631.rs

@@ -0,0 +1,34 @@
+// 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.
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub struct Foo {
+}
+
+impl Foo {
+    fn get(&self) -> Option<&Result<String, String>> {
+        None
+    }
+
+    fn mutate(&mut self) { }
+}
+
+fn main() {
+    let mut foo = Foo { };
+
+    // foo.get() returns type Option<&Result<String, String>>, so
+    // using `string` keeps borrow of `foo` alive. Hence calling
+    // `foo.mutate()` should be an error.
+    while let Some(Ok(string)) = foo.get() {
+        foo.mutate();
+        //~^ ERROR cannot borrow `foo` as mutable
+        println!("foo={:?}", *string);
+    }
+}

src/test/ui/rfc-2005-default-binding-mode/borrowck-issue-49631.stderr

@@ -0,0 +1,13 @@
+error[E0502]: cannot borrow `foo` as mutable because it is also borrowed as immutable
+  --> $DIR/borrowck-issue-49631.rs:30:9
+   |
+LL |     while let Some(Ok(string)) = foo.get() {
+   |                                  ---     - immutable borrow ends here
+   |                                  |
+   |                                  immutable borrow occurs here
+LL |         foo.mutate();
+   |         ^^^ mutable borrow occurs here
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0502`.