From 05e3d200801a6236ddbb5b467b3b12a1b4ff15b2 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 11 Dec 2019 16:40:49 -0600 Subject: [PATCH] Refactor region error handling to be done by mirborrowckctx --- .../borrow_check/diagnostics/mod.rs | 2 +- .../diagnostics/outlives_suggestion.rs | 4 +- .../borrow_check/diagnostics/region_errors.rs | 68 +++- src/librustc_mir/borrow_check/mod.rs | 177 ++++++++-- src/librustc_mir/borrow_check/nll.rs | 68 ++-- .../borrow_check/region_infer/mod.rs | 304 +++++------------- .../hrtb/hrtb-perfect-forwarding.nll.stderr | 12 +- 7 files changed, 345 insertions(+), 290 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/mod.rs b/src/librustc_mir/borrow_check/diagnostics/mod.rs index 0fa79051b783a..cd2138fdf94d3 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mod.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mod.rs @@ -32,7 +32,7 @@ mod region_errors; crate use mutability_errors::AccessKind; crate use outlives_suggestion::OutlivesSuggestionBuilder; -crate use region_errors::{ErrorConstraintInfo, ErrorReportingCtx}; +crate use region_errors::{ErrorConstraintInfo, ErrorReportingCtx, RegionErrorKind, RegionErrors}; crate use region_name::{RegionErrorNamingCtx, RegionName, RegionNameSource}; pub(super) struct IncludingDowncast(pub(super) bool); diff --git a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs index 40fe75d0666fb..8acb2feeb70a9 100644 --- a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs +++ b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs @@ -243,7 +243,9 @@ impl OutlivesSuggestionBuilder<'a> { // If there is only one constraint to suggest, then we already suggested it in the // intermediate suggestion above. - if self.constraints_to_add.len() == 1 { + if self.constraints_to_add.len() == 1 + && self.constraints_to_add.values().next().unwrap().len() == 1 + { debug!("Only 1 suggestion. Skipping."); return; } diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index bf8e51befa25e..e61581336283c 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -2,10 +2,11 @@ use rustc::hir::def_id::DefId; use rustc::infer::{ - error_reporting::nice_region_error::NiceRegionError, InferCtxt, NLLRegionVariableOrigin, + error_reporting::nice_region_error::NiceRegionError, region_constraints::GenericKind, + InferCtxt, NLLRegionVariableOrigin, }; use rustc::mir::{Body, ConstraintCategory, Local, Location}; -use rustc::ty::{self, RegionVid}; +use rustc::ty::{self, RegionVid, Ty}; use rustc_errors::DiagnosticBuilder; use rustc_index::vec::IndexVec; use std::collections::VecDeque; @@ -47,13 +48,74 @@ impl ConstraintDescription for ConstraintCategory { } } -#[derive(Copy, Clone, PartialEq, Eq)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] enum Trace { StartRegion, FromOutlivesConstraint(OutlivesConstraint), NotVisited, } +/// A collection of errors encountered during region inference. This is needed to efficiently +/// report errors after borrow checking. +/// +/// Usually we expect this to either be empty or contain a small number of items, so we can avoid +/// allocation most of the time. +crate type RegionErrors<'tcx> = Vec>; + +#[derive(Clone, Debug)] +crate enum RegionErrorKind<'tcx> { + /// An error for a type test: `T: 'a` does not live long enough. + TypeTestDoesNotLiveLongEnough { + /// The span of the type test. + span: Span, + /// The generic type of the type test. + generic: GenericKind<'tcx>, + }, + + /// A generic bound failure for a type test. + TypeTestGenericBoundError { + /// The span of the type test. + span: Span, + /// The generic type of the type test. + generic: GenericKind<'tcx>, + /// The lower bound region. + lower_bound_region: ty::Region<'tcx>, + }, + + /// An unexpected hidden region for an opaque type. + UnexpectedHiddenRegion { + /// The def id of the opaque type. + opaque_type_def_id: DefId, + /// The hidden type. + hidden_ty: Ty<'tcx>, + /// The unexpected region. + member_region: ty::Region<'tcx>, + }, + + /// Higher-ranked subtyping error. + BoundUniversalRegionError { + /// The placeholder free region. + longer_fr: RegionVid, + /// The region that erroneously must be outlived by `longer_fr`. + error_region: RegionVid, + /// The origin of the placeholder region. + fr_origin: NLLRegionVariableOrigin, + }, + + /// Any other lifetime error. + RegionError { + /// The origin of the region. + fr_origin: NLLRegionVariableOrigin, + /// The region that should outlive `shorter_fr`. + longer_fr: RegionVid, + /// The region that should be shorter, but we can't prove it. + shorter_fr: RegionVid, + /// Indicates whether this is a reported error. We currently only report the first error + /// encountered and leave the rest unreported so as not to overwhelm the user. + is_reported: bool, + }, +} + /// Various pieces of state used when reporting borrow checker errors. pub struct ErrorReportingCtx<'a, 'b, 'tcx> { /// The region inference context used for borrow chekcing this MIR body. diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 4a6379e3bc155..b12ed6d58b126 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1,9 +1,7 @@ //! This query borrow-checks the MIR to (further) ensure it is not broken. -use rustc::hir::def_id::DefId; -use rustc::hir::Node; -use rustc::hir::{self, HirId}; -use rustc::infer::InferCtxt; +use rustc::hir::{self, def_id::DefId, HirId, Node}; +use rustc::infer::{opaque_types, InferCtxt}; use rustc::lint::builtin::MUTABLE_BORROW_RESERVATION_CONFLICT; use rustc::lint::builtin::UNUSED_MUT; use rustc::mir::{ @@ -39,8 +37,11 @@ use crate::dataflow::FlowAtLocation; use crate::dataflow::MoveDataParamEnv; use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; +use crate::transform::MirSource; -use self::diagnostics::AccessKind; +use self::diagnostics::{ + AccessKind, OutlivesSuggestionBuilder, RegionErrorKind, RegionErrorNamingCtx, RegionErrors, +}; use self::flows::Flows; use self::location::LocationTable; use self::prefixes::PrefixSet; @@ -202,22 +203,28 @@ fn do_mir_borrowck<'a, 'tcx>( let borrow_set = Rc::new(BorrowSet::build(tcx, body, locals_are_invalidated_at_exit, &mdpe.move_data)); - // If we are in non-lexical mode, compute the non-lexical lifetimes. - let (regioncx, polonius_output, opt_closure_req) = nll::compute_regions( - infcx, - def_id, - free_regions, - body, - &promoted, - &local_names, - &upvars, - location_table, - param_env, - &mut flow_inits, - &mdpe.move_data, - &borrow_set, - &mut errors_buffer, - ); + // Compute non-lexical lifetimes. + let nll::NllOutput { regioncx, polonius_output, opt_closure_req, nll_errors } = + nll::compute_regions( + infcx, + def_id, + free_regions, + body, + &promoted, + location_table, + param_env, + &mut flow_inits, + &mdpe.move_data, + &borrow_set, + ); + + // Dump MIR results into a file, if that is enabled. This let us + // write unit-tests, as well as helping with debugging. + nll::dump_mir_results(infcx, MirSource::item(def_id), &body, ®ioncx, &opt_closure_req); + + // We also have a `#[rustc_nll]` annotation that causes us to dump + // information. + nll::dump_annotation(infcx, &body, def_id, ®ioncx, &opt_closure_req, &mut errors_buffer); // The various `flow_*` structures can be large. We drop `flow_inits` here // so it doesn't overlap with the others below. This reduces peak memory @@ -288,6 +295,9 @@ fn do_mir_borrowck<'a, 'tcx>( local_names, }; + // Compute and report region errors, if any. + mbcx.report_region_errors(nll_errors); + let mut state = Flows::new(flow_borrows, flow_uninits, flow_ever_inits, polonius_output); if let Some(errors) = move_errors { @@ -1464,6 +1474,131 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // initial reservation. } } + + /// Produces nice borrowck error diagnostics for all the errors collected in `nll_errors`. + fn report_region_errors(&mut self, nll_errors: RegionErrors<'tcx>) { + // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are + // buffered in the `MirBorrowckCtxt`. + + // FIXME(mark-i-m): Would be great to get rid of the naming context. + let mut region_naming = RegionErrorNamingCtx::new(); + let mut outlives_suggestion = + OutlivesSuggestionBuilder::new(self.mir_def_id, &self.local_names); + + for nll_error in nll_errors.into_iter() { + match nll_error { + RegionErrorKind::TypeTestDoesNotLiveLongEnough { span, generic } => { + // FIXME. We should handle this case better. It + // indicates that we have e.g., some region variable + // whose value is like `'a+'b` where `'a` and `'b` are + // distinct unrelated univesal regions that are not + // known to outlive one another. It'd be nice to have + // some examples where this arises to decide how best + // to report it; we could probably handle it by + // iterating over the universal regions and reporting + // an error that multiple bounds are required. + self.infcx + .tcx + .sess + .struct_span_err(span, &format!("`{}` does not live long enough", generic)) + .buffer(&mut self.errors_buffer); + } + + RegionErrorKind::TypeTestGenericBoundError { + span, + generic, + lower_bound_region, + } => { + let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); + self.infcx + .construct_generic_bound_failure( + region_scope_tree, + span, + None, + generic, + lower_bound_region, + ) + .buffer(&mut self.errors_buffer); + } + + RegionErrorKind::UnexpectedHiddenRegion { + opaque_type_def_id, + hidden_ty, + member_region, + } => { + let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); + opaque_types::unexpected_hidden_region_diagnostic( + self.infcx.tcx, + Some(region_scope_tree), + opaque_type_def_id, + hidden_ty, + member_region, + ) + .buffer(&mut self.errors_buffer); + } + + RegionErrorKind::BoundUniversalRegionError { + longer_fr, + fr_origin, + error_region, + } => { + // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. + let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span( + &self.body, + longer_fr, + fr_origin, + error_region, + ); + + // FIXME: improve this error message + self.infcx + .tcx + .sess + .struct_span_err(span, "higher-ranked subtype error") + .buffer(&mut self.errors_buffer); + } + + RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => { + if is_reported { + let db = self.nonlexical_regioncx.report_error( + &self.body, + &self.local_names, + &self.upvars, + self.infcx, + self.mir_def_id, + longer_fr, + fr_origin, + shorter_fr, + &mut outlives_suggestion, + &mut region_naming, + ); + + db.buffer(&mut self.errors_buffer); + } else { + // We only report the first error, so as not to overwhelm the user. See + // `RegRegionErrorKind` docs. + // + // FIXME: currently we do nothing with these, but perhaps we can do better? + // FIXME: try collecting these constraints on the outlives suggestion + // builder. Does it make the suggestions any better? + debug!( + "Unreported region error: can't prove that {:?}: {:?}", + longer_fr, shorter_fr + ); + } + } + } + } + + // Emit one outlives suggestions for each MIR def we borrowck + outlives_suggestion.add_suggestion( + &self.body, + &self.nonlexical_regioncx, + self.infcx, + &mut self.errors_buffer, + &mut region_naming, + ); + } } impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { diff --git a/src/librustc_mir/borrow_check/nll.rs b/src/librustc_mir/borrow_check/nll.rs index ba387cfbbe80d..04a53bafe052b 100644 --- a/src/librustc_mir/borrow_check/nll.rs +++ b/src/librustc_mir/borrow_check/nll.rs @@ -3,8 +3,8 @@ use rustc::hir::def_id::DefId; use rustc::infer::InferCtxt; use rustc::mir::{ - BasicBlock, Body, BodyAndCache, ClosureOutlivesSubject, ClosureRegionRequirements, Local, - LocalKind, Location, Promoted, ReadOnlyBodyAndCache, + BasicBlock, Body, BodyAndCache, ClosureOutlivesSubject, ClosureRegionRequirements, LocalKind, + Location, Promoted, ReadOnlyBodyAndCache, }; use rustc::ty::{self, RegionKind, RegionVid}; use rustc_errors::Diagnostic; @@ -16,7 +16,6 @@ use std::path::PathBuf; use std::rc::Rc; use std::str::FromStr; use syntax::symbol::sym; -use syntax_pos::symbol::Symbol; use self::mir_util::PassWhere; use polonius_engine::{Algorithm, Output}; @@ -31,6 +30,7 @@ use crate::util::pretty; use crate::borrow_check::{ borrow_set::BorrowSet, constraint_generation, + diagnostics::RegionErrors, facts::{AllFacts, AllFactsExt, RustcFacts}, invalidation, location::LocationTable, @@ -38,14 +38,21 @@ use crate::borrow_check::{ renumber, type_check::{self, MirTypeckRegionConstraints, MirTypeckResults}, universal_regions::UniversalRegions, - Upvar, }; crate type PoloniusOutput = Output; -/// Rewrites the regions in the MIR to use NLL variables, also -/// scraping out the set of universal regions (e.g., region parameters) -/// declared on the function. That set will need to be given to +/// The output of `nll::compute_regions`. This includes the computed `RegionInferenceContext`, any +/// closure requirements to propagate, and any generated errors. +crate struct NllOutput<'tcx> { + pub regioncx: RegionInferenceContext<'tcx>, + pub polonius_output: Option>, + pub opt_closure_req: Option>, + pub nll_errors: RegionErrors<'tcx>, +} + +/// Rewrites the regions in the MIR to use NLL variables, also scraping out the set of universal +/// regions (e.g., region parameters) declared on the function. That set will need to be given to /// `compute_regions`. pub(in crate::borrow_check) fn replace_regions_in_mir<'cx, 'tcx>( infcx: &InferCtxt<'cx, 'tcx>, @@ -140,19 +147,12 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( universal_regions: UniversalRegions<'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, promoted: &IndexVec>, - local_names: &IndexVec>, - upvars: &[Upvar], location_table: &LocationTable, param_env: ty::ParamEnv<'tcx>, flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'cx, 'tcx>>, move_data: &MoveData<'tcx>, borrow_set: &BorrowSet<'tcx>, - errors_buffer: &mut Vec, -) -> ( - RegionInferenceContext<'tcx>, - Option>, - Option>, -) { +) -> NllOutput<'tcx> { let mut all_facts = AllFacts::enabled(infcx.tcx).then_some(AllFacts::default()); let universal_regions = Rc::new(universal_regions); @@ -284,34 +284,18 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( }); // Solve the region constraints. - let closure_region_requirements = regioncx.solve( - infcx, - &body, - local_names, - upvars, - def_id, - errors_buffer, - polonius_output.clone(), - ); - - // Dump MIR results into a file, if that is enabled. This let us - // write unit-tests, as well as helping with debugging. - dump_mir_results( - infcx, - MirSource::item(def_id), - &body, - ®ioncx, - &closure_region_requirements, - ); - - // We also have a `#[rustc_nll]` annotation that causes us to dump - // information - dump_annotation(infcx, &body, def_id, ®ioncx, &closure_region_requirements, errors_buffer); - - (regioncx, polonius_output, closure_region_requirements) + let (closure_region_requirements, nll_errors) = + regioncx.solve(infcx, &body, def_id, polonius_output.clone()); + + NllOutput { + regioncx, + polonius_output, + opt_closure_req: closure_region_requirements, + nll_errors, + } } -fn dump_mir_results<'a, 'tcx>( +pub(super) fn dump_mir_results<'a, 'tcx>( infcx: &InferCtxt<'a, 'tcx>, source: MirSource<'tcx>, body: &Body<'tcx>, @@ -362,7 +346,7 @@ fn dump_mir_results<'a, 'tcx>( }; } -fn dump_annotation<'a, 'tcx>( +pub(super) fn dump_annotation<'a, 'tcx>( infcx: &InferCtxt<'a, 'tcx>, body: &Body<'tcx>, mir_def_id: DefId, diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 6d732238c4768..746b11733b36f 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -2,7 +2,6 @@ use std::rc::Rc; use rustc::hir::def_id::DefId; use rustc::infer::canonical::QueryOutlivesConstraint; -use rustc::infer::opaque_types; use rustc::infer::region_constraints::{GenericKind, VarInfos, VerifyBound}; use rustc::infer::{InferCtxt, NLLRegionVariableOrigin, RegionVariableOrigin}; use rustc::mir::{ @@ -10,23 +9,20 @@ use rustc::mir::{ ConstraintCategory, Local, Location, }; use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; -use rustc::util::common::ErrorReported; use rustc_data_structures::binary_search_util; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::graph::scc::Sccs; use rustc_data_structures::graph::vec_graph::VecGraph; use rustc_data_structures::graph::WithSuccessors; -use rustc_errors::{Diagnostic, DiagnosticBuilder}; use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; -use syntax_pos::symbol::Symbol; use syntax_pos::Span; use crate::borrow_check::{ constraints::{ graph::NormalConstraintGraph, ConstraintSccIndex, OutlivesConstraint, OutlivesConstraintSet, }, - diagnostics::{OutlivesSuggestionBuilder, RegionErrorNamingCtx}, + diagnostics::{RegionErrorKind, RegionErrors}, member_constraints::{MemberConstraintSet, NllMemberConstraintIndex}, nll::{PoloniusOutput, ToRegionVid}, region_infer::values::{ @@ -35,7 +31,6 @@ use crate::borrow_check::{ }, type_check::{free_region_relations::UniversalRegionRelations, Locations}, universal_regions::UniversalRegions, - Upvar, }; mod dump_mir; @@ -221,6 +216,15 @@ pub struct TypeTest<'tcx> { pub verify_bound: VerifyBound<'tcx>, } +/// When we have an unmet lifetime constraint, we try to propagate it outward (e.g. to a closure +/// environment). If we can't, it is an error. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum RegionRelationCheckResult { + Ok, + Propagated, + Error, +} + impl<'tcx> RegionInferenceContext<'tcx> { /// Creates a new region inference context with a total of /// `num_region_variables` valid inference variables; the first N @@ -423,7 +427,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } /// Adds annotations for `#[rustc_regions]`; see `UniversalRegions::annotate`. - crate fn annotate(&self, tcx: TyCtxt<'tcx>, err: &mut DiagnosticBuilder<'_>) { + crate fn annotate(&self, tcx: TyCtxt<'tcx>, err: &mut rustc_errors::DiagnosticBuilder<'_>) { self.universal_regions.annotate(tcx, err) } @@ -469,14 +473,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { &mut self, infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], mir_def_id: DefId, - errors_buffer: &mut Vec, polonius_output: Option>, - ) -> Option> { + ) -> (Option>, RegionErrors<'tcx>) { self.propagate_constraints(body); + let mut errors_buffer = RegionErrors::new(); + // If this is a closure, we can propagate unsatisfied // `outlives_requirements` to our creator, so create a vector // to store those. Otherwise, we'll pass in `None` to the @@ -484,57 +487,34 @@ impl<'tcx> RegionInferenceContext<'tcx> { // eagerly. let mut outlives_requirements = infcx.tcx.is_closure(mir_def_id).then(|| vec![]); - self.check_type_tests( - infcx, - body, - mir_def_id, - outlives_requirements.as_mut(), - errors_buffer, - ); - - // If we produce any errors, we keep track of the names of all regions, so that we can use - // the same error names in any suggestions we produce. Note that we need names to be unique - // across different errors for the same MIR def so that we can make suggestions that fix - // multiple problems. - let mut region_naming = RegionErrorNamingCtx::new(); + self.check_type_tests(infcx, body, outlives_requirements.as_mut(), &mut errors_buffer); // In Polonius mode, the errors about missing universal region relations are in the output // and need to be emitted or propagated. Otherwise, we need to check whether the // constraints were too strong, and if so, emit or propagate those errors. if infcx.tcx.sess.opts.debugging_opts.polonius { self.check_polonius_subset_errors( - infcx, body, - local_names, - upvars, - mir_def_id, outlives_requirements.as_mut(), - errors_buffer, - &mut region_naming, + &mut errors_buffer, polonius_output.expect("Polonius output is unavailable despite `-Z polonius`"), ); } else { - self.check_universal_regions( - infcx, - body, - local_names, - upvars, - mir_def_id, - outlives_requirements.as_mut(), - errors_buffer, - &mut region_naming, - ); + self.check_universal_regions(body, outlives_requirements.as_mut(), &mut errors_buffer); } - self.check_member_constraints(infcx, mir_def_id, errors_buffer); + self.check_member_constraints(infcx, &mut errors_buffer); let outlives_requirements = outlives_requirements.unwrap_or(vec![]); if outlives_requirements.is_empty() { - None + (None, errors_buffer) } else { let num_external_vids = self.universal_regions.num_global_and_external_regions(); - Some(ClosureRegionRequirements { num_external_vids, outlives_requirements }) + ( + Some(ClosureRegionRequirements { num_external_vids, outlives_requirements }), + errors_buffer, + ) } } @@ -822,9 +802,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, - errors_buffer: &mut Vec, + errors_buffer: &mut RegionErrors<'tcx>, ) { let tcx = infcx.tcx; @@ -882,32 +861,16 @@ impl<'tcx> RegionInferenceContext<'tcx> { } if let Some(lower_bound_region) = lower_bound_region { - let region_scope_tree = &tcx.region_scope_tree(mir_def_id); - infcx - .construct_generic_bound_failure( - region_scope_tree, - type_test_span, - None, - type_test.generic_kind, - lower_bound_region, - ) - .buffer(errors_buffer); + errors_buffer.push(RegionErrorKind::TypeTestGenericBoundError { + span: type_test_span, + generic: type_test.generic_kind, + lower_bound_region, + }); } else { - // FIXME. We should handle this case better. It - // indicates that we have e.g., some region variable - // whose value is like `'a+'b` where `'a` and `'b` are - // distinct unrelated univesal regions that are not - // known to outlive one another. It'd be nice to have - // some examples where this arises to decide how best - // to report it; we could probably handle it by - // iterating over the universal regions and reporting - // an error that multiple bounds are required. - tcx.sess - .struct_span_err( - type_test_span, - &format!("`{}` does not live long enough", type_test.generic_kind,), - ) - .buffer(errors_buffer); + errors_buffer.push(RegionErrorKind::TypeTestDoesNotLiveLongEnough { + span: type_test_span, + generic: type_test.generic_kind, + }); } } } @@ -1300,17 +1263,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// report them as errors. fn check_universal_regions( &self, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], - mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, - errors_buffer: &mut Vec, - region_naming: &mut RegionErrorNamingCtx, + errors_buffer: &mut RegionErrors<'tcx>, ) { - let mut outlives_suggestion = OutlivesSuggestionBuilder::new(mir_def_id, local_names); - for (fr, fr_definition) in self.definitions.iter_enumerated() { match fr_definition.origin { NLLRegionVariableOrigin::FreeRegion => { @@ -1318,21 +1274,15 @@ impl<'tcx> RegionInferenceContext<'tcx> { // they did not grow too large, accumulating any requirements // for our caller into the `outlives_requirements` vector. self.check_universal_region( - infcx, body, - local_names, - upvars, - mir_def_id, fr, &mut propagated_outlives_requirements, - &mut outlives_suggestion, errors_buffer, - region_naming, ); } NLLRegionVariableOrigin::Placeholder(placeholder) => { - self.check_bound_universal_region(infcx, body, mir_def_id, fr, placeholder); + self.check_bound_universal_region(fr, placeholder, errors_buffer); } NLLRegionVariableOrigin::Existential { .. } => { @@ -1340,9 +1290,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } } - - // Emit outlives suggestions - outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, region_naming); } /// Checks if Polonius has found any unexpected free region relations. @@ -1368,14 +1315,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// report them as errors. fn check_polonius_subset_errors( &self, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], - mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, - errors_buffer: &mut Vec, - region_naming: &mut RegionErrorNamingCtx, + errors_buffer: &mut RegionErrors<'tcx>, polonius_output: Rc, ) { debug!( @@ -1383,8 +1325,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { polonius_output.subset_errors.len() ); - let mut outlives_suggestion = OutlivesSuggestionBuilder::new(mir_def_id, local_names); - // Similarly to `check_universal_regions`: a free region relation, which was not explicitly // declared ("known") was found by Polonius, so emit an error, or propagate the // requirements for our caller into the `propagated_outlives_requirements` vector. @@ -1425,27 +1365,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { body, &mut propagated_outlives_requirements, ); - if !propagated { - // If we are not in a context where we can't propagate errors, or we - // could not shrink `fr` to something smaller, then just report an - // error. - // - // Note: in this case, we use the unapproximated regions to report the - // error. This gives better error messages in some cases. - let db = self.report_error( - body, - local_names, - upvars, - infcx, - mir_def_id, - *longer_fr, - NLLRegionVariableOrigin::FreeRegion, - *shorter_fr, - &mut outlives_suggestion, - region_naming, - ); - - db.buffer(errors_buffer); + if propagated == RegionRelationCheckResult::Error { + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr: *longer_fr, + shorter_fr: *shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: true, + }); } } @@ -1458,7 +1384,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } NLLRegionVariableOrigin::Placeholder(placeholder) => { - self.check_bound_universal_region(infcx, body, mir_def_id, fr, placeholder); + self.check_bound_universal_region(fr, placeholder, errors_buffer); } NLLRegionVariableOrigin::Existential { .. } => { @@ -1466,9 +1392,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } } - - // Emit outlives suggestions - outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, region_naming); } /// Checks the final value for the free region `fr` to see if it @@ -1481,16 +1404,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// `outlives_requirements` vector. fn check_universal_region( &self, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], - mir_def_id: DefId, longer_fr: RegionVid, propagated_outlives_requirements: &mut Option<&mut Vec>>, - outlives_suggestion: &mut OutlivesSuggestionBuilder<'_>, - errors_buffer: &mut Vec, - region_naming: &mut RegionErrorNamingCtx, + errors_buffer: &mut RegionErrors<'tcx>, ) { debug!("check_universal_region(fr={:?})", longer_fr); @@ -1509,76 +1426,60 @@ impl<'tcx> RegionInferenceContext<'tcx> { // one in this SCC, so we will always check the representative here. let representative = self.scc_representatives[longer_fr_scc]; if representative != longer_fr { - self.check_universal_region_relation( + if let RegionRelationCheckResult::Error = self.check_universal_region_relation( longer_fr, representative, - infcx, body, - local_names, - upvars, - mir_def_id, propagated_outlives_requirements, - outlives_suggestion, - errors_buffer, - region_naming, - ); + ) { + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, + shorter_fr: representative, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: true, + }); + } return; } // Find every region `o` such that `fr: o` // (because `fr` includes `end(o)`). + let mut error_reported = false; for shorter_fr in self.scc_values.universal_regions_outlived_by(longer_fr_scc) { - if let Some(ErrorReported) = self.check_universal_region_relation( + if let RegionRelationCheckResult::Error = self.check_universal_region_relation( longer_fr, shorter_fr, - infcx, body, - local_names, - upvars, - mir_def_id, propagated_outlives_requirements, - outlives_suggestion, - errors_buffer, - region_naming, ) { - // continuing to iterate just reports more errors than necessary - // - // FIXME It would also allow us to report more Outlives Suggestions, though, so - // it's not clear that that's a bad thing. Somebody should try commenting out this - // line and see it is actually a regression. - return; + // We only report the first region error. Subsequent errors are hidden so as + // not to overwhelm the user, but we do record them so as to potentially print + // better diagnostics elsewhere... + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, + shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: !error_reported, + }); + + error_reported = true; } } } + /// Checks that we can prove that `longer_fr: shorter_fr`. If we can't we attempt to propagate + /// the constraint outward (e.g. to a closure environment), but if that fails, there is an + /// error. fn check_universal_region_relation( &self, longer_fr: RegionVid, shorter_fr: RegionVid, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], - mir_def_id: DefId, propagated_outlives_requirements: &mut Option<&mut Vec>>, - outlives_suggestion: &mut OutlivesSuggestionBuilder<'_>, - errors_buffer: &mut Vec, - region_naming: &mut RegionErrorNamingCtx, - ) -> Option { + ) -> RegionRelationCheckResult { // If it is known that `fr: o`, carry on. if self.universal_region_relations.outlives(longer_fr, shorter_fr) { - return None; - } - - let propagated = self.try_propagate_universal_region_error( - longer_fr, - shorter_fr, - body, - propagated_outlives_requirements, - ); - - if propagated { - None + RegionRelationCheckResult::Ok } else { // If we are not in a context where we can't propagate errors, or we // could not shrink `fr` to something smaller, then just report an @@ -1586,36 +1487,24 @@ impl<'tcx> RegionInferenceContext<'tcx> { // // Note: in this case, we use the unapproximated regions to report the // error. This gives better error messages in some cases. - let db = self.report_error( - body, - local_names, - upvars, - infcx, - mir_def_id, + self.try_propagate_universal_region_error( longer_fr, - NLLRegionVariableOrigin::FreeRegion, shorter_fr, - outlives_suggestion, - region_naming, - ); - - db.buffer(errors_buffer); - - Some(ErrorReported) + body, + propagated_outlives_requirements, + ) } } /// Attempt to propagate a region error (e.g. `'a: 'b`) that is not met to a closure's /// creator. If we cannot, then the caller should report an error to the user. - /// - /// Returns `true` if the error was propagated, and `false` otherwise. fn try_propagate_universal_region_error( &self, longer_fr: RegionVid, shorter_fr: RegionVid, body: &Body<'tcx>, propagated_outlives_requirements: &mut Option<&mut Vec>>, - ) -> bool { + ) -> RegionRelationCheckResult { if let Some(propagated_outlives_requirements) = propagated_outlives_requirements { // Shrink `longer_fr` until we find a non-local region (if we do). // We'll call it `fr-` -- it's ever so slightly smaller than @@ -1649,20 +1538,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { category: blame_span_category.0, }); } - return true; + return RegionRelationCheckResult::Propagated; } } - false + RegionRelationCheckResult::Error } fn check_bound_universal_region( &self, - infcx: &InferCtxt<'_, 'tcx>, - body: &Body<'tcx>, - _mir_def_id: DefId, longer_fr: RegionVid, placeholder: ty::PlaceholderRegion, + errors_buffer: &mut RegionErrors<'tcx>, ) { debug!("check_bound_universal_region(fr={:?}, placeholder={:?})", longer_fr, placeholder,); @@ -1699,28 +1586,17 @@ impl<'tcx> RegionInferenceContext<'tcx> { .unwrap(), }; - // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. - let (_, span) = self.find_outlives_blame_span( - body, + errors_buffer.push(RegionErrorKind::BoundUniversalRegionError { longer_fr, - NLLRegionVariableOrigin::Placeholder(placeholder), error_region, - ); - - // Obviously, this error message is far from satisfactory. - // At present, though, it only appears in unit tests -- - // the AST-based checker uses a more conservative check, - // so to even see this error, one must pass in a special - // flag. - let mut diag = infcx.tcx.sess.struct_span_err(span, "higher-ranked subtype error"); - diag.emit(); + fr_origin: NLLRegionVariableOrigin::Placeholder(placeholder), + }); } fn check_member_constraints( &self, infcx: &InferCtxt<'_, 'tcx>, - mir_def_id: DefId, - errors_buffer: &mut Vec, + errors_buffer: &mut RegionErrors<'tcx>, ) { let member_constraints = self.member_constraints.clone(); for m_c_i in member_constraints.all_indices() { @@ -1744,16 +1620,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { } // If not, report an error. - let region_scope_tree = &infcx.tcx.region_scope_tree(mir_def_id); let member_region = infcx.tcx.mk_region(ty::ReVar(member_region_vid)); - opaque_types::unexpected_hidden_region_diagnostic( - infcx.tcx, - Some(region_scope_tree), - m_c.opaque_type_def_id, - m_c.hidden_ty, + errors_buffer.push(RegionErrorKind::UnexpectedHiddenRegion { + opaque_type_def_id: m_c.opaque_type_def_id, + hidden_ty: m_c.hidden_ty, member_region, - ) - .buffer(errors_buffer); + }); } } } diff --git a/src/test/ui/hrtb/hrtb-perfect-forwarding.nll.stderr b/src/test/ui/hrtb/hrtb-perfect-forwarding.nll.stderr index f1647d3d2e43b..afa07cc60eb84 100644 --- a/src/test/ui/hrtb/hrtb-perfect-forwarding.nll.stderr +++ b/src/test/ui/hrtb/hrtb-perfect-forwarding.nll.stderr @@ -44,12 +44,6 @@ LL | | } | = help: a `loop` may express intention better if this is on purpose -error: higher-ranked subtype error - --> $DIR/hrtb-perfect-forwarding.rs:46:5 - | -LL | foo_hrtb_bar_not(&mut t); - | ^^^^^^^^^^^^^^^^^^^^^^^^ - error: lifetime may not live long enough --> $DIR/hrtb-perfect-forwarding.rs:46:5 | @@ -61,6 +55,12 @@ LL | foo_hrtb_bar_not(&mut t); | = help: consider replacing `'b` with `'static` +error: higher-ranked subtype error + --> $DIR/hrtb-perfect-forwarding.rs:46:5 + | +LL | foo_hrtb_bar_not(&mut t); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + warning: function cannot return without recursing --> $DIR/hrtb-perfect-forwarding.rs:49:1 |