From 43888e8c7c39588bfeb4ae1e0a45774932a9f934 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 11 Dec 2019 16:40:49 -0600 Subject: [PATCH 01/11] Refactor region error handling to be done by mirborrowckctx --- .../borrow_check/diagnostics/mod.rs | 2 +- .../borrow_check/diagnostics/region_errors.rs | 69 +++++- src/librustc_mir/borrow_check/mod.rs | 121 ++++++++++- src/librustc_mir/borrow_check/nll.rs | 65 +++--- .../borrow_check/region_infer/mod.rs | 197 ++++-------------- 5 files changed, 253 insertions(+), 201 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/mod.rs b/src/librustc_mir/borrow_check/diagnostics/mod.rs index 1a76265fbdcf0..ede0c2084097c 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mod.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mod.rs @@ -32,7 +32,7 @@ mod explain_borrow; crate use mutability_errors::AccessKind; crate use region_name::{RegionName, RegionNameSource, RegionErrorNamingCtx}; -crate use region_errors::{ErrorReportingCtx, ErrorConstraintInfo}; +crate use region_errors::{ErrorReportingCtx, ErrorConstraintInfo, RegionErrors, RegionErrorKind}; crate use outlives_suggestion::OutlivesSuggestionBuilder; pub(super) struct IncludingDowncast(pub(super) bool); diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index b78cd6bccf8ca..1cb6643e7d091 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -3,12 +3,13 @@ use rustc::hir::def_id::DefId; use rustc::infer::{ error_reporting::nice_region_error::NiceRegionError, + region_constraints::GenericKind, InferCtxt, NLLRegionVariableOrigin, }; use rustc::mir::{ ConstraintCategory, Local, Location, Body, }; -use rustc::ty::{self, RegionVid}; +use rustc::ty::{self, RegionVid, Ty}; use rustc_index::vec::IndexVec; use rustc_errors::DiagnosticBuilder; use std::collections::VecDeque; @@ -53,13 +54,61 @@ 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. +#[derive(Clone, Debug)] +crate struct RegionErrors<'tcx> { + errors: smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>, +} + +#[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>, + }, + + /// 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, + }, +} + /// 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. @@ -95,6 +144,22 @@ pub struct ErrorConstraintInfo { pub(super) span: Span, } +impl<'tcx> RegionErrors<'tcx> { + pub fn new() -> Self { + RegionErrors { + errors: smallvec::SmallVec::new(), + } + } + + pub fn push(&mut self, error: RegionErrorKind<'tcx>) { + self.errors.push(error) + } + + pub fn into_iter(self) -> impl Iterator> { + self.errors.into_iter() + } +} + impl<'tcx> RegionInferenceContext<'tcx> { /// Converts a region inference variable into a `ty::Region` that /// we can use for error reporting. If `r` is universally bound, diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 2554d5e729da9..1f45da083bce2 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -3,7 +3,7 @@ use rustc::hir::{self, HirId}; use rustc::hir::Node; use rustc::hir::def_id::DefId; -use rustc::infer::InferCtxt; +use rustc::infer::{opaque_types, InferCtxt}; use rustc::lint::builtin::UNUSED_MUT; use rustc::lint::builtin::{MUTABLE_BORROW_RESERVATION_CONFLICT}; use rustc::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind}; @@ -39,12 +39,15 @@ use crate::dataflow::MoveDataParamEnv; use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::EverInitializedPlaces; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; +use crate::transform::MirSource; use self::flows::Flows; use self::location::LocationTable; use self::prefixes::PrefixSet; use self::MutateMode::{JustWrite, WriteAndRead}; -use self::diagnostics::AccessKind; +use self::diagnostics::{ + AccessKind, RegionErrors, RegionErrorKind, OutlivesSuggestionBuilder, RegionErrorNamingCtx, +}; use self::path_utils::*; @@ -211,20 +214,40 @@ 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( + // 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, - &local_names, - &upvars, 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, ); @@ -297,6 +320,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, @@ -1560,6 +1586,89 @@ 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`. + + // TODO(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::RegionError { + fr_origin, longer_fr, shorter_fr, + } => { + 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); + + // Emit outlives suggestions + //outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, 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 6d28a8caa92bd..2bbf6c9c84799 100644 --- a/src/librustc_mir/borrow_check/nll.rs +++ b/src/librustc_mir/borrow_check/nll.rs @@ -3,12 +3,11 @@ use rustc::hir::def_id::DefId; use rustc::infer::InferCtxt; use rustc::mir::{ClosureOutlivesSubject, ClosureRegionRequirements, - Local, Location, Body, BodyAndCache, LocalKind, BasicBlock, + Location, Body, BodyAndCache, LocalKind, BasicBlock, Promoted, ReadOnlyBodyAndCache}; use rustc::ty::{self, RegionKind, RegionVid}; use rustc_index::vec::IndexVec; use rustc_errors::Diagnostic; -use syntax_pos::symbol::Symbol; use std::fmt::Debug; use std::env; use std::io; @@ -29,19 +28,28 @@ use crate::transform::MirSource; use crate::borrow_check::{ borrow_set::BorrowSet, + diagnostics::RegionErrors, location::LocationTable, facts::{AllFacts, AllFactsExt, RustcFacts}, region_infer::{RegionInferenceContext, values::RegionValueElements}, universal_regions::UniversalRegions, type_check::{self, MirTypeckResults, MirTypeckRegionConstraints}, - Upvar, renumber, constraint_generation, invalidation, + renumber, constraint_generation, invalidation, }; 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>, @@ -152,19 +160,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); @@ -306,40 +307,22 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( }); // Solve the region constraints. - let closure_region_requirements = regioncx.solve( + let (closure_region_requirements, nll_errors) = 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) + 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>, @@ -400,7 +383,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 dedc6b9b09af2..210106c8e6415 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::{ @@ -11,6 +10,7 @@ use rustc::mir::{ }; use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; use rustc::util::common::ErrorReported; +use rustc_errors::DiagnosticBuilder; use rustc_data_structures::binary_search_util; use rustc_index::bit_set::BitSet; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -18,9 +18,7 @@ use rustc_data_structures::graph::WithSuccessors; use rustc_data_structures::graph::scc::Sccs; use rustc_data_structures::graph::vec_graph::VecGraph; use rustc_index::vec::IndexVec; -use rustc_errors::{Diagnostic, DiagnosticBuilder}; use syntax_pos::Span; -use syntax_pos::symbol::Symbol; use crate::borrow_check::{ constraints::{ @@ -35,12 +33,9 @@ use crate::borrow_check::{ RegionValues, }, type_check::{free_region_relations::UniversalRegionRelations, Locations}, - diagnostics::{ - OutlivesSuggestionBuilder, RegionErrorNamingCtx, - }, + diagnostics::{RegionErrors, RegionErrorKind}, nll::{ToRegionVid, PoloniusOutput}, universal_regions::UniversalRegions, - Upvar, }; mod dump_mir; @@ -477,14 +472,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 @@ -496,17 +490,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.check_type_tests( infcx, body, - mir_def_id, outlives_requirements.as_mut(), - errors_buffer, + &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(); - // 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. @@ -514,36 +501,33 @@ impl<'tcx> RegionInferenceContext<'tcx> { 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, + &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, + ) } } @@ -838,9 +822,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; @@ -898,32 +881,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, + }); } } } @@ -1138,7 +1105,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// include the CFG anyhow. /// - For each `end('x)` element in `'r`, compute the mutual LUB, yielding /// a result `'y`. - pub (in crate::borrow_check) fn universal_upper_bound(&self, r: RegionVid) -> RegionVid { + pub(in crate::borrow_check) fn universal_upper_bound(&self, r: RegionVid) -> RegionVid { debug!("universal_upper_bound(r={:?}={})", r, self.region_value_str(r)); // Find the smallest universal region that contains all other @@ -1318,15 +1285,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { &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 => { @@ -1334,16 +1296,10 @@ 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, ); } @@ -1356,9 +1312,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. @@ -1386,12 +1339,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { &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!( @@ -1399,8 +1349,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. @@ -1439,26 +1387,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { &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); + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr: *longer_fr, + shorter_fr: *shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); } } @@ -1480,8 +1413,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 @@ -1494,16 +1425,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); @@ -1525,15 +1450,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { 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, ); return; } @@ -1544,21 +1463,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { if let Some(ErrorReported) = 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. + // + // TODO(mark-i-m): perhaps we could add them to the `RegionErrors` with a flag that + // says "don't report me"? return; } } @@ -1568,15 +1484,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { &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, + errors_buffer: &mut RegionErrors<'tcx>, ) -> Option { // If it is known that `fr: o`, carry on. if self.universal_region_relations.outlives(longer_fr, shorter_fr) { @@ -1599,20 +1509,10 @@ 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, - longer_fr, - NLLRegionVariableOrigin::FreeRegion, - shorter_fr, - outlives_suggestion, - region_naming, - ); - - db.buffer(errors_buffer); + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); Some(ErrorReported) } @@ -1727,8 +1627,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { 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() { @@ -1752,16 +1651,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); + }); } } } From 94a9c6262bba181ca97e5df75ee7a305348cf036 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Tue, 17 Dec 2019 10:43:00 -0600 Subject: [PATCH 02/11] add unreported error variant --- .../borrow_check/diagnostics/region_errors.rs | 11 +++++++++++ src/librustc_mir/borrow_check/mod.rs | 19 ++++++++++++++++++- .../borrow_check/region_infer/mod.rs | 16 +++++++--------- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 1cb6643e7d091..58f2991088355 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -107,6 +107,17 @@ crate enum RegionErrorKind<'tcx> { /// The region that should be shorter, but we can't prove it shorter_fr: RegionVid, }, + + /// The same as `RegionError`, but this is an unreported error. We currently only report the + /// first error encountered and leave the rest unreported so as not to overwhelm the user. + UnreportedError { + /// 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, + } } /// Various pieces of state used when reporting borrow checker errors. diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 1f45da083bce2..0d40b571cf3fa 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1664,7 +1664,24 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { db.buffer(&mut self.errors_buffer); // Emit outlives suggestions - //outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, region_naming); + outlives_suggestion.add_suggestion( + &self.body, + &self.nonlexical_regioncx, + self.infcx, + &mut self.errors_buffer, + &mut region_naming + ); + } + + RegionErrorKind::UnreportedError { + longer_fr, shorter_fr, + fr_origin: _, + } => { + // FIXME: currently we do nothing with these, but perhaps we can do better? + debug!( + "Unreported region error: can't prove that {:?}: {:?}", + longer_fr, shorter_fr + ); } } } diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 210106c8e6415..c3e469e3b9f7b 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -1467,15 +1467,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { propagated_outlives_requirements, errors_buffer, ) { - // 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. - // - // TODO(mark-i-m): perhaps we could add them to the `RegionErrors` with a flag that - // says "don't report me"? - 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::UnreportedError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); } } } From 3902e563fb1054aa0fb4e142fdef8dc59d5845d5 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 18 Dec 2019 13:27:01 -0600 Subject: [PATCH 03/11] tidy --- src/librustc_mir/borrow_check/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 0d40b571cf3fa..d669a75071c59 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1592,9 +1592,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are // buffered in the `MirBorrowckCtxt`. - // TODO(mark-i-m): Would be great to get rid of the naming context. + // 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); + let mut outlives_suggestion = OutlivesSuggestionBuilder::new( + self.mir_def_id, &self.local_names + ); for nll_error in nll_errors.into_iter() { match nll_error { From e5a1f48fa7e6ada9c1bd6ab5a00afd07eac94fac Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 18 Dec 2019 17:00:00 -0600 Subject: [PATCH 04/11] fix outlives suggestions --- .../diagnostics/outlives_suggestion.rs | 4 +++- src/librustc_mir/borrow_check/mod.rs | 20 ++++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs index b61c37b061396..dc5fed4822984 100644 --- a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs +++ b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs @@ -250,7 +250,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/mod.rs b/src/librustc_mir/borrow_check/mod.rs index d669a75071c59..4171190805665 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1664,15 +1664,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); db.buffer(&mut self.errors_buffer); - - // Emit outlives suggestions - outlives_suggestion.add_suggestion( - &self.body, - &self.nonlexical_regioncx, - self.infcx, - &mut self.errors_buffer, - &mut region_naming - ); } RegionErrorKind::UnreportedError { @@ -1680,6 +1671,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { fr_origin: _, } => { // 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 @@ -1687,6 +1680,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } } + + // 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 + ); } } From 5d9e74ebd1d5cc02be354a0791df1f6ed295f09f Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 18 Dec 2019 17:02:49 -0600 Subject: [PATCH 05/11] some more refactoring + maintain diagnostic status quo --- .../borrow_check/region_infer/mod.rs | 90 ++++++++++--------- 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index c3e469e3b9f7b..05a21e6958e3e 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -9,7 +9,6 @@ use rustc::mir::{ ConstraintCategory, Local, Location, }; use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; -use rustc::util::common::ErrorReported; use rustc_errors::DiagnosticBuilder; use rustc_data_structures::binary_search_util; use rustc_index::bit_set::BitSet; @@ -225,6 +224,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 @@ -1386,7 +1394,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { body, &mut propagated_outlives_requirements, ); - if !propagated { + if propagated == RegionRelationCheckResult::Error { errors_buffer.push(RegionErrorKind::RegionError { longer_fr: *longer_fr, shorter_fr: *shorter_fr, @@ -1412,7 +1420,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } } - } /// Checks the final value for the free region `fr` to see if it @@ -1447,59 +1454,64 @@ 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, body, propagated_outlives_requirements, - errors_buffer, - ); + ) { + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, + shorter_fr: representative, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); + } 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, body, propagated_outlives_requirements, - errors_buffer, ) { - // 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::UnreportedError { - longer_fr, shorter_fr, - fr_origin: NLLRegionVariableOrigin::FreeRegion, - }); + // 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... + if error_reported { + errors_buffer.push(RegionErrorKind::UnreportedError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); + } else { + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); + } + + 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, body: &Body<'tcx>, propagated_outlives_requirements: &mut Option<&mut Vec>>, - errors_buffer: &mut RegionErrors<'tcx>, - ) -> 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 @@ -1507,26 +1519,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. - errors_buffer.push(RegionErrorKind::RegionError { - longer_fr, shorter_fr, - fr_origin: NLLRegionVariableOrigin::FreeRegion, - }); - - Some(ErrorReported) + self.try_propagate_universal_region_error( + longer_fr, + shorter_fr, + 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 @@ -1558,11 +1568,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { category: blame_span_category.0, }); } - return true; + return RegionRelationCheckResult::Propagated; } } - false + RegionRelationCheckResult::Error } fn check_bound_universal_region( From e4379e66ff2c1af052b7714f54433ef2e2afe0ff Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 18 Dec 2019 17:45:05 -0600 Subject: [PATCH 06/11] make regionerrors a typedef --- .../borrow_check/diagnostics/region_errors.rs | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 58f2991088355..59bd81c52ddc8 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -63,10 +63,7 @@ enum Trace { /// A collection of errors encountered during region inference. This is needed to efficiently /// report errors after borrow checking. -#[derive(Clone, Debug)] -crate struct RegionErrors<'tcx> { - errors: smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>, -} +crate type RegionErrors<'tcx> = smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>; #[derive(Clone, Debug)] crate enum RegionErrorKind<'tcx> { @@ -155,22 +152,6 @@ pub struct ErrorConstraintInfo { pub(super) span: Span, } -impl<'tcx> RegionErrors<'tcx> { - pub fn new() -> Self { - RegionErrors { - errors: smallvec::SmallVec::new(), - } - } - - pub fn push(&mut self, error: RegionErrorKind<'tcx>) { - self.errors.push(error) - } - - pub fn into_iter(self) -> impl Iterator> { - self.errors.into_iter() - } -} - impl<'tcx> RegionInferenceContext<'tcx> { /// Converts a region inference variable into a `ty::Region` that /// we can use for error reporting. If `r` is universally bound, From 5b06025f99a68b39a49b7de5932a5b652edf4ac7 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 12:54:43 -0600 Subject: [PATCH 07/11] is_reported flag --- .../borrow_check/diagnostics/region_errors.rs | 14 ++--- src/librustc_mir/borrow_check/mod.rs | 51 +++++++++---------- .../borrow_check/region_infer/mod.rs | 18 +++---- 3 files changed, 34 insertions(+), 49 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 59bd81c52ddc8..4e85b110ddaf4 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -103,18 +103,10 @@ crate enum RegionErrorKind<'tcx> { 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, }, - - /// The same as `RegionError`, but this is an unreported error. We currently only report the - /// first error encountered and leave the rest unreported so as not to overwhelm the user. - UnreportedError { - /// 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, - } } /// Various pieces of state used when reporting borrow checker errors. diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 4171190805665..751ebc155f5ce 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1648,35 +1648,32 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } RegionErrorKind::RegionError { - fr_origin, longer_fr, shorter_fr, + fr_origin, longer_fr, shorter_fr, 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); - } + 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, + ); - RegionErrorKind::UnreportedError { - longer_fr, shorter_fr, - fr_origin: _, - } => { - // 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 - ); + db.buffer(&mut self.errors_buffer); + } else { + // 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 + ); + } } } } diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 05a21e6958e3e..3afe2056a926f 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -1399,6 +1399,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { longer_fr: *longer_fr, shorter_fr: *shorter_fr, fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: true, }); } } @@ -1464,6 +1465,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { longer_fr, shorter_fr: representative, fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: true, }); } return; @@ -1482,17 +1484,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { // 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... - if error_reported { - errors_buffer.push(RegionErrorKind::UnreportedError { - longer_fr, shorter_fr, - fr_origin: NLLRegionVariableOrigin::FreeRegion, - }); - } else { - errors_buffer.push(RegionErrorKind::RegionError { - longer_fr, shorter_fr, - fr_origin: NLLRegionVariableOrigin::FreeRegion, - }); - } + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: !error_reported, + }); error_reported = true; } From bd0ea19eaa96dc607c285d0aeba2fffa44fc1019 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 13:27:00 -0600 Subject: [PATCH 08/11] add comments --- src/librustc_mir/borrow_check/diagnostics/region_errors.rs | 3 +++ src/librustc_mir/borrow_check/mod.rs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 4e85b110ddaf4..4ce0311f3bb4c 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -63,6 +63,9 @@ enum Trace { /// 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> = smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>; #[derive(Clone, Debug)] diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 751ebc155f5ce..153ee23811e92 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1666,6 +1666,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { 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? From a155a9b67d9688a3d5b90361e5ea8a4a9bf3b0d6 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 13:35:21 -0600 Subject: [PATCH 09/11] use vec instead of smallvec --- src/librustc_mir/borrow_check/diagnostics/region_errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 4ce0311f3bb4c..481494f6a3abe 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -66,7 +66,7 @@ enum Trace { /// /// 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> = smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>; +crate type RegionErrors<'tcx> = Vec>; #[derive(Clone, Debug)] crate enum RegionErrorKind<'tcx> { From d85578287c40031b61c252daba0572181b6117f4 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 15:31:52 -0600 Subject: [PATCH 10/11] convert hrtb error --- .../borrow_check/diagnostics/region_errors.rs | 10 ++++++ src/librustc_mir/borrow_check/mod.rs | 19 ++++++++++ .../borrow_check/region_infer/mod.rs | 36 +++++-------------- 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 481494f6a3abe..98a1d7dad94bd 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -98,6 +98,16 @@ crate enum RegionErrorKind<'tcx> { 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 diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 153ee23811e92..9fc6b0ffb2d59 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1647,6 +1647,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { .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, } => { diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 3afe2056a926f..5310ac6c48faa 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -9,7 +9,6 @@ use rustc::mir::{ ConstraintCategory, Local, Location, }; use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; -use rustc_errors::DiagnosticBuilder; use rustc_data_structures::binary_search_util; use rustc_index::bit_set::BitSet; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -435,7 +434,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) } @@ -507,18 +506,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { // 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, - mir_def_id, outlives_requirements.as_mut(), &mut errors_buffer, polonius_output.expect("Polonius output is unavailable despite `-Z polonius`"), ); } else { self.check_universal_regions( - infcx, body, - mir_def_id, outlives_requirements.as_mut(), &mut errors_buffer, ); @@ -1291,9 +1286,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// report them as errors. fn check_universal_regions( &self, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, errors_buffer: &mut RegionErrors<'tcx>, ) { @@ -1312,7 +1305,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 { .. } => { @@ -1345,9 +1338,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// report them as errors. fn check_polonius_subset_errors( &self, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, errors_buffer: &mut RegionErrors<'tcx>, polonius_output: Rc, @@ -1413,7 +1404,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 { .. } => { @@ -1573,11 +1564,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { 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,); @@ -1614,18 +1603,11 @@ 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, 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(); + errors_buffer.push(RegionErrorKind::BoundUniversalRegionError { + longer_fr, + error_region, + fr_origin: NLLRegionVariableOrigin::Placeholder(placeholder), + }); } fn check_member_constraints( From 1d9c56189f4b6f09ffd269e63bff98ede0b93b11 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 15:33:34 -0600 Subject: [PATCH 11/11] minor updates to comments --- .../borrow_check/diagnostics/region_errors.rs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 98a1d7dad94bd..801f801fc9097 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -70,51 +70,51 @@ 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 + /// An error for a type test: `T: 'a` does not live long enough. TypeTestDoesNotLiveLongEnough { - /// The span of the type test + /// The span of the type test. span: Span, - /// The generic type of the type test + /// The generic type of the type test. generic: GenericKind<'tcx>, }, - /// A generic bound failure for a type test + /// A generic bound failure for a type test. TypeTestGenericBoundError { - /// The span of the type test + /// The span of the type test. span: Span, - /// The generic type of the type test + /// The generic type of the type test. generic: GenericKind<'tcx>, - /// The lower bound region + /// The lower bound region. lower_bound_region: ty::Region<'tcx>, }, - /// An unexpected hidden region for an opaque type + /// An unexpected hidden region for an opaque type. UnexpectedHiddenRegion { - /// The def id of the opaque type + /// The def id of the opaque type. opaque_type_def_id: DefId, - /// The hidden type + /// The hidden type. hidden_ty: Ty<'tcx>, - /// The unexpected region + /// The unexpected region. member_region: ty::Region<'tcx>, }, - /// Higher-ranked subtyping error + /// 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 + /// The origin of the placeholder region. fr_origin: NLLRegionVariableOrigin, }, - /// Any other lifetime error + /// Any other lifetime error. RegionError { - /// The origin of the region + /// The origin of the region. fr_origin: NLLRegionVariableOrigin, - /// The region that should outlive `shorter_fr` + /// The region that should outlive `shorter_fr`. longer_fr: RegionVid, - /// The region that should be shorter, but we can't prove it + /// 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.