Skip to content

Commit

Permalink
Unrolled build for rust-lang#116960
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#116960 - lqd:applied-member-constraints-scope, r=matthewjasper

Location-insensitive polonius: consider a loan escaping if an SCC has member constraints applied only

The location-insensitive analysis considered loans to escape if there were member constraints, which makes *some* sense for scopes and matches the scopes that NLL computes on all the tests.

However, polonius and NLLs differ on the fuzzed case rust-lang#116657, where an SCC has member constraints but no applied ones (and is kinda surprising). The existing UI tests with member constraints impacting scopes all have some constraint applied.

This PR changes the location-insensitive analysis to consider a loan to escape if there are applied member constraints, and for extra paranoia/insurance via fuzzing and crater: actually checks the constraint's min choice is indeed a universal region as we expect. (This could be turned into a `debug_assert` and early return as a slight optimization after these periods of verification)

The 4 UI tests where member constraints are meaningful for computing scopes still pass obviously, and this also fixes rust-lang#116657.

r? `@matthewjasper`
  • Loading branch information
rust-timer authored Oct 23, 2023
2 parents 858a42b + d9c213c commit deef8a8
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 25 deletions.
21 changes: 14 additions & 7 deletions compiler/rustc_borrowck/src/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
);
}

Expand Down
18 changes: 7 additions & 11 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2292,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 {
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3463,9 +3463,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`.
Expand All @@ -3475,12 +3476,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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<T: PlusOne> 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`.
Original file line number Diff line number Diff line change
@@ -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<T: PlusOne> 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`.
33 changes: 33 additions & 0 deletions tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.rs
Original file line number Diff line number Diff line change
@@ -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<T: PlusOne> 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() {}

0 comments on commit deef8a8

Please sign in to comment.