From de7a8305ae2b6eef11133de783f5089a94928f79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 4 Nov 2023 00:55:27 +0000 Subject: [PATCH 1/2] traverse region graph instead of SCCs to compute polonius loan scopes By using SCC for better performance, we also have to take into account SCCs whose representative is an existential region but also contains a placeholder. By only checking the representative, we may miss that the loan escapes the function. This can be fixed by picking a better representative, or removing placeholders from the main path. This is the simplest fix: forgo efficiency and traverse the region graph instead of the SCCs. --- compiler/rustc_borrowck/src/dataflow.rs | 6 +++--- .../rustc_borrowck/src/region_infer/mod.rs | 21 ++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_borrowck/src/dataflow.rs b/compiler/rustc_borrowck/src/dataflow.rs index 16814950b0d98..8676d2ba7c4c0 100644 --- a/compiler/rustc_borrowck/src/dataflow.rs +++ b/compiler/rustc_borrowck/src/dataflow.rs @@ -273,11 +273,10 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> { ) { let sccs = self.regioncx.constraint_sccs(); let universal_regions = self.regioncx.universal_regions(); - let issuing_region_scc = sccs.scc(issuing_region); // We first handle the cases where the loan doesn't go out of scope, depending on the issuing // region's successors. - for scc in sccs.depth_first_search(issuing_region_scc) { + for successor in self.regioncx.region_graph().depth_first_search(issuing_region) { // 1. Via applied member constraints // // The issuing region can flow into the choice regions, and they are either: @@ -290,6 +289,7 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> { // For additional insurance via fuzzing and crater, we verify that the constraint's min // choice indeed escapes the function. In the future, we could e.g. turn this check into // a debug assert and early return as an optimization. + let scc = sccs.scc(successor); for constraint in self.regioncx.applied_member_constraints(scc) { if universal_regions.is_universal_region(constraint.min_choice) { return; @@ -300,7 +300,7 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> { // // If the issuing region outlives such a region, its loan escapes the function and // cannot go out of scope. We can early return. - if self.regioncx.scc_is_live_at_all_points(scc) { + if self.regioncx.is_region_live_at_all_points(successor) { return; } } diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 05c2cbd49692c..b1f91a0562822 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -22,11 +22,10 @@ use rustc_middle::traits::ObligationCauseCode; use rustc_middle::ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable, TypeVisitableExt}; use rustc_span::Span; +use crate::constraints::graph::{self, NormalConstraintGraph, RegionGraph}; use crate::dataflow::BorrowIndex; use crate::{ - constraints::{ - graph::NormalConstraintGraph, ConstraintSccIndex, OutlivesConstraint, OutlivesConstraintSet, - }, + constraints::{ConstraintSccIndex, OutlivesConstraint, OutlivesConstraintSet}, diagnostics::{RegionErrorKind, RegionErrors, UniverseInfo}, member_constraints::{MemberConstraintSet, NllMemberConstraintIndex}, nll::PoloniusOutput, @@ -2293,19 +2292,21 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.constraint_sccs.as_ref() } - /// Returns whether the given SCC is live at all points: whether the representative is a + /// Access to the region graph, built from the outlives constraints. + pub(crate) fn region_graph(&self) -> RegionGraph<'_, 'tcx, graph::Normal> { + self.constraint_graph.region_graph(&self.constraints, self.universal_regions.fr_static) + } + + /// Returns whether the given region is considered live at all points: whether it is a /// placeholder or a free region. - pub(crate) fn scc_is_live_at_all_points(&self, scc: ConstraintSccIndex) -> bool { + pub(crate) fn is_region_live_at_all_points(&self, region: RegionVid) -> bool { // FIXME: there must be a cleaner way to find this information. At least, when // higher-ranked subtyping is abstracted away from the borrowck main path, we'll only // need to check whether this is a universal region. - let representative = self.scc_representatives[scc]; - let origin = self.var_infos[representative].origin; + let origin = self.region_definition(region).origin; let live_at_all_points = matches!( origin, - RegionVariableOrigin::Nll( - NllRegionVariableOrigin::Placeholder(_) | NllRegionVariableOrigin::FreeRegion - ) + NllRegionVariableOrigin::Placeholder(_) | NllRegionVariableOrigin::FreeRegion ); live_at_all_points } From 2dff90dc233dd30244191af6a71e3b2d85a551eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 4 Nov 2023 01:03:49 +0000 Subject: [PATCH 2/2] add test for issue 117146 --- ...insensitive-scopes-issue-117146.nll.stderr | 40 +++++++++++++++++++ ...sitive-scopes-issue-117146.polonius.stderr | 40 +++++++++++++++++++ ...ocation-insensitive-scopes-issue-117146.rs | 20 ++++++++++ 3 files changed, 100 insertions(+) create mode 100644 tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.nll.stderr create mode 100644 tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.polonius.stderr create mode 100644 tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.rs diff --git a/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.nll.stderr b/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.nll.stderr new file mode 100644 index 0000000000000..5227ca8ec17d8 --- /dev/null +++ b/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.nll.stderr @@ -0,0 +1,40 @@ +error[E0597]: `a` does not live long enough + --> $DIR/location-insensitive-scopes-issue-117146.rs:10:18 + | +LL | let b = |_| &a; + | --- -^ + | | || + | | |borrowed value does not live long enough + | | returning this value requires that `a` is borrowed for `'static` + | value captured here +... +LL | } + | - `a` dropped here while still borrowed + | +note: due to current limitations in the borrow checker, this implies a `'static` lifetime + --> $DIR/location-insensitive-scopes-issue-117146.rs:20:22 + | +LL | fn bad &()>(_: F) {} + | ^^^ + +error: implementation of `Fn` is not general enough + --> $DIR/location-insensitive-scopes-issue-117146.rs:13:5 + | +LL | bad(&b); + | ^^^^^^^ implementation of `Fn` is not general enough + | + = note: closure with signature `fn(&'2 ()) -> &()` must implement `Fn<(&'1 (),)>`, for any lifetime `'1`... + = note: ...but it actually implements `Fn<(&'2 (),)>`, for some specific lifetime `'2` + +error: implementation of `FnOnce` is not general enough + --> $DIR/location-insensitive-scopes-issue-117146.rs:13:5 + | +LL | bad(&b); + | ^^^^^^^ implementation of `FnOnce` is not general enough + | + = note: closure with signature `fn(&'2 ()) -> &()` must implement `FnOnce<(&'1 (),)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 (),)>`, for some specific lifetime `'2` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0597`. diff --git a/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.polonius.stderr b/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.polonius.stderr new file mode 100644 index 0000000000000..5227ca8ec17d8 --- /dev/null +++ b/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.polonius.stderr @@ -0,0 +1,40 @@ +error[E0597]: `a` does not live long enough + --> $DIR/location-insensitive-scopes-issue-117146.rs:10:18 + | +LL | let b = |_| &a; + | --- -^ + | | || + | | |borrowed value does not live long enough + | | returning this value requires that `a` is borrowed for `'static` + | value captured here +... +LL | } + | - `a` dropped here while still borrowed + | +note: due to current limitations in the borrow checker, this implies a `'static` lifetime + --> $DIR/location-insensitive-scopes-issue-117146.rs:20:22 + | +LL | fn bad &()>(_: F) {} + | ^^^ + +error: implementation of `Fn` is not general enough + --> $DIR/location-insensitive-scopes-issue-117146.rs:13:5 + | +LL | bad(&b); + | ^^^^^^^ implementation of `Fn` is not general enough + | + = note: closure with signature `fn(&'2 ()) -> &()` must implement `Fn<(&'1 (),)>`, for any lifetime `'1`... + = note: ...but it actually implements `Fn<(&'2 (),)>`, for some specific lifetime `'2` + +error: implementation of `FnOnce` is not general enough + --> $DIR/location-insensitive-scopes-issue-117146.rs:13:5 + | +LL | bad(&b); + | ^^^^^^^ implementation of `FnOnce` is not general enough + | + = note: closure with signature `fn(&'2 ()) -> &()` must implement `FnOnce<(&'1 (),)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 (),)>`, for some specific lifetime `'2` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0597`. diff --git a/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.rs b/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.rs new file mode 100644 index 0000000000000..c165e7a1d1a53 --- /dev/null +++ b/tests/ui/nll/polonius/location-insensitive-scopes-issue-117146.rs @@ -0,0 +1,20 @@ +// This is a non-regression test for issue #117146, where NLL and `-Zpolonius=next` computed +// different loan scopes when a region flowed into an SCC whose representative was an existential +// region. + +// revisions: nll polonius +// [polonius] compile-flags: -Zpolonius=next + +fn main() { + let a = (); + let b = |_| &a; + //[nll]~^ ERROR `a` does not live long enough + //[polonius]~^^ ERROR `a` does not live long enough + bad(&b); + //[nll]~^ ERROR implementation of `Fn` + //[nll]~| ERROR implementation of `FnOnce` + //[polonius]~^^^ ERROR implementation of `Fn` + //[polonius]~| ERROR implementation of `FnOnce` +} + +fn bad &()>(_: F) {}