Rollup merge of rust-lang#42669 - gaurikholkar:master, r=nikomatsakis

Adding diagnostic code 0611 for lifetime errors with one named, one anonymous lifetime parameter

This is a fix for rust-lang#42517
Note that this only handles the above case for **function declarations** and **traits**.
`impl items` and `closures` will be handled in a later PR.
Example
```
fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
    if x > y { x } else { y }
}
```
now displays the following error message. ui tests have been added for the same.
```
error[E0611]: explicit lifetime required in the type of `x`
11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
   |                     ^ consider changing the type of `x` to `&'a i32`
12 |     if x > y { x } else { y }
   |                  - lifetime `'a` required
```
rust-lang#42516
r? @nikomatsakis

Author: GuillaumeGomez

src/librustc/diagnostics.rs

@@ -1946,6 +1946,44 @@ Maybe you just misspelled the lint name or the lint doesn't exist anymore.
 Either way, try to update/remove it in order to fix the error.
 "##,
 
+E0621: r##"
+This error code indicates a mismatch between the function signature (i.e.,
+the parameter types and the return type) and the function body. Most of
+the time, this indicates that the function signature needs to be changed to
+match the body, but it may be that the body needs to be changed to match
+the signature.
+
+Specifically, one or more of the parameters contain borrowed data that
+needs to have a named lifetime in order for the body to type-check. Most of
+the time, this is because the borrowed data is being returned from the
+function, as in this example:
+
+```compile_fail,E0621
+fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { // explicit lifetime required
+                                             // in the type of `y`
+    if x > y { x } else { y }
+}
+```
+
+Here, the function is returning data borrowed from either x or y, but the
+'a annotation indicates that it is returning data only from x. We can make
+the signature match the body by changing the type of y to &'a i32, like so:
+
+```
+fn foo<'a>(x: &'a i32, y: &'a i32) -> &'a i32 {
+    if x > y { x } else { y }
+}
+```
+
+Alternatively, you could change the body not to return data from y:
+
+```
+fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 {
+    x
+}
+```
+"##,
+
 }
 
 

src/librustc/infer/error_reporting/mod.rs

@@ -72,9 +72,11 @@ use ty::error::TypeError;
 use syntax::ast::DUMMY_NODE_ID;
 use syntax_pos::{Pos, Span};
 use errors::{DiagnosticBuilder, DiagnosticStyledString};
-
 mod note;
+
 mod need_type_info;
+mod named_anon_conflict;
+
 
 impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
     pub fn note_and_explain_region(self,
@@ -255,34 +257,48 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
 }
 
 impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
-    pub fn report_region_errors(&self,
-                                errors: &Vec<RegionResolutionError<'tcx>>) {
+
+    pub fn report_region_errors(&self, errors: &Vec<RegionResolutionError<'tcx>>) {
         debug!("report_region_errors(): {} errors to start", errors.len());
 
         // try to pre-process the errors, which will group some of them
         // together into a `ProcessedErrors` group:
         let errors = self.process_errors(errors);
 
-        debug!("report_region_errors: {} errors after preprocessing", errors.len());
+        debug!("report_region_errors: {} errors after preprocessing",
+               errors.len());
 
         for error in errors {
+
             debug!("report_region_errors: error = {:?}", error);
-            match error.clone() {
-                ConcreteFailure(origin, sub, sup) => {
-                    self.report_concrete_failure(origin, sub, sup).emit();
-                }
 
-                GenericBoundFailure(kind, param_ty, sub) => {
-                    self.report_generic_bound_failure(kind, param_ty, sub);
-                }
+            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();
+                  }
 
-                SubSupConflict(var_origin,
+                  GenericBoundFailure(kind, param_ty, sub) => {
+                      self.report_generic_bound_failure(kind, param_ty, sub);
+                  }
+
+                  SubSupConflict(var_origin,
                                sub_origin, sub_r,
                                sup_origin, sup_r) => {
-                    self.report_sub_sup_conflict(var_origin,
+                      self.report_sub_sup_conflict(var_origin,
                                                  sub_origin, sub_r,
                                                  sup_origin, sup_r);
-                }
+                  }
+               }
             }
         }
     }

src/librustc/infer/error_reporting/named_anon_conflict.rs

@@ -0,0 +1,199 @@
+// Copyright 2012-2013 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.
+
+//! Error Reporting for Anonymous Region Lifetime Errors.
+use hir;
+use infer::InferCtxt;
+use ty::{self, Region};
+use infer::region_inference::RegionResolutionError::*;
+use infer::region_inference::RegionResolutionError;
+use hir::map as hir_map;
+use hir::def_id::DefId;
+
+impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
+    // This method walks the Type of the function body arguments using
+    // `fold_regions()` function and returns the
+    // &hir::Arg of the function argument corresponding to the anonymous
+    // region and the Ty corresponding to the named region.
+    // 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, we would return the hir::Arg for y, we return the type &'a
+    // i32, which is the type of y but with the anonymous region replaced
+    // with 'a, the corresponding bound region and is_first which is true if
+    // the hir::Arg is the first argument in the function declaration.
+    fn find_arg_with_anonymous_region
+        (&self,
+         anon_region: Region<'tcx>,
+         named_region: Region<'tcx>)
+         -> Option<(&hir::Arg, ty::Ty<'tcx>, ty::BoundRegion, bool)> {
+
+        match *anon_region {
+            ty::ReFree(ref free_region) => {
+
+                let id = free_region.scope;
+                let node_id = self.tcx.hir.as_local_node_id(id).unwrap();
+                let body_id = self.tcx.hir.maybe_body_owned_by(node_id).unwrap();
+                let body = self.tcx.hir.body(body_id);
+                if let Some(tables) = self.in_progress_tables {
+                    body.arguments
+                        .iter()
+                        .enumerate()
+                        .filter_map(|(index, arg)| {
+                            let ty = tables.borrow().node_id_to_type(arg.id);
+                            let mut found_anon_region = false;
+                            let new_arg_ty = self.tcx
+                                .fold_regions(&ty, &mut false, |r, _| if *r == *anon_region {
+                                    found_anon_region = true;
+                                    named_region
+                                } else {
+                                    r
+                                });
+                            if found_anon_region {
+                                let is_first = index == 0;
+                                Some((arg, new_arg_ty, free_region.bound_region, is_first))
+                            } else {
+                                None
+                            }
+                        })
+                        .next()
+                } else {
+                    None
+                }
+            }
+            _ => None,
+
+        }
+    }
+
+    // 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 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
+        };
+
+        // 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, br, is_first), scope_def_id) =
+            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).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).unwrap())
+            } else {
+                return false; // inapplicable
+            };
+
+        // Here, we check for the case where the anonymous region
+        // is in the return type.
+        // FIXME(#42703) - Need to handle certain cases here.
+        let ret_ty = self.tcx.type_of(scope_def_id);
+        match ret_ty.sty {
+            ty::TyFnDef(_, _) => {
+                let sig = ret_ty.fn_sig(self.tcx);
+                let late_bound_regions = self.tcx
+                    .collect_referenced_late_bound_regions(&sig.output());
+                if late_bound_regions.iter().any(|r| *r == br) {
+                    return false;
+                } else {
+                }
+            }
+            _ => {}
+        }
+
+        // Here we check for the case where anonymous region
+        // corresponds to self and if yes, we display E0312.
+        // FIXME(#42700) - Need to format self properly to
+        // enable E0621 for it.
+        if is_first &&
+           self.tcx
+               .opt_associated_item(scope_def_id)
+               .map(|i| i.method_has_self_argument)
+               .unwrap_or(false) {
+            return false;
+        }
+
+        let (error_var, span_label_var) = if let Some(simple_name) = arg.pat.simple_name() {
+            (format!("the type of `{}`", simple_name), format!("the type of `{}`", simple_name))
+        } else {
+            (format!("parameter type"), format!("type"))
+        };
+
+
+        struct_span_err!(self.tcx.sess,
+                         span,
+                         E0621,
+                         "explicit lifetime required in {}",
+                         error_var)
+                .span_label(arg.pat.span,
+                            format!("consider changing {} to `{}`", span_label_var, new_ty))
+                .span_label(span, format!("lifetime `{}` required", named))
+                .emit();
+
+        return true;
+
+    }
+
+    // This method returns whether the given Region is Anonymous
+    // and returns the DefId corresponding to the region.
+    pub fn is_suitable_anonymous_region(&self, region: Region<'tcx>) -> Option<DefId> {
+
+        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(..)) => {
+                                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;
+                                }
+                            }
+                            _ => return None, // inapplicable
+                            // we target only top-level functions
+                        }
+                        return Some(anonymous_region_binding_scope);
+                    }
+                    _ => None,
+                }
+            }
+            _ => None,
+        }
+    }
+}

src/librustc/infer/mod.rs

@@ -38,7 +38,6 @@ use errors::DiagnosticBuilder;
 use syntax_pos::{self, Span, DUMMY_SP};
 use util::nodemap::FxHashMap;
 use arena::DroplessArena;
-
 use self::combine::CombineFields;
 use self::higher_ranked::HrMatchResult;
 use self::region_inference::{RegionVarBindings, RegionSnapshot};
@@ -1077,6 +1076,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                                                region_map,
                                                free_regions);
         let errors = self.region_vars.resolve_regions(&region_rels);
+
         if !self.is_tainted_by_errors() {
             // As a heuristic, just skip reporting region errors
             // altogether if other errors have been reported while

src/librustc/ty/sty.rs

@@ -990,6 +990,20 @@ impl RegionKind {
 
         flags
     }
+
+    // This method returns whether the given Region is Named
+    pub fn is_named_region(&self) -> bool {
+
+        match *self {
+            ty::ReFree(ref free_region) => {
+                match free_region.bound_region {
+                    ty::BrNamed(..) => true,
+                    _ => false,
+                }
+            }
+            _ => false,
+        }
+    }
 }
 
 /// Type utilities

src/test/compile-fail/E0621-does-not-trigger-for-closures.rs

@@ -0,0 +1,28 @@
+// 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.
+
+// Test that we give the generic E0495 when one of the free regions is
+// bound in a closure (rather than suggesting a change to the signature
+// of the closure, which is not specified in `foo` but rather in `invoke`).
+
+// FIXME - This might be better as a UI test, but the finer details
+// of the error seem to vary on different machines.
+fn invoke<'a, F>(x: &'a i32, f: F) -> &'a i32
+where F: FnOnce(&'a i32, &i32) -> &'a i32
+{
+    let y = 22;
+    f(x, &y)
+}
+
+fn foo<'a>(x: &'a i32) {
+    invoke(&x, |a, b| if a > b { a } else { b }); //~ ERROR E0495
+}
+
+fn main() {}