Enabling E0611 for inherent functions

gaurikholkar · gaurikholkar · commit 8fb6f74e57f6 · 2017-06-29T06:37:18.000-07:00
diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs
@@ -67,7 +67,7 @@ use hir::def_id::DefId;
 use middle::region;
 use traits::{ObligationCause, ObligationCauseCode};
 use ty::{self, TyCtxt, TypeFoldable};
-use ty::{Region, Issue32330 };
+use ty::{Region, Issue32330};
 use ty::error::TypeError;
 use syntax::ast::DUMMY_NODE_ID;
 use syntax_pos::{Pos, Span};
@@ -272,11 +272,17 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         for error in errors {
 
             debug!("report_region_errors: error = {:?}", error);
-        // If ConcreteFailure does not have an anonymous region
-            if !self.report_named_anon_conflict(&error){
+            
+            if !self.try_report_named_anon_conflict(&error){
 
                match error.clone() {
-
+                  // These errors could indicate all manner of different
+                  // problems with many different solutions. Rather
+                  // than generate a "one size fits all" error, what we
+                  // attempt to do is go through a number of specific
+                  // scenarios and try to find the best way to present
+                  // the error. If all of these fails, we fall back to a rather
+                  // general bit of code that displays the error information
                   ConcreteFailure(origin, sub, sup) => {
 
                       self.report_concrete_failure(origin, sub, sup).emit();
diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs
@@ -14,6 +14,7 @@ use infer::InferCtxt;
 use ty::{self, Region};
 use infer::region_inference::RegionResolutionError::*;
 use infer::region_inference::RegionResolutionError;
+use hir::map as hir_map;
 
 impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     // This method walks the Type of the function body arguments using
@@ -23,12 +24,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     // Currently only the case where the function declaration consists of
     // one named region and one anonymous region is handled.
     // Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32`
-    // Here, the `y` and the `Ty` of `y` is returned after being substituted
-    // by that of the named region.
-    pub fn find_arg_with_anonymous_region(&self,
-                                          anon_region: Region<'tcx>,
-                                          named_region: Region<'tcx>)
-                                          -> Option<(&hir::Arg, ty::Ty<'tcx>)> {
+    // Here, we would return the hir::Arg for y, and we return the type &'a
+    // i32, which is the type of y but with the anonymous region replaced
+    // with 'a.
+    fn find_arg_with_anonymous_region(&self,
+                                      anon_region: Region<'tcx>,
+                                      named_region: Region<'tcx>)
+                                      -> Option<(&hir::Arg, ty::Ty<'tcx>)> {
 
         match *anon_region {
             ty::ReFree(ref free_region) => {
@@ -70,29 +72,35 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     // This method generates the error message for the case when
     // the function arguments consist of a named region and an anonymous
     // region and corresponds to `ConcreteFailure(..)`
-    pub fn report_named_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool {
+    pub fn try_report_named_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool {
 
         let (span, sub, sup) = match *error {
             ConcreteFailure(ref origin, sub, sup) => (origin.span(), sub, sup),
             _ => return false, // inapplicable
         };
 
-        let (named, (var, new_ty)) =
-            if self.is_named_region(sub) && self.is_anonymous_region(sup) {
+        // Determine whether the sub and sup consist of one named region ('a)
+        // and one anonymous (elided) region. If so, find the parameter arg
+        // where the anonymous region appears (there must always be one; we
+        // only introduced anonymous regions in parameters) as well as a
+        // version new_ty of its type where the anonymous region is replaced
+        // with the named one.
+        let (named, (arg, new_ty)) =
+            if self.is_named_region(sub) && self.is_suitable_anonymous_region(sup) {
                 (sub, self.find_arg_with_anonymous_region(sup, sub).unwrap())
-            } else if self.is_named_region(sup) && self.is_anonymous_region(sub) {
+            } else if self.is_named_region(sup) && self.is_suitable_anonymous_region(sub) {
                 (sup, self.find_arg_with_anonymous_region(sub, sup).unwrap())
             } else {
                 return false; // inapplicable
             };
 
-        if let Some(simple_name) = var.pat.simple_name() {
+        if let Some(simple_name) = arg.pat.simple_name() {
             struct_span_err!(self.tcx.sess,
                              span,
                              E0611,
                              "explicit lifetime required in the type of `{}`",
                              simple_name)
-                    .span_label(var.pat.span,
+                    .span_label(arg.pat.span,
                                 format!("consider changing the type of `{}` to `{}`",
                                         simple_name,
                                         new_ty))
@@ -104,12 +112,55 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                              span,
                              E0611,
                              "explicit lifetime required in parameter type")
-                    .span_label(var.pat.span,
+                    .span_label(arg.pat.span,
                                 format!("consider changing type to `{}`", new_ty))
                     .span_label(span, format!("lifetime `{}` required", named))
                     .emit();
         }
         return true;
 
     }
+
+    // This method returns whether the given Region is Anonymous
+    pub fn is_suitable_anonymous_region(&self, region: Region<'tcx>) -> bool {
+
+        match *region {
+            ty::ReFree(ref free_region) => {
+                match free_region.bound_region {
+                    ty::BrAnon(..) => {
+                        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(..)) => {
+                                // proceed ahead //
+                            }
+                            Some(hir_map::NodeImplItem(..)) => {
+                                if self.tcx.impl_trait_ref(self.tcx.
+associated_item(anonymous_region_binding_scope).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 false;//None;
+                                }
+                              else{  }
+
+                            }
+                            _ => return false, // inapplicable
+                            // we target only top-level functions
+                        }
+                        return true;
+                    }
+                    _ => false,
+                }
+            }
+            _ => false,
+        }
+    }
 }
diff --git a/src/librustc/infer/error_reporting/util.rs b/src/librustc/infer/error_reporting/util.rs
@@ -12,7 +12,6 @@
 
 use ty::{self, Region};
 use infer::InferCtxt;
-use hir::map as hir_map;
 
 impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     // This method returns whether the given Region is Named
@@ -28,28 +27,4 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             _ => false,
         }
     }
-
-    // This method returns whether the given Region is Anonymous
-    pub fn is_anonymous_region(&self, region: Region<'tcx>) -> bool {
-
-        match *region {
-            ty::ReFree(ref free_region) => {
-                match free_region.bound_region {
-                    ty::BrAnon(..) => {
-                        let id = free_region.scope;
-                        let node_id = self.tcx.hir.as_local_node_id(id).unwrap();
-                        match self.tcx.hir.find(node_id) {
-                            Some(hir_map::NodeItem(..)) |
-                            Some(hir_map::NodeTraitItem(..)) => { /* proceed ahead */ }
-                            _ => return false, // inapplicable
-                            // we target only top-level functions
-                        }
-                        return true;
-                    }
-                    _ => false,
-                }
-            }
-            _ => false,
-        }
-    }
 }
diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.rs
@@ -0,0 +1,18 @@
+// Copyright 2016 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.
+
+trait Foo {
+
+fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
+   if x > y { x } else { y }
+   }
+}
+
+fn main() { }
diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr
@@ -0,0 +1,10 @@
+error[E0611]: explicit lifetime required in the type of `x`
+  --> $DIR/ex1-return-one-existing-name-if-else-using-impl-2.rs:14:15
+   |
+13 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
+   |            - consider changing the type of `x` to `&'a i32`
+14 |    if x > y { x } else { y }
+   |               ^ lifetime `'a` required
+
+error: aborting due to previous error(s)
+
diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs
@@ -0,0 +1,24 @@
+// Copyright 2016 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.
+
+struct Foo {
+  field: i32
+}
+
+impl Foo {
+  fn foo<'a>(&'a self, x: &i32) -> &i32 {
+   
+    if true { &self.field } else { x }
+
+  }
+
+}
+
+fn main() { }
diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr
@@ -0,0 +1,11 @@
+error[E0611]: explicit lifetime required in the type of `x`
+  --> $DIR/ex1-return-one-existing-name-if-else-using-impl-3.rs:18:36
+   |
+16 |   fn foo<'a>(&'a self, x: &i32) -> &i32 {
+   |                        - consider changing the type of `x` to `&'a i32`
+17 |    
+18 |     if true { &self.field } else { x }
+   |                                    ^ lifetime `'a` required
+
+error: aborting due to previous error(s)
+
