Improve HRTB error span when -Zno-leak-check is used

src/librustc/infer/mod.rs

@@ -418,15 +418,27 @@ pub enum NLLRegionVariableOrigin {
     /// from a `for<'a> T` binder). Meant to represent "any region".
     Placeholder(ty::PlaceholderRegion),
 
-    Existential,
+    Existential {
+        /// If this is true, then this variable was created to represent a lifetime
+        /// bound in a `for` binder. For example, it might have been created to
+        /// represent the lifetime `'a` in a type like `for<'a> fn(&'a u32)`.
+        /// Such variables are created when we are trying to figure out if there
+        /// is any valid instantiation of `'a` that could fit into some scenario.
+        ///
+        /// This is used to inform error reporting: in the case that we are trying to
+        /// determine whether there is any valid instantiation of a `'a` variable that meets
+        /// some constraint C, we want to blame the "source" of that `for` type,
+        /// rather than blaming the source of the constraint C.
+        from_forall: bool
+    },
 }
 
 impl NLLRegionVariableOrigin {
     pub fn is_universal(self) -> bool {
         match self {
             NLLRegionVariableOrigin::FreeRegion => true,
             NLLRegionVariableOrigin::Placeholder(..) => true,
-            NLLRegionVariableOrigin::Existential => false,
+            NLLRegionVariableOrigin::Existential{ .. } => false,
         }
     }
 

src/librustc/infer/nll_relate/mod.rs

@@ -93,7 +93,7 @@ pub trait TypeRelatingDelegate<'tcx> {
     /// we will invoke this method to instantiate `'a` with an
     /// inference variable (though `'b` would be instantiated first,
     /// as a placeholder).
-    fn next_existential_region_var(&mut self) -> ty::Region<'tcx>;
+    fn next_existential_region_var(&mut self, was_placeholder: bool) -> ty::Region<'tcx>;
 
     /// Creates a new region variable representing a
     /// higher-ranked region that is instantiated universally.
@@ -193,7 +193,7 @@ where
                     let placeholder = ty::PlaceholderRegion { universe, name: br };
                     delegate.next_placeholder_region(placeholder)
                 } else {
-                    delegate.next_existential_region_var()
+                    delegate.next_existential_region_var(true)
                 }
             }
         };

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs

@@ -98,9 +98,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         &self,
         body: &Body<'tcx>,
         from_region: RegionVid,
