From ab45ab83ac0c9b19b6d692ca5d2e9b7b98c3565a Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Tue, 12 Oct 2021 08:56:24 +0000 Subject: [PATCH] review comments * take diagnostic logic out of happy-path * sort/dedup once * add more comments --- .../nice_region_error/static_impl_trait.rs | 10 ++++- .../src/infer/lexical_region_resolve/mod.rs | 40 ++++++++++--------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs index 3f7aa7773d1..e1f2ec44431 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -150,8 +150,10 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { if mention_capture { spans.push(sup_origin.span()); } - spans.sort(); - spans.dedup(); + // We sort the spans *ignoring* expansion context. Below, the closure logic is repeated + // because one method expects a closure taking `&Span` and the other `&mut Span`. + spans.sort_by_key(|span| (span.lo(), span.hi())); + spans.dedup_by_key(|span| (span.lo(), span.hi())); // We try to make the output have fewer overlapping spans if possible. let (require_msg, require_span) = if sup_origin.span().overlaps(return_sp) { @@ -165,8 +167,12 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { } if spans.iter().any(|sp| sp.overlaps(return_sp) || *sp > return_sp) { + // If any of the "captured here" labels appears on the same line or after + // `require_span`, we put it on a note to ensure the text flows by appearing + // always at the end. err.span_note(require_span, require_msg); } else { + // We don't need a note, it's already at the end, it can be shown as a `span_label`. err.span_label(require_span, require_msg); } diff --git a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs index 14f6c72bb1c..3d4e96fa0f2 100644 --- a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs +++ b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs @@ -146,8 +146,8 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { let graph = self.construct_graph(); self.expand_givens(&graph); self.expansion(&mut var_data); - let captures = self.collect_errors(&mut var_data, errors); - self.collect_var_errors(&var_data, &graph, errors, captures); + self.collect_errors(&mut var_data, errors); + self.collect_var_errors(&var_data, &graph, errors); var_data } @@ -445,16 +445,9 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { &self, var_data: &mut LexicalRegionResolutions<'tcx>, errors: &mut Vec>, - ) -> Vec { - let mut captures = vec![]; - + ) { for (constraint, origin) in &self.data.constraints { debug!(?constraint, ?origin); - if let (Constraint::VarSubVar(_, _), SubregionOrigin::DataBorrowed(_, sp)) = - (constraint, origin) - { - captures.push(*sp); - } match *constraint { Constraint::RegSubVar(..) | Constraint::VarSubVar(..) => { // Expansion will ensure that these constraints hold. Ignore. @@ -524,7 +517,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { sub, )); } - captures } /// Go over the variables that were declared to be error variables @@ -534,7 +526,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { var_data: &LexicalRegionResolutions<'tcx>, graph: &RegionGraph<'tcx>, errors: &mut Vec>, - captures: Vec, ) { debug!("collect_var_errors, var_data = {:#?}", var_data.values); @@ -578,12 +569,27 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { // if this rule starts to create problems we'll // have to revisit this portion of the code and // think hard about it. =) -- nikomatsakis + + // Obtain the spans for all the capture points for + // richer diagnostics in `static_impl_trait`. + let captures: Vec = self + .data + .constraints + .iter() + .filter_map(|(constraint, origin)| match (constraint, origin) { + (Constraint::VarSubVar(_, _), SubregionOrigin::DataBorrowed(_, sp)) => { + Some(*sp) + } + _ => None, + }) + .collect(); + self.collect_error_for_expanding_node( graph, &mut dup_vec, node_vid, errors, - &captures, + captures, ); } } @@ -638,7 +644,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { dup_vec: &mut IndexVec>, node_idx: RegionVid, errors: &mut Vec>, - captures: &[Span], + captures: Vec, ) { // Errors in expanding nodes result from a lower-bound that is // not contained by an upper-bound. @@ -686,10 +692,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { origin, node_idx, lower_bound.region, upper_bound.region ); - let mut capture_spans: Vec = captures.iter().cloned().collect(); - // Below, one span expects `&Span` and the other `&mut Span`, hence the dupes. - capture_spans.sort_by_key(|span| (span.lo(), span.hi())); - capture_spans.dedup_by_key(|span| (span.lo(), span.hi())); errors.push(RegionResolutionError::SubSupConflict( node_idx, origin, @@ -697,7 +699,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { lower_bound.region, upper_bound.origin.clone(), upper_bound.region, - capture_spans, + captures, )); return; }