From 8b76518a28e5e826c115fc06d8eebe51a59a58bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 19 Oct 2023 21:11:21 +0000 Subject: [PATCH 1/4] add non-regression test for issue 116657 --- ...insensitive-scopes-issue-116657.nll.stderr | 36 +++++++++++++++++++ ...sitive-scopes-issue-116657.polonius.stderr | 36 +++++++++++++++++++ ...ocation-insensitive-scopes-issue-116657.rs | 33 +++++++++++++++++ 3 files changed, 105 insertions(+) create mode 100644 tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.nll.stderr create mode 100644 tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.polonius.stderr create mode 100644 tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.rs diff --git a/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.nll.stderr b/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.nll.stderr new file mode 100644 index 0000000000000..6f9b330316378 --- /dev/null +++ b/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.nll.stderr @@ -0,0 +1,36 @@ +error[E0046]: not all trait items implemented, missing: `call` + --> $DIR/location-insensitive-scopes-issue-116657.rs:18:1 + | +LL | fn call(x: Self) -> Self::Output; + | --------------------------------- `call` from trait +... +LL | impl Callable for T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `call` in implementation + +error: unconstrained opaque type + --> $DIR/location-insensitive-scopes-issue-116657.rs:22:19 + | +LL | type Output = impl PlusOne; + | ^^^^^^^^^^^^ + | + = note: `Output` must be used in combination with a concrete type within the same impl + +error[E0700]: hidden type for `impl PlusOne` captures lifetime that does not appear in bounds + --> $DIR/location-insensitive-scopes-issue-116657.rs:28:5 + | +LL | fn test<'a>(y: &'a mut i32) -> impl PlusOne { + | -- ------------ opaque type defined here + | | + | hidden type `<&'a mut i32 as Callable>::Output` captures the lifetime `'a` as defined here +LL | <&mut i32 as Callable>::call(y) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: to declare that `impl PlusOne` captures `'a`, you can add an explicit `'a` lifetime bound + | +LL | fn test<'a>(y: &'a mut i32) -> impl PlusOne + 'a { + | ++++ + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0046, E0700. +For more information about an error, try `rustc --explain E0046`. diff --git a/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.polonius.stderr b/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.polonius.stderr new file mode 100644 index 0000000000000..6f9b330316378 --- /dev/null +++ b/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.polonius.stderr @@ -0,0 +1,36 @@ +error[E0046]: not all trait items implemented, missing: `call` + --> $DIR/location-insensitive-scopes-issue-116657.rs:18:1 + | +LL | fn call(x: Self) -> Self::Output; + | --------------------------------- `call` from trait +... +LL | impl Callable for T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `call` in implementation + +error: unconstrained opaque type + --> $DIR/location-insensitive-scopes-issue-116657.rs:22:19 + | +LL | type Output = impl PlusOne; + | ^^^^^^^^^^^^ + | + = note: `Output` must be used in combination with a concrete type within the same impl + +error[E0700]: hidden type for `impl PlusOne` captures lifetime that does not appear in bounds + --> $DIR/location-insensitive-scopes-issue-116657.rs:28:5 + | +LL | fn test<'a>(y: &'a mut i32) -> impl PlusOne { + | -- ------------ opaque type defined here + | | + | hidden type `<&'a mut i32 as Callable>::Output` captures the lifetime `'a` as defined here +LL | <&mut i32 as Callable>::call(y) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: to declare that `impl PlusOne` captures `'a`, you can add an explicit `'a` lifetime bound + | +LL | fn test<'a>(y: &'a mut i32) -> impl PlusOne + 'a { + | ++++ + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0046, E0700. +For more information about an error, try `rustc --explain E0046`. diff --git a/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.rs b/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.rs new file mode 100644 index 0000000000000..ec17e0b093b38 --- /dev/null +++ b/tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.rs @@ -0,0 +1,33 @@ +// This is a non-regression test for issue #116657, where NLL and `-Zpolonius=next` computed +// different loan scopes when a member constraint was not ultimately applied. + +// revisions: nll polonius +// [polonius] compile-flags: -Zpolonius=next + +#![feature(impl_trait_in_assoc_type)] + +trait Callable { + type Output; + fn call(x: Self) -> Self::Output; +} + +trait PlusOne {} + +impl<'a> PlusOne for &'a mut i32 {} + +impl Callable for T { + //[nll]~^ ERROR not all trait items implemented + //[polonius]~^^ ERROR not all trait items implemented + + type Output = impl PlusOne; + //[nll]~^ ERROR unconstrained opaque type + //[polonius]~^^ ERROR unconstrained opaque type +} + +fn test<'a>(y: &'a mut i32) -> impl PlusOne { + <&mut i32 as Callable>::call(y) + //[nll]~^ ERROR hidden type for `impl PlusOne` captures lifetime + //[polonius]~^^ ERROR hidden type for `impl PlusOne` captures lifetime +} + +fn main() {} From c69bd9480abd4d26d7e2f5c877053878eec334a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 19 Oct 2023 21:25:12 +0000 Subject: [PATCH 2/4] make `applied_member_constraints` accept an SCC instead of a region --- compiler/rustc_borrowck/src/region_infer/mod.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 96cbe98c21694..e9c7d2918b9f6 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -644,11 +644,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.scc_universes[scc] } - /// Once region solving has completed, this function will return - /// the member constraints that were applied to the value of a given - /// region `r`. See `AppliedMemberConstraint`. - pub(crate) fn applied_member_constraints(&self, r: RegionVid) -> &[AppliedMemberConstraint] { - let scc = self.constraint_sccs.scc(r); + /// Once region solving has completed, this function will return the member constraints that + /// were applied to the value of a given SCC `scc`. See `AppliedMemberConstraint`. + pub(crate) fn applied_member_constraints( + &self, + scc: ConstraintSccIndex, + ) -> &[AppliedMemberConstraint] { binary_search_util::binary_search_slice( &self.member_constraints_applied, |applied| applied.member_region_scc, @@ -1945,7 +1946,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { // Member constraints can also give rise to `'r: 'x` edges that // were not part of the graph initially, so watch out for those. // (But they are extremely rare; this loop is very cold.) - for constraint in self.applied_member_constraints(r) { + for constraint in self.applied_member_constraints(self.constraint_sccs.scc(r)) { let p_c = &self.member_constraints[constraint.member_constraint_index]; let constraint = OutlivesConstraint { sup: r, From fa45efaafbad50454f8de065be8c9fdea7d89094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 19 Oct 2023 21:44:03 +0000 Subject: [PATCH 3/4] consider a loan escapes the function via applied member constraints --- compiler/rustc_borrowck/src/dataflow.rs | 21 ++++++++++++------- .../rustc_borrowck/src/region_infer/mod.rs | 5 ----- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_borrowck/src/dataflow.rs b/compiler/rustc_borrowck/src/dataflow.rs index 6ea84620bbed8..16814950b0d98 100644 --- a/compiler/rustc_borrowck/src/dataflow.rs +++ b/compiler/rustc_borrowck/src/dataflow.rs @@ -272,21 +272,28 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> { loan_issued_at: Location, ) { 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) { - // 1. Via member constraints + // 1. Via applied member constraints // // The issuing region can flow into the choice regions, and they are either: // - placeholders or free regions themselves, // - or also transitively outlive a free region. // - // That is to say, if there are member constraints here, the loan escapes the function - // and cannot go out of scope. We can early return. - if self.regioncx.scc_has_member_constraints(scc) { - return; + // That is to say, if there are applied member constraints here, the loan escapes the + // function and cannot go out of scope. We could early return here. + // + // 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. + for constraint in self.regioncx.applied_member_constraints(scc) { + if universal_regions.is_universal_region(constraint.min_choice) { + return; + } } // 2. Via regions that are live at all points: placeholders and free regions. @@ -413,12 +420,12 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { let mut polonius_prec = PoloniusOutOfScopePrecomputer::new(body, regioncx); for (loan_idx, loan_data) in borrow_set.iter_enumerated() { let issuing_region = loan_data.region; - let issued_location = loan_data.reserve_location; + let loan_issued_at = loan_data.reserve_location; polonius_prec.precompute_loans_out_of_scope( loan_idx, issuing_region, - issued_location, + loan_issued_at, ); } diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index e9c7d2918b9f6..05c2cbd49692c 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -2293,11 +2293,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.constraint_sccs.as_ref() } - /// Returns whether the given SCC has any member constraints. - pub(crate) fn scc_has_member_constraints(&self, scc: ConstraintSccIndex) -> bool { - self.member_constraints.indices(scc).next().is_some() - } - /// Returns whether the given SCC is live at all points: whether the representative is a /// placeholder or a free region. pub(crate) fn scc_is_live_at_all_points(&self, scc: ConstraintSccIndex) -> bool { From d9c213cd5e5b5ec480297960ca080ed5b493f4f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Fri, 20 Oct 2023 15:32:22 +0000 Subject: [PATCH 4/4] slight Default cleanup for option --- compiler/rustc_session/src/config.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index bbba800e84081..ef0ad69c4a2c3 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -3420,9 +3420,10 @@ impl DumpMonoStatsFormat { /// `-Zpolonius` values, enabling the borrow checker polonius analysis, and which version: legacy, /// or future prototype. -#[derive(Clone, Copy, PartialEq, Hash, Debug)] +#[derive(Clone, Copy, PartialEq, Hash, Debug, Default)] pub enum Polonius { /// The default value: disabled. + #[default] Off, /// Legacy version, using datalog and the `polonius-engine` crate. Historical value for `-Zpolonius`. @@ -3432,12 +3433,6 @@ pub enum Polonius { Next, } -impl Default for Polonius { - fn default() -> Self { - Polonius::Off - } -} - impl Polonius { /// Returns whether the legacy version of polonius is enabled pub fn is_legacy_enabled(&self) -> bool {