From 479cc7ae9ab02700516829221291865944e2c6cb Mon Sep 17 00:00:00 2001 From: lqd Date: Mon, 18 Nov 2019 13:45:41 +0100 Subject: [PATCH 01/10] update to polonius 0.11 to compute subset errors - adapt to the new polonius `FactTypes` API - reorganize the type aliases referring to polonius to avoid referencing the inner atom or fact types multiple times: only one input and output types should be enough for everyone. They could equally be in `borrow_check` as `nll` though. --- Cargo.lock | 4 ++-- src/librustc/Cargo.toml | 2 +- src/librustc_mir/Cargo.toml | 2 +- src/librustc_mir/borrow_check/flows.rs | 9 +++------ src/librustc_mir/borrow_check/nll/facts.rs | 15 +++++++++++++-- src/librustc_mir/borrow_check/nll/mod.rs | 11 ++++++----- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 26727c5c1db60..65e58b798df67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2575,9 +2575,9 @@ checksum = "676e8eb2b1b4c9043511a9b7bea0915320d7e502b0a079fb03f9635a5252b18c" [[package]] name = "polonius-engine" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50fa9dbfd0d3d60594da338cfe6f94028433eecae4b11b7e83fd99759227bbfe" +checksum = "1e478d7c38eb785c6416cbe58df12aa55d7aefa3759b6d3e044b2ed03f423cec" dependencies = [ "datafrog", "log", diff --git a/src/librustc/Cargo.toml b/src/librustc/Cargo.toml index f8ad6f8f30edb..fb5387ffd018e 100644 --- a/src/librustc/Cargo.toml +++ b/src/librustc/Cargo.toml @@ -20,7 +20,7 @@ scoped-tls = "1.0" log = { version = "0.4", features = ["release_max_level_info", "std"] } rustc-rayon = "0.3.0" rustc-rayon-core = "0.3.0" -polonius-engine = "0.10.0" +polonius-engine = "0.11.0" rustc_apfloat = { path = "../librustc_apfloat" } rustc_feature = { path = "../librustc_feature" } rustc_target = { path = "../librustc_target" } diff --git a/src/librustc_mir/Cargo.toml b/src/librustc_mir/Cargo.toml index 4afbb4d85d025..7e3bd98176e6a 100644 --- a/src/librustc_mir/Cargo.toml +++ b/src/librustc_mir/Cargo.toml @@ -16,7 +16,7 @@ dot = { path = "../libgraphviz", package = "graphviz" } itertools = "0.8" log = "0.4" log_settings = "0.1.1" -polonius-engine = "0.10.0" +polonius-engine = "0.11.0" rustc = { path = "../librustc" } rustc_target = { path = "../librustc_target" } rustc_data_structures = { path = "../librustc_data_structures" } diff --git a/src/librustc_mir/borrow_check/flows.rs b/src/librustc_mir/borrow_check/flows.rs index ce5d2a14bd122..57c544fda0c54 100644 --- a/src/librustc_mir/borrow_check/flows.rs +++ b/src/librustc_mir/borrow_check/flows.rs @@ -3,16 +3,15 @@ //! FIXME: this might be better as a "generic" fixed-point combinator, //! but is not as ugly as it is right now. -use rustc::mir::{BasicBlock, Local, Location}; -use rustc::ty::RegionVid; +use rustc::mir::{BasicBlock, Location}; use rustc_index::bit_set::BitIter; use crate::borrow_check::location::LocationIndex; -use polonius_engine::Output; +use crate::borrow_check::nll::PoloniusOutput; use crate::dataflow::indexes::BorrowIndex; -use crate::dataflow::move_paths::{HasMoveData, MovePathIndex}; +use crate::dataflow::move_paths::HasMoveData; use crate::dataflow::Borrows; use crate::dataflow::EverInitializedPlaces; use crate::dataflow::MaybeUninitializedPlaces; @@ -21,8 +20,6 @@ use either::Either; use std::fmt; use std::rc::Rc; -crate type PoloniusOutput = Output; - crate struct Flows<'b, 'tcx> { borrows: FlowAtLocation<'tcx, Borrows<'b, 'tcx>>, pub uninits: FlowAtLocation<'tcx, MaybeUninitializedPlaces<'b, 'tcx>>, diff --git a/src/librustc_mir/borrow_check/nll/facts.rs b/src/librustc_mir/borrow_check/nll/facts.rs index 13e5769c5bea8..eabb11acab91e 100644 --- a/src/librustc_mir/borrow_check/nll/facts.rs +++ b/src/librustc_mir/borrow_check/nll/facts.rs @@ -1,6 +1,6 @@ use crate::borrow_check::location::{LocationIndex, LocationTable}; use crate::dataflow::indexes::{BorrowIndex, MovePathIndex}; -use polonius_engine::AllFacts as PoloniusAllFacts; +use polonius_engine::AllFacts as PoloniusFacts; use polonius_engine::Atom; use rustc::mir::Local; use rustc::ty::{RegionVid, TyCtxt}; @@ -11,7 +11,18 @@ use std::fs::{self, File}; use std::io::Write; use std::path::Path; -crate type AllFacts = PoloniusAllFacts; +#[derive(Copy, Clone, Debug)] +crate struct RustcFacts; + +impl polonius_engine::FactTypes for RustcFacts { + type Origin = RegionVid; + type Loan = BorrowIndex; + type Point = LocationIndex; + type Variable = Local; + type Path = MovePathIndex; +} + +crate type AllFacts = PoloniusFacts; crate trait AllFactsExt { /// Returns `true` if there is a need to gather `AllFacts` given the diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index b9363200cdf28..337f612f7cea9 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -1,10 +1,9 @@ use crate::borrow_check::borrow_set::BorrowSet; -use crate::borrow_check::location::{LocationIndex, LocationTable}; +use crate::borrow_check::location::LocationTable; use crate::borrow_check::nll::facts::AllFactsExt; use crate::borrow_check::nll::type_check::{MirTypeckResults, MirTypeckRegionConstraints}; use crate::borrow_check::nll::region_infer::values::RegionValueElements; -use crate::dataflow::indexes::BorrowIndex; -use crate::dataflow::move_paths::{InitLocation, MoveData, MovePathIndex, InitKind}; +use crate::dataflow::move_paths::{InitLocation, MoveData, InitKind}; use crate::dataflow::FlowAtLocation; use crate::dataflow::MaybeInitializedPlaces; use crate::transform::MirSource; @@ -43,10 +42,12 @@ crate mod universal_regions; crate mod type_check; crate mod region_infer; -use self::facts::AllFacts; +use self::facts::{AllFacts, RustcFacts}; use self::region_infer::RegionInferenceContext; use self::universal_regions::UniversalRegions; +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 @@ -170,7 +171,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( errors_buffer: &mut Vec, ) -> ( RegionInferenceContext<'tcx>, - Option>>, + Option>, Option>, ) { let mut all_facts = if AllFacts::enabled(infcx.tcx) { From 4b16ae1609c65d337d4f22e2f4ddea0c4c467b3b Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Tue, 19 Nov 2019 16:53:52 +0100 Subject: [PATCH 02/10] Add a way to list the base non-transitive edges in `TransitiveRelation` --- src/librustc_data_structures/transitive_relation.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/librustc_data_structures/transitive_relation.rs b/src/librustc_data_structures/transitive_relation.rs index a3926c1555172..bbf6999b98375 100644 --- a/src/librustc_data_structures/transitive_relation.rs +++ b/src/librustc_data_structures/transitive_relation.rs @@ -373,6 +373,14 @@ impl TransitiveRelation { } matrix } + + /// Lists all the base edges in the graph: the initial _non-transitive_ set of element + /// relations, which will be later used as the basis for the transitive closure computation. + pub fn base_edges(&self) -> impl Iterator { + self.edges + .iter() + .map(move |edge| (&self.elements[edge.source.0], &self.elements[edge.target.0])) + } } /// Pare down is used as a step in the LUB computation. It edits the From 4dd6292c3c563e53732672d878930a9c26d512fc Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Tue, 19 Nov 2019 16:55:58 +0100 Subject: [PATCH 03/10] UniversalRegionRelations: add a way to list the base non-transitive `outlives` constraints --- .../borrow_check/nll/type_check/free_region_relations.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs b/src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs index d18a8e87453a5..8bb68383a49ba 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs @@ -217,6 +217,11 @@ impl UniversalRegionRelations<'tcx> { crate fn regions_outlived_by(&self, fr1: RegionVid) -> Vec<&RegionVid> { self.outlives.reachable_from(&fr1) } + + /// Returns the _non-transitive_ set of known `outlives` constraints between free regions. + crate fn known_outlives(&self) -> impl Iterator { + self.outlives.base_edges() + } } struct UniversalRegionRelationsBuilder<'this, 'tcx> { From 7a3dca69bb706a015cf220a80ca35234dd2aeee6 Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Tue, 19 Nov 2019 16:58:09 +0100 Subject: [PATCH 04/10] Polonius: emit `placeholder` and `known_subset` facts, as inputs to the subset error computation --- src/librustc_mir/borrow_check/nll/facts.rs | 2 ++ src/librustc_mir/borrow_check/nll/mod.rs | 30 ++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/librustc_mir/borrow_check/nll/facts.rs b/src/librustc_mir/borrow_check/nll/facts.rs index eabb11acab91e..a16c36d749f0d 100644 --- a/src/librustc_mir/borrow_check/nll/facts.rs +++ b/src/librustc_mir/borrow_check/nll/facts.rs @@ -66,6 +66,7 @@ impl AllFactsExt for AllFacts { wr.write_facts_to_path(self.[ borrow_region, universal_region, + placeholder, cfg_edge, killed, outlives, @@ -80,6 +81,7 @@ impl AllFactsExt for AllFacts { initialized_at, moved_out_at, path_accessed_at, + known_subset, ]) } Ok(()) diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index 337f612f7cea9..ffd9c011717df 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -209,6 +209,36 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( .universal_region .extend(universal_regions.universal_regions()); populate_polonius_move_facts(all_facts, move_data, location_table, &body); + + // Emit universal regions facts, and their relations, for Polonius. + // + // 1: universal regions are modeled in Polonius as a pair: + // - the universal region vid itself. + // - a "placeholder loan" associated to this universal region. Since they don't exist in + // the `borrow_set`, their `BorrowIndex` are synthesized as the universal region index + // added to the existing number of loans, as if they succeeded them in the set. + // + let borrow_count = borrow_set.borrows.len(); + debug!( + "compute_regions: polonius placeholders, num_universals={}, borrow_count={}", + universal_regions.len(), + borrow_count + ); + + for universal_region in universal_regions.universal_regions() { + let universal_region_idx = universal_region.index(); + let placeholder_loan_idx = borrow_count + universal_region_idx; + all_facts.placeholder.push((universal_region, placeholder_loan_idx.into())); + } + + // 2: the universal region relations `outlives` constraints are emitted as + // `known_subset` facts. + for (fr1, fr2) in universal_region_relations.known_outlives() { + if fr1 != fr2 { + debug!("compute_regions: emitting polonius `known_subset` fr1={:?}, fr2={:?}", fr1, fr2); + all_facts.known_subset.push((*fr1, *fr2)); + } + } } // Create the region inference context, taking ownership of the From 02a6662e2feec6ecf49d497c78ecf17599223e03 Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Wed, 20 Nov 2019 11:13:03 +0100 Subject: [PATCH 05/10] Implement subset errors using Polonius - switches to using the Naive variant by default - emits subset errors or propagates unsatisfied obligations to the caller --- src/librustc_mir/borrow_check/nll/mod.rs | 4 +- .../borrow_check/nll/region_infer/mod.rs | 186 ++++++++++++++++-- 2 files changed, 172 insertions(+), 18 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index ffd9c011717df..5be299f2a9440 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -300,7 +300,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( if infcx.tcx.sess.opts.debugging_opts.polonius { let algorithm = env::var("POLONIUS_ALGORITHM") - .unwrap_or_else(|_| String::from("Hybrid")); + .unwrap_or_else(|_| String::from("Naive")); let algorithm = Algorithm::from_str(&algorithm).unwrap(); debug!("compute_regions: using polonius algorithm {:?}", algorithm); Some(Rc::new(Output::compute( @@ -315,7 +315,7 @@ 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); + 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. diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 85031d6210a4d..574c64aa571cb 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -44,7 +44,7 @@ use crate::borrow_check::{ use self::values::{LivenessValues, RegionValueElements, RegionValues}; use super::universal_regions::UniversalRegions; -use super::ToRegionVid; +use super::{PoloniusOutput, ToRegionVid}; mod dump_mir; mod graphviz; @@ -484,6 +484,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { upvars: &[Upvar], mir_def_id: DefId, errors_buffer: &mut Vec, + polonius_output: Option>, ) -> Option> { self.propagate_constraints(body); @@ -509,16 +510,33 @@ impl<'tcx> RegionInferenceContext<'tcx> { // multiple problems. let mut region_naming = RegionErrorNamingCtx::new(); - self.check_universal_regions( - infcx, - body, - local_names, - upvars, - mir_def_id, - outlives_requirements.as_mut(), - errors_buffer, - &mut region_naming, - ); + // 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, + 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_member_constraints(infcx, mir_def_id, errors_buffer); @@ -1375,6 +1393,111 @@ impl<'tcx> RegionInferenceContext<'tcx> { outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, region_naming); } + /// Checks if Polonius has found any unexpected free region relations. + /// + /// In Polonius terms, a "subset error" (or "illegal subset relation error") is the equivalent + /// of NLL's "checking if any region constraints were too strong": a placeholder origin `'a` + /// was unexpectedly found to be a subset of another placeholder origin `'b`, and means in NLL + /// terms that the "longer free region" `'a` outlived the "shorter free region" `'b`. + /// + /// More details can be found in this blog post by Niko: + /// http://smallcultfollowing.com/babysteps/blog/2019/01/17/polonius-and-region-errors/ + /// + /// In the canonical example + /// + /// fn foo<'a, 'b>(x: &'a u32) -> &'b u32 { x } + /// + /// returning `x` requires `&'a u32 <: &'b u32` and hence we establish (transitively) a + /// constraint that `'a: 'b`. It is an error that we have no evidence that this + /// constraint holds. + /// + /// If `propagated_outlives_requirements` is `Some`, then we will + /// push unsatisfied obligations into there. Otherwise, we'll + /// 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, + polonius_output: Rc, + ) { + debug!("check_polonius_subset_errors: {} subset_errors", 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. + // + // Polonius doesn't model regions ("origins") as CFG-subsets or durations, but the + // `longer_fr` and `shorter_fr` terminology will still be used here, for consistency with + // the rest of the NLL infrastructure. The "subset origin" is the "longer free region", + // and the "superset origin" is the outlived "shorter free region". + // + // Note: Polonius will produce a subset error at every point where the unexpected + // `longer_fr`'s "placeholder loan" is contained in the `shorter_fr`. This can be helpful + // for diagnostics in the future, e.g. to point more precisely at the key locations + // requiring this constraint to hold. However, the error and diagnostics code downstream + // expects that these errors are not duplicated (and that they are in a certain order). + // Otherwise, diagnostics messages such as the ones giving names like `'1` to elided or + // anonymous lifetimes for example, could give these names differently, while others like + // the outlives suggestions or the debug output from `#[rustc_regions]` would be + // duplicated. The polonius subset errors are deduplicated here, while keeping the + // CFG-location ordering. + let mut subset_errors: Vec<_> = polonius_output + .subset_errors + .iter() + .flat_map(|(_location, subset_errors)| subset_errors.iter()) + .collect(); + subset_errors.sort(); + subset_errors.dedup(); + + for (longer_fr, shorter_fr) in subset_errors.into_iter() { + debug!("check_polonius_subset_errors: subset_error longer_fr={:?},\ + shorter_fr={:?}", longer_fr, shorter_fr); + + self.report_or_propagate_universal_region_error( + *longer_fr, + *shorter_fr, + infcx, + body, + local_names, + upvars, + mir_def_id, + &mut propagated_outlives_requirements, + &mut outlives_suggestion, + errors_buffer, + region_naming, + ); + } + + // Handle the placeholder errors as usual, until the chalk-rustc-polonius triumvirate has + // a more complete picture on how to separate this responsibility. + for (fr, fr_definition) in self.definitions.iter_enumerated() { + match fr_definition.origin { + NLLRegionVariableOrigin::FreeRegion => { + // handled by polonius above + } + + NLLRegionVariableOrigin::Placeholder(placeholder) => { + self.check_bound_universal_region(infcx, body, mir_def_id, fr, placeholder); + } + + NLLRegionVariableOrigin::Existential { .. } => { + // nothing to check here + } + } + } + + // 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 /// grew too large. In particular, examine what `end(X)` points /// wound up in `fr`'s final value; for each `end(X)` where `X != @@ -1474,8 +1597,37 @@ impl<'tcx> RegionInferenceContext<'tcx> { return None; } + self.report_or_propagate_universal_region_error( + longer_fr, + shorter_fr, + infcx, + body, + local_names, + upvars, + mir_def_id, + propagated_outlives_requirements, + outlives_suggestion, + errors_buffer, + region_naming, + ) + } + + fn report_or_propagate_universal_region_error( + &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 { debug!( - "check_universal_region_relation: fr={:?} does not outlive shorter_fr={:?}", + "report_or_propagate_universal_region_error: fr={:?} does not outlive shorter_fr={:?}", longer_fr, shorter_fr, ); @@ -1484,9 +1636,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { // We'll call it `fr-` -- it's ever so slightly smaller than // `longer_fr`. - if let Some(fr_minus) = self.universal_region_relations.non_local_lower_bound(longer_fr) - { - debug!("check_universal_region: fr_minus={:?}", fr_minus); + if let Some(fr_minus) = + self.universal_region_relations.non_local_lower_bound(longer_fr) { + debug!("report_or_propagate_universal_region_error: fr_minus={:?}", fr_minus); let blame_span_category = self.find_outlives_blame_span(body, longer_fr, @@ -1497,7 +1649,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { // so slightly larger than `shorter_fr`. let shorter_fr_plus = self.universal_region_relations.non_local_upper_bounds(&shorter_fr); - debug!("check_universal_region: shorter_fr_plus={:?}", shorter_fr_plus); + debug!( + "report_or_propagate_universal_region_error: shorter_fr_plus={:?}", shorter_fr_plus + ); for &&fr in &shorter_fr_plus { // Push the constraint `fr-: shorter_fr+` propagated_outlives_requirements.push(ClosureOutlivesRequirement { From 67b04d5f641efe6bc0dd989d0a2747c210c134eb Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Mon, 2 Dec 2019 18:38:05 +0100 Subject: [PATCH 06/10] bless polonius output of test hrtb-perfect-forwarding.rs The plan is to use chalk and not have polonius deal with this. --- .../hrtb-perfect-forwarding.polonius.stderr | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 src/test/ui/hrtb/hrtb-perfect-forwarding.polonius.stderr diff --git a/src/test/ui/hrtb/hrtb-perfect-forwarding.polonius.stderr b/src/test/ui/hrtb/hrtb-perfect-forwarding.polonius.stderr new file mode 100644 index 0000000000000..558d643cde895 --- /dev/null +++ b/src/test/ui/hrtb/hrtb-perfect-forwarding.polonius.stderr @@ -0,0 +1,68 @@ +warning: function cannot return without recursing + --> $DIR/hrtb-perfect-forwarding.rs:22:1 + | +LL | / fn no_hrtb<'b,T>(mut t: T) +LL | | where T : Bar<&'b isize> +LL | | { +LL | | // OK -- `T : Bar<&'b isize>`, and thus the impl above ensures that +LL | | // `&mut T : Bar<&'b isize>`. +LL | | no_hrtb(&mut t); + | | --------------- recursive call site +LL | | } + | |_^ cannot return without recursing + | + = note: `#[warn(unconditional_recursion)]` on by default + = help: a `loop` may express intention better if this is on purpose + +warning: function cannot return without recursing + --> $DIR/hrtb-perfect-forwarding.rs:30:1 + | +LL | / fn bar_hrtb(mut t: T) +LL | | where T : for<'b> Bar<&'b isize> +LL | | { +LL | | // OK -- `T : for<'b> Bar<&'b isize>`, and thus the impl above +... | +LL | | bar_hrtb(&mut t); + | | ---------------- recursive call site +LL | | } + | |_^ cannot return without recursing + | + = help: a `loop` may express intention better if this is on purpose + +warning: function cannot return without recursing + --> $DIR/hrtb-perfect-forwarding.rs:39:1 + | +LL | / fn foo_hrtb_bar_not<'b,T>(mut t: T) +LL | | where T : for<'a> Foo<&'a isize> + Bar<&'b isize> +LL | | { +LL | | // Not OK -- The forwarding impl for `Foo` requires that `Bar` also +... | +LL | | foo_hrtb_bar_not(&mut t); + | | ------------------------ recursive call site +LL | | } + | |_^ cannot return without recursing + | + = 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); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: function cannot return without recursing + --> $DIR/hrtb-perfect-forwarding.rs:49:1 + | +LL | / fn foo_hrtb_bar_hrtb(mut t: T) +LL | | where T : for<'a> Foo<&'a isize> + for<'b> Bar<&'b isize> +LL | | { +LL | | // OK -- now we have `T : for<'b> Bar&'b isize>`. +LL | | foo_hrtb_bar_hrtb(&mut t); + | | ------------------------- recursive call site +LL | | } + | |_^ cannot return without recursing + | + = help: a `loop` may express intention better if this is on purpose + +error: aborting due to previous error + From 695640816a48098efaddc83d1abaa9edcc4fd109 Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Mon, 2 Dec 2019 18:44:23 +0100 Subject: [PATCH 07/10] bless polonius output of test ui/nll/outlives-suggestion-simple.rs The polonius output has one more error which should be displayed in the regular case, but error reporting in the regular case stopped at the first error. Admittedly it would be nice to combine suggestions for the same source lifetime so that `'a: 'b` and `'a: 'c` are not bothsuggested, but instead a single `'a: 'b + 'c` is. --- ...outlives-suggestion-simple.polonius.stderr | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 src/test/ui/nll/outlives-suggestion-simple.polonius.stderr diff --git a/src/test/ui/nll/outlives-suggestion-simple.polonius.stderr b/src/test/ui/nll/outlives-suggestion-simple.polonius.stderr new file mode 100644 index 0000000000000..815744618f62c --- /dev/null +++ b/src/test/ui/nll/outlives-suggestion-simple.polonius.stderr @@ -0,0 +1,121 @@ +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:6:5 + | +LL | fn foo1<'a, 'b>(x: &'a usize) -> &'b usize { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +LL | x + | ^ returning this value requires that `'a` must outlive `'b` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:10:5 + | +LL | fn foo2<'a>(x: &'a usize) -> &'static usize { + | -- lifetime `'a` defined here +LL | x + | ^ returning this value requires that `'a` must outlive `'static` + | + = help: consider replacing `'a` with `'static` + +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:14:5 + | +LL | fn foo3<'a, 'b>(x: &'a usize, y: &'b usize) -> (&'b usize, &'a usize) { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +LL | (x, y) + | ^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:14:5 + | +LL | fn foo3<'a, 'b>(x: &'a usize, y: &'b usize) -> (&'b usize, &'a usize) { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +LL | (x, y) + | ^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | + = help: consider adding the following bound: `'b: 'a` + +help: `'a` and `'b` must be the same: replace one with the other + +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:22:5 + | +LL | fn foo4<'a, 'b, 'c>(x: &'a usize) -> (&'b usize, &'c usize) { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... +LL | (x, x) + | ^^^^^^ returning this value requires that `'a` must outlive `'b` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:22:5 + | +LL | fn foo4<'a, 'b, 'c>(x: &'a usize) -> (&'b usize, &'c usize) { + | -- -- lifetime `'c` defined here + | | + | lifetime `'a` defined here +... +LL | (x, x) + | ^^^^^^ returning this value requires that `'a` must outlive `'c` + | + = help: consider adding the following bound: `'a: 'c` + +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:31:9 + | +LL | pub fn foo<'a>(x: &'a usize) -> Self { + | -- lifetime `'a` defined here +LL | Foo { x } + | ^^^^^^^^^ returning this value requires that `'a` must outlive `'static` + | + = help: consider replacing `'a` with `'static` + +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:41:9 + | +LL | impl<'a> Bar<'a> { + | -- lifetime `'a` defined here +LL | pub fn get<'b>(&self) -> &'b usize { + | -- lifetime `'b` defined here +LL | self.x + | ^^^^^^ returning this value requires that `'a` must outlive `'b` + | + = help: consider adding the following bound: `'a: 'b` + +error: lifetime may not live long enough + --> $DIR/outlives-suggestion-simple.rs:52:9 + | +LL | impl<'a> Baz<'a> { + | -- lifetime `'a` defined here +LL | fn get<'b>(&'b self) -> &'a i32 { + | -- lifetime `'b` defined here +LL | self.x + | ^^^^^^ returning this value requires that `'b` must outlive `'a` + | + = help: consider adding the following bound: `'b: 'a` + +error[E0521]: borrowed data escapes outside of function + --> $DIR/outlives-suggestion-simple.rs:73:9 + | +LL | fn get_bar(&self) -> Bar2 { + | ----- + | | + | `self` is declared here, outside of the function body + | `self` is a reference that is only valid in the function body +LL | Bar2::new(&self) + | ^^^^^^^^^^^^^^^^ `self` escapes the function body here + +error: aborting due to 10 previous errors + From 720716f9d08094e66581ce069caaa500ee4e04e6 Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Mon, 2 Dec 2019 18:54:42 +0100 Subject: [PATCH 08/10] bless polonius output due to lacking the 'static special-casing --- ...xpect-region-supply-region.polonius.stderr | 56 +++++++++++++++++ .../error-handling.polonius.stderr | 12 ++++ .../closure-substs.polonius.stderr | 60 +++++++++++++++++++ 3 files changed, 128 insertions(+) create mode 100644 src/test/ui/closures/closure-expected-type/expect-region-supply-region.polonius.stderr create mode 100644 src/test/ui/impl-trait/multiple-lifetimes/error-handling.polonius.stderr create mode 100644 src/test/ui/nll/user-annotations/closure-substs.polonius.stderr diff --git a/src/test/ui/closures/closure-expected-type/expect-region-supply-region.polonius.stderr b/src/test/ui/closures/closure-expected-type/expect-region-supply-region.polonius.stderr new file mode 100644 index 0000000000000..2a7461fb469b2 --- /dev/null +++ b/src/test/ui/closures/closure-expected-type/expect-region-supply-region.polonius.stderr @@ -0,0 +1,56 @@ +error[E0521]: borrowed data escapes outside of closure + --> $DIR/expect-region-supply-region.rs:18:9 + | +LL | let mut f: Option<&u32> = None; + | ----- `f` is declared here, outside of the closure body +LL | closure_expecting_bound(|x| { + | - `x` is a reference that is only valid in the closure body +LL | f = Some(x); + | ^^^^^^^^^^^ `x` escapes the closure body here + +error[E0521]: borrowed data escapes outside of closure + --> $DIR/expect-region-supply-region.rs:28:9 + | +LL | let mut f: Option<&u32> = None; + | ----- `f` is declared here, outside of the closure body +LL | closure_expecting_bound(|x: &u32| { + | - `x` is a reference that is only valid in the closure body +LL | f = Some(x); + | ^^^^^^^^^^^ `x` escapes the closure body here + +error: lifetime may not live long enough + --> $DIR/expect-region-supply-region.rs:37:30 + | +LL | fn expect_bound_supply_named<'x>() { + | -- lifetime `'x` defined here +... +LL | closure_expecting_bound(|x: &'x u32| { + | ^ - let's call the lifetime of this reference `'1` + | | + | requires that `'1` must outlive `'x` + +error[E0521]: borrowed data escapes outside of closure + --> $DIR/expect-region-supply-region.rs:42:9 + | +LL | let mut f: Option<&u32> = None; + | ----- `f` is declared here, outside of the closure body +... +LL | closure_expecting_bound(|x: &'x u32| { + | - `x` is a reference that is only valid in the closure body +... +LL | f = Some(x); + | ^^^^^^^^^^^ `x` escapes the closure body here + +error: lifetime may not live long enough + --> $DIR/expect-region-supply-region.rs:37:30 + | +LL | fn expect_bound_supply_named<'x>() { + | -- lifetime `'x` defined here +... +LL | closure_expecting_bound(|x: &'x u32| { + | ^ requires that `'x` must outlive `'static` + | + = help: consider replacing `'x` with `'static` + +error: aborting due to 5 previous errors + diff --git a/src/test/ui/impl-trait/multiple-lifetimes/error-handling.polonius.stderr b/src/test/ui/impl-trait/multiple-lifetimes/error-handling.polonius.stderr new file mode 100644 index 0000000000000..72e8fa33d7b4d --- /dev/null +++ b/src/test/ui/impl-trait/multiple-lifetimes/error-handling.polonius.stderr @@ -0,0 +1,12 @@ +error: lifetime may not live long enough + --> $DIR/error-handling.rs:13:56 + | +LL | fn foo<'a, 'b, 'c>(x: &'static i32, mut y: &'a i32) -> E<'b, 'c> { + | -- -- lifetime `'b` defined here ^^^^^^^^^ opaque type requires that `'a` must outlive `'b` + | | + | lifetime `'a` defined here + | + = help: consider adding the following bound: `'a: 'b` + +error: aborting due to previous error + diff --git a/src/test/ui/nll/user-annotations/closure-substs.polonius.stderr b/src/test/ui/nll/user-annotations/closure-substs.polonius.stderr new file mode 100644 index 0000000000000..d5bcdf6444171 --- /dev/null +++ b/src/test/ui/nll/user-annotations/closure-substs.polonius.stderr @@ -0,0 +1,60 @@ +error: lifetime may not live long enough + --> $DIR/closure-substs.rs:8:16 + | +LL | fn foo<'a>() { + | -- lifetime `'a` defined here +... +LL | return x; + | ^ returning this value requires that `'a` must outlive `'static` + | + = help: consider replacing `'a` with `'static` + +error: lifetime may not live long enough + --> $DIR/closure-substs.rs:15:16 + | +LL | |x: &i32| -> &'static i32 { + | - let's call the lifetime of this reference `'1` +LL | return x; + | ^ returning this value requires that `'1` must outlive `'static` + +error: lifetime may not live long enough + --> $DIR/closure-substs.rs:15:16 + | +LL | |x: &i32| -> &'static i32 { + | - ------------ return type of closure is &'2 i32 + | | + | let's call the lifetime of this reference `'1` +LL | return x; + | ^ returning this value requires that `'1` must outlive `'2` + +error: lifetime may not live long enough + --> $DIR/closure-substs.rs:22:9 + | +LL | fn bar<'a>() { + | -- lifetime `'a` defined here +... +LL | b(x); + | ^^^^ argument requires that `'a` must outlive `'static` + | + = help: consider replacing `'a` with `'static` + +error[E0521]: borrowed data escapes outside of closure + --> $DIR/closure-substs.rs:29:9 + | +LL | |x: &i32, b: fn(&'static i32)| { + | - `x` is a reference that is only valid in the closure body +LL | b(x); + | ^^^^ `x` escapes the closure body here + +error[E0521]: borrowed data escapes outside of closure + --> $DIR/closure-substs.rs:29:9 + | +LL | |x: &i32, b: fn(&'static i32)| { + | - - `b` is declared here, outside of the closure body + | | + | `x` is a reference that is only valid in the closure body +LL | b(x); + | ^^^^ `x` escapes the closure body here + +error: aborting due to 6 previous errors + From e2230a4366dc7854f766e4f79e964f719671fa0c Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Mon, 2 Dec 2019 19:37:24 +0100 Subject: [PATCH 09/10] appease the vociferous tidy --- src/librustc_mir/borrow_check/nll/mod.rs | 16 +++++++++++++--- .../borrow_check/nll/region_infer/mod.rs | 13 +++++++++---- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index 5be299f2a9440..0c2e2320bf984 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -235,7 +235,10 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( // `known_subset` facts. for (fr1, fr2) in universal_region_relations.known_outlives() { if fr1 != fr2 { - debug!("compute_regions: emitting polonius `known_subset` fr1={:?}, fr2={:?}", fr1, fr2); + debug!( + "compute_regions: emitting polonius `known_subset` fr1={:?}, fr2={:?}", + fr1, fr2 + ); all_facts.known_subset.push((*fr1, *fr2)); } } @@ -314,8 +317,15 @@ 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()); + 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. diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 574c64aa571cb..8d0f1b8e24a6b 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -1426,7 +1426,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { region_naming: &mut RegionErrorNamingCtx, polonius_output: Rc, ) { - debug!("check_polonius_subset_errors: {} subset_errors", polonius_output.subset_errors.len()); + debug!( + "check_polonius_subset_errors: {} subset_errors", + polonius_output.subset_errors.len() + ); let mut outlives_suggestion = OutlivesSuggestionBuilder::new(mir_def_id, local_names); @@ -1647,10 +1650,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { // Grow `shorter_fr` until we find some non-local regions. (We // always will.) We'll call them `shorter_fr+` -- they're ever // so slightly larger than `shorter_fr`. - let shorter_fr_plus = - self.universal_region_relations.non_local_upper_bounds(&shorter_fr); + let shorter_fr_plus = self + .universal_region_relations + .non_local_upper_bounds(&shorter_fr); debug!( - "report_or_propagate_universal_region_error: shorter_fr_plus={:?}", shorter_fr_plus + "report_or_propagate_universal_region_error: shorter_fr_plus={:?}", + shorter_fr_plus ); for &&fr in &shorter_fr_plus { // Push the constraint `fr-: shorter_fr+` From 1314ba323b6612d5109344c1d8bf9ae16e1e421f Mon Sep 17 00:00:00 2001 From: Remy Rakic Date: Tue, 3 Dec 2019 14:07:41 +0100 Subject: [PATCH 10/10] add subset relations test using polonius It's a relatively simple smoke-test for subset errors, executed outside of the polonius compare-mode. --- src/test/ui/nll/polonius/subset-relations.rs | 30 +++++++++++++++++++ .../ui/nll/polonius/subset-relations.stderr | 14 +++++++++ 2 files changed, 44 insertions(+) create mode 100644 src/test/ui/nll/polonius/subset-relations.rs create mode 100644 src/test/ui/nll/polonius/subset-relations.stderr diff --git a/src/test/ui/nll/polonius/subset-relations.rs b/src/test/ui/nll/polonius/subset-relations.rs new file mode 100644 index 0000000000000..3f6f67ebf4030 --- /dev/null +++ b/src/test/ui/nll/polonius/subset-relations.rs @@ -0,0 +1,30 @@ +// Checks that Polonius can compute cases of universal regions errors: +// "illegal subset relation errors", cases where analysis finds that +// two free regions outlive each other, without any evidence that this +// relation holds. + +// ignore-compare-mode-nll +// compile-flags: -Z borrowck=mir -Zpolonius + +// returning `y` requires that `'b: 'a`, but it's not known to be true +fn missing_subset<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 { + y //~ ERROR +} + +// `'b: 'a` is explicitly declared +fn valid_subset<'a, 'b: 'a>(x: &'a u32, y: &'b u32) -> &'a u32 { + y +} + +// because of `x`, it is implied that `'b: 'a` holds +fn implied_bounds_subset<'a, 'b>(x: &'a &'b mut u32) -> &'a u32 { + x +} + +// `'b: 'a` is declared, and `'a: 'c` is known via implied bounds: +// `'b: 'c` is therefore known to hold transitively +fn transitively_valid_subset<'a, 'b: 'a, 'c>(x: &'c &'a u32, y: &'b u32) -> &'c u32 { + y +} + +fn main() {} diff --git a/src/test/ui/nll/polonius/subset-relations.stderr b/src/test/ui/nll/polonius/subset-relations.stderr new file mode 100644 index 0000000000000..63645106f82c1 --- /dev/null +++ b/src/test/ui/nll/polonius/subset-relations.stderr @@ -0,0 +1,14 @@ +error: lifetime may not live long enough + --> $DIR/subset-relations.rs:11:5 + | +LL | fn missing_subset<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +LL | y + | ^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | + = help: consider adding the following bound: `'b: 'a` + +error: aborting due to previous error +