code review fixes

Author: gaurikholkar

src/librustc/infer/error_reporting/anon_anon_conflict.rs

@@ -48,8 +48,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         // Determine whether the sub and sup consist of both anonymous (elided) regions.
         let (ty_sup, ty_sub, scope_def_id_sup, scope_def_id_sub, bregion_sup, bregion_sub) =
             if let (Some(anon_reg_sup), Some(anon_reg_sub)) =
-                (self.is_suitable_anonymous_region(sup, true),
-                 self.is_suitable_anonymous_region(sub, true)) {
+                (self.is_suitable_anonymous_region(sup), self.is_suitable_anonymous_region(sub)) {
                 let (def_id_sup, br_sup, def_id_sub, br_sub) = (anon_reg_sup.def_id,
                                                                 anon_reg_sup.boundregion,
                                                                 anon_reg_sub.def_id,
@@ -64,7 +63,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                 return false;
             };
 
-        let (label1, label2) = if let (Some(sup_arg), Some(sub_arg)) =
+        let (main_label, label1, label2) = if let (Some(sup_arg), Some(sub_arg)) =
             (self.find_arg_with_anonymous_region(sup, sup),
              self.find_arg_with_anonymous_region(sub, sub)) {
 
@@ -81,7 +80,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             }
 
             if anon_arg_sup == anon_arg_sub {
-                (format!(" with one lifetime"), format!(" into the other"))
+                (format!("this type was declared with multiple lifetimes..."),
+                 format!(" with one lifetime"),
+                 format!(" into the other"))
             } else {
                 let span_label_var1 = if let Some(simple_name) = anon_arg_sup.pat.simple_name() {
                     format!(" from `{}`", simple_name)
@@ -95,15 +96,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                     format!("")
                 };
 
-                (span_label_var1, span_label_var2)
+                let span_label =
+                    format!("these two types are declared with different lifetimes...",);
+
+                (span_label, span_label_var1, span_label_var2)
             }
         } else {
             return false;
         };
 
+
         struct_span_err!(self.tcx.sess, span, E0623, "lifetime mismatch")
-            .span_label(ty_sup.span,
-                        format!("these two types are declared with different lifetimes..."))
+            .span_label(ty_sup.span, main_label)
             .span_label(ty_sub.span, format!(""))
             .span_label(span, format!("...but data{} flows{} here", label1, label2))
             .emit();
@@ -126,7 +130,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     /// The function returns the nested type corresponding to the anonymous region
     /// for e.g. `&u8` and Vec<`&u8`.
     pub fn find_anon_type(&self, region: Region<'tcx>, br: &ty::BoundRegion) -> Option<&hir::Ty> {
-        if let Some(anon_reg) = self.is_suitable_anonymous_region(region, true) {
+        if let Some(anon_reg) = self.is_suitable_anonymous_region(region) {
             let def_id = anon_reg.def_id;
             if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
                 let ret_ty = self.tcx.type_of(def_id);

src/librustc/infer/error_reporting/named_anon_conflict.rs

@@ -31,24 +31,28 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         // version new_ty of its type where the anonymous region is replaced
         // with the named one.//scope_def_id
         let (named, anon_arg_info, region_info) =
-            if sub.is_named_region() && self.is_suitable_anonymous_region(sup, false).is_some() {
+            if sub.is_named_region() && self.is_suitable_anonymous_region(sup).is_some() {
                 (sub,
                  self.find_arg_with_anonymous_region(sup, sub).unwrap(),
-                 self.is_suitable_anonymous_region(sup, false).unwrap())
-            } else if sup.is_named_region() &&
-                      self.is_suitable_anonymous_region(sub, false).is_some() {
+                 self.is_suitable_anonymous_region(sup).unwrap())
+            } else if sup.is_named_region() && self.is_suitable_anonymous_region(sub).is_some() {
                 (sup,
                  self.find_arg_with_anonymous_region(sub, sup).unwrap(),
-                 self.is_suitable_anonymous_region(sub, false).unwrap())
+                 self.is_suitable_anonymous_region(sub).unwrap())
             } else {
                 return false; // inapplicable
             };
 
-        let (arg, new_ty, br, is_first, scope_def_id) = (anon_arg_info.arg,
-                                                         anon_arg_info.arg_ty,
-                                                         anon_arg_info.bound_region,
-                                                         anon_arg_info.is_first,
-                                                         region_info.def_id);
+        let (arg, new_ty, br, is_first, scope_def_id, is_impl_item) = (anon_arg_info.arg,
+                                                                       anon_arg_info.arg_ty,
+                                                                       anon_arg_info.bound_region,
+                                                                       anon_arg_info.is_first,
+                                                                       region_info.def_id,
+                                                                       region_info.is_impl_item);
+        if is_impl_item {
+            return false;
+        }
+
         if self.is_return_type_anon(scope_def_id, br) || self.is_self_anon(is_first, scope_def_id) {
             return false;
         } else {

src/librustc/infer/error_reporting/util.rs

@@ -37,6 +37,8 @@ pub struct FreeRegionInfo {
     pub def_id: DefId,
     // the bound region corresponding to FreeRegion
     pub boundregion: ty::BoundRegion,
+    // checks if bound region is in Impl Item
+    pub is_impl_item: bool,
 }
 
 impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
@@ -106,48 +108,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
 
     // This method returns whether the given Region is Anonymous
     // and returns the DefId and the BoundRegion corresponding to the given region.
-    // The is_anon_anon is set true when we are dealing with cases where
-    // both the regions are anonymous i.e. E0623.
-    pub fn is_suitable_anonymous_region(&self,
-                                        region: Region<'tcx>,
-                                        is_anon_anon: bool)
-                                        -> Option<FreeRegionInfo> {
+    pub fn is_suitable_anonymous_region(&self, region: Region<'tcx>) -> Option<FreeRegionInfo> {
         if let ty::ReFree(ref free_region) = *region {
             if let ty::BrAnon(..) = free_region.bound_region {
                 let anonymous_region_binding_scope = free_region.scope;
-                let node_id = self.tcx
-                    .hir
-                    .as_local_node_id(anonymous_region_binding_scope)
-                    .unwrap();
-                match self.tcx.hir.find(node_id) {
-                    Some(hir_map::NodeItem(..)) |
-                    Some(hir_map::NodeTraitItem(..)) => {
-                        // Success -- proceed to return Some below
-                    }
-                    Some(hir_map::NodeImplItem(..)) => {
-                        if !is_anon_anon {
-                            let container_id = self.tcx
-                                .associated_item(anonymous_region_binding_scope)
-                                .container
-                                .id();
-                            if self.tcx.impl_trait_ref(container_id).is_some() {
-                                // For now, we do not try to target impls of traits. This is
-                                // because this message is going to suggest that the user
-                                // change the fn signature, but they may not be free to do so,
-                                // since the signature must match the trait.
-                                //
-                                // FIXME(#42706) -- in some cases, we could do better here.
-                                return None;
-                            }
-                        } else {
-                        }
-                    }
-                    _ => return None, // inapplicable
-                    // we target only top-level functions
-                }
                 return Some(FreeRegionInfo {
                                 def_id: anonymous_region_binding_scope,
                                 boundregion: free_region.bound_region,
+                                is_impl_item:
+                                self.is_bound_region_in_impl_item(anonymous_region_binding_scope),
                             });
             }
         }
@@ -177,12 +146,42 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     // FIXME(#42700) - Need to format self properly to
     // enable E0621 for it.
     pub fn is_self_anon(&self, is_first: bool, scope_def_id: DefId) -> bool {
-        if is_first &&
-           self.tcx
-               .opt_associated_item(scope_def_id)
-               .map(|i| i.method_has_self_argument)
-               .unwrap_or(false) {
-            return true;
+        is_first &&
+        self.tcx
+            .opt_associated_item(scope_def_id)
+            .map(|i| i.method_has_self_argument) == Some(true)
+    }
+
+    // Here we check if the bound region is in Impl Item.
+    pub fn is_bound_region_in_impl_item(&self, anonymous_region_binding_scope: DefId) -> bool {
+        let node_id = self.tcx
+            .hir
+            .as_local_node_id(anonymous_region_binding_scope)
+            .unwrap();
+        match self.tcx.hir.find(node_id) {
+
+            Some(hir_map::NodeItem(..)) |
+            Some(hir_map::NodeTraitItem(..)) => {
+                // Success -- proceed to return Some below
+            }
+            Some(hir_map::NodeImplItem(..)) => {
+                let container_id = self.tcx
+                    .associated_item(anonymous_region_binding_scope)
+                    .container
+                    .id();
+                if self.tcx.impl_trait_ref(container_id).is_some() {
+                    // For now, we do not try to target impls of traits. This is
+                    // because this message is going to suggest that the user
+                    // change the fn signature, but they may not be free to do so,
+                    // since the signature must match the trait.
+                    //
+                    // FIXME(#42706) -- in some cases, we could do better here.
+                    return true;
+                }
+            }
+            _ => {
+                return false;
+            }
         }
         false
     }

src/test/ui/lifetime-errors/ex3-both-anon-regions-both-are-structs-3.stderr

@@ -4,7 +4,7 @@ error[E0623]: lifetime mismatch
 15 | fn foo(mut x: Ref) {
    |               ---
    |               |
-   |               these two types are declared with different lifetimes...
+   |               this type was declared with multiple lifetimes...
 16 |     x.a = x.b;
    |           ^^^ ...but data with one lifetime flows into the other here
 

src/test/ui/lifetime-errors/ex3-both-anon-regions-both-are-structs-4.rs

@@ -11,7 +11,7 @@
 struct Ref<'a, 'b> { a: &'a u32, b: &'b u32 }
 
 fn foo(mut y: Ref, x: &u32) {
-    x = y.b;
+    y.b = x;
 }
 
 fn main() { }

src/test/ui/lifetime-errors/ex3-both-anon-regions-both-are-structs-4.stderr

@@ -1,12 +1,10 @@
 error[E0623]: lifetime mismatch
-  --> $DIR/ex3-both-anon-regions-one-is-struct-3.rs:14:9
+  --> $DIR/ex3-both-anon-regions-one-is-struct-4.rs:14:11
    |
 13 | fn foo(mut y: Ref, x: &u32) {
-   |               ---     ----
-   |               |
-   |               these two types are declared with different lifetimes...
-14 |     x = y.b;
-   |         ^^^ ...but data from `y` flows into `x` here
+   |               ---     ---- these two types are declared with different lifetimes...
+14 |     y.b = x;
+   |           ^ ...but data from `x` flows into `y` here
 
 error: aborting due to previous error