+        from_region_origin: NLLRegionVariableOrigin,
         target_test: impl Fn(RegionVid) -> bool,
     ) -> (ConstraintCategory, bool, Span) {
-        debug!("best_blame_constraint(from_region={:?})", from_region);
+        debug!("best_blame_constraint(from_region={:?}, from_region_origin={:?})",
+            from_region, from_region_origin);
 
         // Find all paths
         let (path, target_region) =
@@ -153,19 +155,85 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // we still want to screen for an "interesting" point to
         // highlight (e.g., a call site or something).
         let target_scc = self.constraint_sccs.scc(target_region);
-        let best_choice = (0..path.len()).rev().find(|&i| {
-            let constraint = path[i];
+        let mut range = 0..path.len();
+
+        // As noted above, when reporting an error, there is typically a chain of constraints
+        // leading from some "source" region which must outlive some "target" region.
+        // In most cases, we prefer to "blame" the constraints closer to the target --
+        // but there is one exception. When constraints arise from higher-ranked subtyping,
+        // we generally prefer to blame the source value,
+        // as the "target" in this case tends to be some type annotation that the user gave.
+        // Therefore, if we find that the region origin is some instantiation
+        // of a higher-ranked region, we start our search from the "source" point
+        // rather than the "target", and we also tweak a few other things.
+        //
+        // An example might be this bit of Rust code:
+        //
+        // ```rust
+        // let x: fn(&'static ()) = |_| {};
+        // let y: for<'a> fn(&'a ()) = x;
+        // ```
+        //
+        // In MIR, this will be converted into a combination of assignments and type ascriptions.
+        // In particular, the 'static is imposed through a type ascription:
+        //
+        // ```rust
+        // x = ...;
+        // AscribeUserType(x, fn(&'static ())
+        // y = x;
+        // ```
+        //
+        // We wind up ultimately with constraints like
+        //
+        // ```rust
+        // !a: 'temp1 // from the `y = x` statement
+        // 'temp1: 'temp2
+        // 'temp2: 'static // from the AscribeUserType
+        // ```
+        //
+        // and here we prefer to blame the source (the y = x statement).
+        let blame_source = match from_region_origin {
+            NLLRegionVariableOrigin::FreeRegion
+                | NLLRegionVariableOrigin::Existential { from_forall: false  } => {
+                    true
+            }
+            NLLRegionVariableOrigin::Placeholder(_)
+                | NLLRegionVariableOrigin::Existential { from_forall: true  } => {
+                    false
+            }
+        };
+
+        let find_region = |i: &usize| {
+            let constraint = path[*i];
 
             let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup);
 
-            match categorized_path[i].0 {
-                ConstraintCategory::OpaqueType | ConstraintCategory::Boring |
-                ConstraintCategory::BoringNoLocation | ConstraintCategory::Internal => false,
-                ConstraintCategory::TypeAnnotation | ConstraintCategory::Return |
-                ConstraintCategory::Yield => true,
-                _ => constraint_sup_scc != target_scc,
+            if blame_source {
+                match categorized_path[*i].0 {
+                    ConstraintCategory::OpaqueType | ConstraintCategory::Boring |
+                    ConstraintCategory::BoringNoLocation | ConstraintCategory::Internal => false,
+                    ConstraintCategory::TypeAnnotation | ConstraintCategory::Return |
+                    ConstraintCategory::Yield => true,
+                    _ => constraint_sup_scc != target_scc,
+                }
+            } else {
+                match categorized_path[*i].0 {
+                    ConstraintCategory::OpaqueType | ConstraintCategory::Boring |
+                    ConstraintCategory::BoringNoLocation | ConstraintCategory::Internal => false,
+                    _ => true
+                }
             }
-        });
+        };
+
+        let best_choice = if blame_source {
+            range.rev().find(find_region)
+        } else {
+            range.find(find_region)
+        };
+
+        debug!("best_blame_constraint: best_choice={:?} blame_source={}",
+            best_choice, blame_source);
+
         if let Some(i) = best_choice {
             if let Some(next) = categorized_path.get(i + 1) {
                 if categorized_path[i].0 == ConstraintCategory::Return
@@ -297,12 +365,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         infcx: &'a InferCtxt<'a, 'tcx>,
         mir_def_id: DefId,
         fr: RegionVid,
+        fr_origin: NLLRegionVariableOrigin,
         outlived_fr: RegionVid,
         renctx: &mut RegionErrorNamingCtx,
     ) -> DiagnosticBuilder<'a> {
         debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);
 
-        let (category, _, span) = self.best_blame_constraint(body, fr, |r| {
+        let (category, _, span) = self.best_blame_constraint(body, fr, fr_origin, |r| {
             self.provides_universal_region(r, fr, outlived_fr)
         });
 
@@ -709,6 +778,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         let (category, from_closure, span) = self.best_blame_constraint(
             body,
             borrow_region,
+            NLLRegionVariableOrigin::FreeRegion,
             |r| self.provides_universal_region(r, borrow_region, outlived_region)
         );
 
@@ -768,11 +838,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         &self,
         body: &Body<'tcx>,
         fr1: RegionVid,
+        fr1_origin: NLLRegionVariableOrigin,
         fr2: RegionVid,
     ) -> (ConstraintCategory, Span) {
         let (category, _, span) = self.best_blame_constraint(
             body,
             fr1,
+            fr1_origin,
             |r| self.provides_universal_region(r, fr1, fr2),
         );
         (category, span)
@@ -825,7 +897,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 universe1.cannot_name(placeholder.universe)
             }
 
-            NLLRegionVariableOrigin::FreeRegion | NLLRegionVariableOrigin::Existential => false,
+            NLLRegionVariableOrigin::FreeRegion | NLLRegionVariableOrigin::Existential { .. } => {
+                false
+            }
         }
     }
 }

