From 61b52a33b3a5a170f7760ba3e3efaac25e5085ef Mon Sep 17 00:00:00 2001 From: SNCPlay42 Date: Tue, 8 Sep 2020 05:35:24 +0100 Subject: [PATCH] use RegionNameHighlight for async fn and closure returns --- .../borrow_check/diagnostics/region_name.rs | 152 ++++++++++++++---- .../issue-74072-lifetime-name-annotations.rs | 8 + ...sue-74072-lifetime-name-annotations.stderr | 16 +- .../issue-74497-lifetime-in-opaque.rs | 19 +++ .../issue-74497-lifetime-in-opaque.stderr | 11 ++ ...ted-closure-outlives-borrowed-value.stderr | 2 +- src/test/ui/nll/issue-58053.stderr | 4 +- 7 files changed, 173 insertions(+), 39 deletions(-) create mode 100644 src/test/ui/async-await/issue-74497-lifetime-in-opaque.rs create mode 100644 src/test/ui/async-await/issue-74497-lifetime-in-opaque.stderr diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/region_name.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/region_name.rs index db5c14843b9..2a90fb042dd 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/region_name.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/region_name.rs @@ -6,8 +6,8 @@ use rustc_hir::def::{DefKind, Res}; use rustc_middle::ty::print::RegionHighlightMode; use rustc_middle::ty::subst::{GenericArgKind, SubstsRef}; use rustc_middle::ty::{self, RegionVid, Ty}; -use rustc_span::symbol::kw; -use rustc_span::{symbol::Symbol, Span, DUMMY_SP}; +use rustc_span::symbol::{kw, sym, Ident, Symbol}; +use rustc_span::{Span, DUMMY_SP}; use crate::borrow_check::{nll::ToRegionVid, universal_regions::DefiningTy, MirBorrowckCtxt}; @@ -39,7 +39,7 @@ crate enum RegionNameSource { /// The region corresponding to a closure upvar. AnonRegionFromUpvar(Span, String), /// The region corresponding to the return type of a closure. - AnonRegionFromOutput(Span, String, String), + AnonRegionFromOutput(RegionNameHighlight, String), /// The region from a type yielded by a generator. AnonRegionFromYieldTy(Span, String), /// An anonymous region from an async fn. @@ -85,10 +85,10 @@ impl RegionName { | RegionNameSource::NamedFreeRegion(span) | RegionNameSource::SynthesizedFreeEnvRegion(span, _) | RegionNameSource::AnonRegionFromUpvar(span, _) - | RegionNameSource::AnonRegionFromOutput(span, _, _) | RegionNameSource::AnonRegionFromYieldTy(span, _) | RegionNameSource::AnonRegionFromAsyncFn(span) => Some(span), - RegionNameSource::AnonRegionFromArgument(ref highlight) => match *highlight { + RegionNameSource::AnonRegionFromArgument(ref highlight) + | RegionNameSource::AnonRegionFromOutput(ref highlight, _) => match *highlight { RegionNameHighlight::MatchedHirTy(span) | RegionNameHighlight::MatchedAdtAndSegment(span) | RegionNameHighlight::CannotMatchHirTy(span, _) @@ -117,6 +117,7 @@ impl RegionName { diag.span_label(*span, format!("has type `{}`", type_name)); } RegionNameSource::AnonRegionFromArgument(RegionNameHighlight::MatchedHirTy(span)) + | RegionNameSource::AnonRegionFromOutput(RegionNameHighlight::MatchedHirTy(span), _) | RegionNameSource::AnonRegionFromAsyncFn(span) => { diag.span_label( *span, @@ -125,6 +126,10 @@ impl RegionName { } RegionNameSource::AnonRegionFromArgument( RegionNameHighlight::MatchedAdtAndSegment(span), + ) + | RegionNameSource::AnonRegionFromOutput( + RegionNameHighlight::MatchedAdtAndSegment(span), + _, ) => { diag.span_label(*span, format!("let's call this `{}`", self)); } @@ -137,13 +142,28 @@ impl RegionName { format!("lifetime `{}` appears in the type {}", self, type_name), ); } + RegionNameSource::AnonRegionFromOutput( + RegionNameHighlight::Occluded(span, type_name), + mir_description, + ) => { + diag.span_label( + *span, + format!( + "return type{} `{}` contains a lifetime `{}`", + mir_description, type_name, self + ), + ); + } RegionNameSource::AnonRegionFromUpvar(span, upvar_name) => { diag.span_label( *span, format!("lifetime `{}` appears in the type of `{}`", self, upvar_name), ); } - RegionNameSource::AnonRegionFromOutput(span, mir_description, type_name) => { + RegionNameSource::AnonRegionFromOutput( + RegionNameHighlight::CannotMatchHirTy(span, type_name), + mir_description, + ) => { diag.span_label(*span, format!("return type{} is {}", mir_description, type_name)); } RegionNameSource::AnonRegionFromYieldTy(span, type_name) => { @@ -659,6 +679,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { /// or be early bound (named, not in argument). fn give_name_if_anonymous_region_appears_in_output(&self, fr: RegionVid) -> Option { let tcx = self.infcx.tcx; + let hir = tcx.hir(); let return_ty = self.regioncx.universal_regions().unnormalized_output_ty; debug!("give_name_if_anonymous_region_appears_in_output: return_ty = {:?}", return_ty); @@ -666,60 +687,123 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { return None; } - let mut highlight = RegionHighlightMode::default(); - highlight.highlighting_region_vid(fr, *self.next_region_name.try_borrow().unwrap()); - let type_name = - self.infcx.extract_inference_diagnostics_data(return_ty.into(), Some(highlight)).name; + let mir_hir_id = self.mir_hir_id(); - let (return_span, mir_description) = match tcx.hir().get(self.mir_hir_id()) { + let (return_span, mir_description, hir_ty) = match hir.get(mir_hir_id) { hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(_, return_ty, body_id, span, _), .. }) => { - let mut span = match return_ty.output { - hir::FnRetTy::DefaultReturn(_) => tcx.sess.source_map().end_point(*span), - hir::FnRetTy::Return(_) => return_ty.output.span(), + let (mut span, mut hir_ty) = match return_ty.output { + hir::FnRetTy::DefaultReturn(_) => { + (tcx.sess.source_map().end_point(*span), None) + } + hir::FnRetTy::Return(hir_ty) => (return_ty.output.span(), Some(hir_ty)), }; - let mir_description = match tcx.hir().body(*body_id).generator_kind { + let mir_description = match hir.body(*body_id).generator_kind { Some(hir::GeneratorKind::Async(gen)) => match gen { hir::AsyncGeneratorKind::Block => " of async block", hir::AsyncGeneratorKind::Closure => " of async closure", hir::AsyncGeneratorKind::Fn => { - span = tcx - .hir() - .get(tcx.hir().get_parent_item(mir_hir_id)) + let parent_item = hir.get(hir.get_parent_item(mir_hir_id)); + let output = &parent_item .fn_decl() .expect("generator lowered from async fn should be in fn") - .output - .span(); + .output; + span = output.span(); + if let hir::FnRetTy::Return(ret) = output { + hir_ty = Some(self.get_future_inner_return_ty(*ret)); + } " of async function" } }, Some(hir::GeneratorKind::Gen) => " of generator", None => " of closure", }; - (span, mir_description) + (span, mir_description, hir_ty) } - hir::Node::ImplItem(hir::ImplItem { - kind: hir::ImplItemKind::Fn(method_sig, _), - .. - }) => (method_sig.decl.output.span(), ""), - _ => (self.body.span, ""), + node => match node.fn_decl() { + Some(fn_decl) => { + let hir_ty = match fn_decl.output { + hir::FnRetTy::DefaultReturn(_) => None, + hir::FnRetTy::Return(ty) => Some(ty), + }; + (fn_decl.output.span(), "", hir_ty) + } + None => (self.body.span, "", None), + }, }; + let highlight = hir_ty + .and_then(|hir_ty| self.highlight_if_we_can_match_hir_ty(fr, return_ty, hir_ty)) + .unwrap_or_else(|| { + // `highlight_if_we_cannot_match_hir_ty` needs to know the number we will give to + // the anonymous region. If it succeeds, the `synthesize_region_name` call below + // will increment the counter, "reserving" the number we just used. + let counter = *self.next_region_name.try_borrow().unwrap(); + self.highlight_if_we_cannot_match_hir_ty(fr, return_ty, return_span, counter) + }); + Some(RegionName { - // This counter value will already have been used, so this function will increment it - // so the next value will be used next and return the region name that would have been - // used. name: self.synthesize_region_name(), - source: RegionNameSource::AnonRegionFromOutput( - return_span, - mir_description.to_string(), - type_name, - ), + source: RegionNameSource::AnonRegionFromOutput(highlight, mir_description.to_string()), }) } + /// From the [`hir::Ty`] of an async function's lowered return type, + /// retrieve the `hir::Ty` representing the type the user originally wrote. + /// + /// e.g. given the function: + /// + /// ``` + /// async fn foo() -> i32 {} + /// ``` + /// + /// this function, given the lowered return type of `foo`, an [`OpaqueDef`] that implements `Future`, + /// returns the `i32`. + /// + /// [`OpaqueDef`]: hir::TyKind::OpaqueDef + fn get_future_inner_return_ty(&self, hir_ty: &'tcx hir::Ty<'tcx>) -> &'tcx hir::Ty<'tcx> { + let hir = self.infcx.tcx.hir(); + + if let hir::TyKind::OpaqueDef(id, _) = hir_ty.kind { + let opaque_ty = hir.item(id.id); + if let hir::ItemKind::OpaqueTy(hir::OpaqueTy { + bounds: + [hir::GenericBound::LangItemTrait( + hir::LangItem::Future, + _, + _, + hir::GenericArgs { + bindings: + [hir::TypeBinding { + ident: Ident { name: sym::Output, .. }, + kind: hir::TypeBindingKind::Equality { ty }, + .. + }], + .. + }, + )], + .. + }) = opaque_ty.kind + { + ty + } else { + span_bug!( + hir_ty.span, + "bounds from lowered return type of async fn did not match expected format: {:?}", + opaque_ty + ); + } + } else { + span_bug!( + hir_ty.span, + "lowered return type of async fn is not OpaqueDef: {:?}", + hir_ty + ); + } + } + fn give_name_if_anonymous_region_appears_in_yield_ty( &self, fr: RegionVid, diff --git a/src/test/ui/async-await/issue-74072-lifetime-name-annotations.rs b/src/test/ui/async-await/issue-74072-lifetime-name-annotations.rs index f8e11633359..95683241aba 100644 --- a/src/test/ui/async-await/issue-74072-lifetime-name-annotations.rs +++ b/src/test/ui/async-await/issue-74072-lifetime-name-annotations.rs @@ -18,6 +18,14 @@ pub fn async_closure(x: &mut i32) -> impl Future { })() } +pub fn async_closure_explicit_return_type(x: &mut i32) -> impl Future { + (async move || -> &i32 { + let y = &*x; + *x += 1; //~ ERROR cannot assign to `*x` because it is borrowed + y + })() +} + pub fn async_block(x: &mut i32) -> impl Future { async move { let y = &*x; diff --git a/src/test/ui/async-await/issue-74072-lifetime-name-annotations.stderr b/src/test/ui/async-await/issue-74072-lifetime-name-annotations.stderr index 997e36fe22c..123c3192cff 100644 --- a/src/test/ui/async-await/issue-74072-lifetime-name-annotations.stderr +++ b/src/test/ui/async-await/issue-74072-lifetime-name-annotations.stderr @@ -2,7 +2,7 @@ error[E0506]: cannot assign to `*x` because it is borrowed --> $DIR/issue-74072-lifetime-name-annotations.rs:9:5 | LL | pub async fn async_fn(x: &mut i32) -> &i32 { - | ---- return type of async function is &'1 i32 + | - let's call the lifetime of this reference `'1` LL | let y = &*x; | --- borrow of `*x` occurs here LL | *x += 1; @@ -25,6 +25,18 @@ LL | })() error[E0506]: cannot assign to `*x` because it is borrowed --> $DIR/issue-74072-lifetime-name-annotations.rs:24:9 | +LL | (async move || -> &i32 { + | - let's call the lifetime of this reference `'1` +LL | let y = &*x; + | --- borrow of `*x` occurs here +LL | *x += 1; + | ^^^^^^^ assignment to borrowed `*x` occurs here +LL | y + | - returning this value requires that `*x` is borrowed for `'1` + +error[E0506]: cannot assign to `*x` because it is borrowed + --> $DIR/issue-74072-lifetime-name-annotations.rs:32:9 + | LL | let y = &*x; | --- borrow of `*x` occurs here LL | *x += 1; @@ -34,6 +46,6 @@ LL | y LL | } | - return type of async block is &'1 i32 -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0506`. diff --git a/src/test/ui/async-await/issue-74497-lifetime-in-opaque.rs b/src/test/ui/async-await/issue-74497-lifetime-in-opaque.rs new file mode 100644 index 00000000000..2d765eb41be --- /dev/null +++ b/src/test/ui/async-await/issue-74497-lifetime-in-opaque.rs @@ -0,0 +1,19 @@ +// edition:2018 + +// test that names give to anonymous lifetimes in opaque types like `impl Future` are correctly +// introduced in error messages + +use std::future::Future; + +pub async fn foo(_: F) +where + F: Fn(&u8) -> T, + T: Future, +{ +} + +pub async fn bar(_: &u8) {} + +fn main() { + let _ = foo(|x| bar(x)); //~ ERROR lifetime may not live long enough +} diff --git a/src/test/ui/async-await/issue-74497-lifetime-in-opaque.stderr b/src/test/ui/async-await/issue-74497-lifetime-in-opaque.stderr new file mode 100644 index 00000000000..89fe1abb365 --- /dev/null +++ b/src/test/ui/async-await/issue-74497-lifetime-in-opaque.stderr @@ -0,0 +1,11 @@ +error: lifetime may not live long enough + --> $DIR/issue-74497-lifetime-in-opaque.rs:18:21 + | +LL | let _ = foo(|x| bar(x)); + | -- ^^^^^^ returning this value requires that `'1` must outlive `'2` + | || + | |return type of closure `impl Future` contains a lifetime `'2` + | has type `&'1 u8` + +error: aborting due to previous error + diff --git a/src/test/ui/borrowck/issue-53432-nested-closure-outlives-borrowed-value.stderr b/src/test/ui/borrowck/issue-53432-nested-closure-outlives-borrowed-value.stderr index e8a026cfab9..a7c3b9eec73 100644 --- a/src/test/ui/borrowck/issue-53432-nested-closure-outlives-borrowed-value.stderr +++ b/src/test/ui/borrowck/issue-53432-nested-closure-outlives-borrowed-value.stderr @@ -4,7 +4,7 @@ error: lifetime may not live long enough LL | let _action = move || { | ------- | | | - | | return type of closure is [closure@$DIR/issue-53432-nested-closure-outlives-borrowed-value.rs:4:9: 4:15] + | | return type of closure `[closure@$DIR/issue-53432-nested-closure-outlives-borrowed-value.rs:4:9: 4:15]` contains a lifetime `'2` | lifetime `'1` represents this closure's body LL | || f() // The `nested` closure | ^^^^^^ returning this value requires that `'1` must outlive `'2` diff --git a/src/test/ui/nll/issue-58053.stderr b/src/test/ui/nll/issue-58053.stderr index 297681ff403..e41ee8a8970 100644 --- a/src/test/ui/nll/issue-58053.stderr +++ b/src/test/ui/nll/issue-58053.stderr @@ -2,9 +2,9 @@ error: lifetime may not live long enough --> $DIR/issue-58053.rs:6:33 | LL | let f = |x: &i32| -> &i32 { x }; - | - ---- ^ returning this value requires that `'1` must outlive `'2` + | - - ^ returning this value requires that `'1` must outlive `'2` | | | - | | return type of closure is &'2 i32 + | | let's call the lifetime of this reference `'2` | let's call the lifetime of this reference `'1` error: lifetime may not live long enough