src/librustc_mir/borrow_check/nll/region_infer/mod.rs

@@ -406,7 +406,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                     }
                 }
 
-                NLLRegionVariableOrigin::Existential => {
+                NLLRegionVariableOrigin::Existential { .. } => {
                     // For existential, regions, nothing to do.
                 }
             }
@@ -1348,7 +1348,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                     self.check_bound_universal_region(infcx, body, mir_def_id, fr, placeholder);
                 }
 
-                NLLRegionVariableOrigin::Existential => {
+                NLLRegionVariableOrigin::Existential { .. } => {
                     // nothing to check here
                 }
             }
@@ -1461,7 +1461,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 debug!("check_universal_region: fr_minus={:?}", fr_minus);
 
                 let blame_span_category =
-                    self.find_outlives_blame_span(body, longer_fr, shorter_fr);
+                    self.find_outlives_blame_span(body, longer_fr,
+                                                  NLLRegionVariableOrigin::FreeRegion,shorter_fr);
 
                 // Grow `shorter_fr` until we find some non-local regions. (We
                 // always will.)  We'll call them `shorter_fr+` -- they're ever
@@ -1494,6 +1495,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             infcx,
             mir_def_id,
             longer_fr,
+            NLLRegionVariableOrigin::FreeRegion,
             shorter_fr,
             region_naming,
         );
@@ -1547,7 +1549,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         };
 
         // Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
-        let (_, span) = self.find_outlives_blame_span(body, longer_fr, error_region);
+        let (_, span) = self.find_outlives_blame_span(
+            body, longer_fr, NLLRegionVariableOrigin::Placeholder(placeholder), error_region
+        );
 
         // Obviously, this error message is far from satisfactory.
         // At present, though, it only appears in unit tests --
@@ -1608,7 +1612,7 @@ impl<'tcx> RegionDefinition<'tcx> {
 
         let origin = match rv_origin {
             RegionVariableOrigin::NLL(origin) => origin,
-            _ => NLLRegionVariableOrigin::Existential,
+            _ => NLLRegionVariableOrigin::Existential { from_forall: false },
         };
 
         Self { origin, universe, external_name: None }

src/librustc_mir/borrow_check/nll/renumber.rs

@@ -35,7 +35,7 @@ where
     infcx
         .tcx
         .fold_regions(value, &mut false, |_region, _depth| {
-            let origin = NLLRegionVariableOrigin::Existential;
+            let origin = NLLRegionVariableOrigin::Existential { from_forall: false };
             infcx.next_nll_region_var(origin)
         })
 }

src/librustc_mir/borrow_check/nll/type_check/relate_tys.rs

@@ -66,9 +66,9 @@ impl TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx> {
         self.infcx.create_next_universe()
     }
 
-    fn next_existential_region_var(&mut self) -> ty::Region<'tcx> {
+    fn next_existential_region_var(&mut self, from_forall: bool) -> ty::Region<'tcx> {
         if let Some(_) = &mut self.borrowck_context {
-            let origin = NLLRegionVariableOrigin::Existential;
+            let origin = NLLRegionVariableOrigin::Existential { from_forall };
             self.infcx.next_nll_region_var(origin)
         } else {
             self.infcx.tcx.lifetimes.re_erased
@@ -88,7 +88,9 @@ impl TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx> {
 
     fn generalize_existential(&mut self, universe: ty::UniverseIndex) -> ty::Region<'tcx> {
         self.infcx
-            .next_nll_region_var_in_universe(NLLRegionVariableOrigin::Existential, universe)
+            .next_nll_region_var_in_universe(NLLRegionVariableOrigin::Existential {
+                from_forall: false
+            }, universe)
     }
 
     fn push_outlives(&mut self, sup: ty::Region<'tcx>, sub: ty::Region<'tcx>) {

src/librustc_traits/chalk_context/unify.rs

@@ -65,7 +65,7 @@ impl TypeRelatingDelegate<'tcx> for &mut ChalkTypeRelatingDelegate<'_, 'tcx> {
         self.infcx.create_next_universe()
     }
 
-    fn next_existential_region_var(&mut self) -> ty::Region<'tcx> {
+    fn next_existential_region_var(&mut self, _was_placeholder: bool) -> ty::Region<'tcx> {
         self.infcx.next_region_var(RegionVariableOrigin::MiscVariable(DUMMY_SP))
     }
 

src/test/ui/hrtb/issue-30786.nll.stderr

@@ -1,14 +1,20 @@
 error: higher-ranked subtype error
-  --> $DIR/issue-30786.rs:113:18
+  --> $DIR/issue-30786.rs:108:15
+   |
+LL |     let map = source.map(|x: &_| x);
+   |               ^^^^^^^^^^^^^^^^^^^^^
+
+error: higher-ranked subtype error
+  --> $DIR/issue-30786.rs:114:18
    |
 LL |     let filter = map.filter(|x: &_| true);
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: higher-ranked subtype error
-  --> $DIR/issue-30786.rs:115:17
+  --> $DIR/issue-30786.rs:116:17
    |
 LL |     let count = filter.count(); // Assert that we still have a valid stream.
    |                 ^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors
 

src/test/ui/hrtb/issue-30786.rs

@@ -106,12 +106,14 @@ impl<T> StreamExt for T where for<'a> &'a mut T: Stream { }
 fn main() {
     let source = Repeat(10);
     let map = source.map(|x: &_| x);
-    //[migrate]~^ ERROR implementation of `Stream` is not general enough
+    //[nll]~^ ERROR higher-ranked subtype error
+    //[migrate]~^^ ERROR implementation of `Stream` is not general enough
     //[migrate]~| NOTE  `Stream` would have to be implemented for the type `&'0 mut Map
     //[migrate]~| NOTE  but `Stream` is actually implemented for the type `&'1
     //[migrate]~| NOTE  implementation of `Stream` is not general enough
     let filter = map.filter(|x: &_| true);
     //[nll]~^ ERROR higher-ranked subtype error
     let count = filter.count(); // Assert that we still have a valid stream.
     //[nll]~^ ERROR higher-ranked subtype error
+
 }

src/test/ui/nll/relate_tys/fn-subtype.rs

@@ -0,0 +1,10 @@
+// Test that NLL produces correct spans for higher-ranked subtyping errors.
+//
+// compile-flags:-Zno-leak-check
+
+#![feature(nll)]
+
+fn main() {
+    let x: fn(&'static ()) = |_| {};
+    let y: for<'a> fn(&'a ()) = x; //~ ERROR higher-ranked subtype error
+}

src/test/ui/nll/relate_tys/fn-subtype.stderr

@@ -0,0 +1,8 @@
+error: higher-ranked subtype error
+  --> $DIR/fn-subtype.rs:9:33
+   |
+LL |     let y: for<'a> fn(&'a ()) = x;
+   |                                 ^
+
+error: aborting due to previous error
+

src/test/ui/nll/relate_tys/trait-hrtb.rs

@@ -0,0 +1,16 @@
+// Test that NLL generates proper error spans for trait HRTB errors
+//
+// compile-flags:-Zno-leak-check
+
+#![feature(nll)]
+
+trait Foo<'a> {}
+
+fn make_foo<'a>() -> Box<dyn Foo<'a>> {
+    panic!()
+}
+
+fn main() {
+    let x: Box<dyn Foo<'static>> = make_foo();
+    let y: Box<dyn for<'a> Foo<'a>> = x; //~ ERROR higher-ranked subtype error
+}

src/test/ui/nll/relate_tys/trait-hrtb.stderr

@@ -0,0 +1,8 @@
+error: higher-ranked subtype error
+  --> $DIR/trait-hrtb.rs:15:39
+   |
+LL |     let y: Box<dyn for<'a> Foo<'a>> = x;
+   |                                       ^
+
+error: aborting due to previous error
+