Skip to content

Commit

Permalink
Auto merge of #88698 - Noble-Mushtak:master, r=nikomatsakis,oli-obk
Browse files Browse the repository at this point in the history
Add check that live_region is live in sanitize_promoted

This pull request fixes #88434 by adding a check in `sanitize_promoted` to ensure that only regions which are actually live are added to the `liveness_constraints` of the `BorrowCheckContext`.

To implement this change, I needed to add a method to `LivenessValues` which gets the elements contained by a region:

    /// Returns an iterator of all the elements contained by the region `r`
    crate fn get_elements(&self, row: N) -> impl Iterator<Item = Location> + '_

Then, inside `sanitize_promoted`, we check whether the iterator returned by this method is non-empty to ensure that the region is actually live at at least one location before adding that region to the `liveness_constraints` of the `BorrowCheckContext`.

This is my first pull request to the Rust repo, so any feedback on how I can improve this pull request or if there is a better way to fix this issue would be very appreciated.
  • Loading branch information
bors committed Oct 14, 2021
2 parents c34ac87 + 8fc329f commit 0a56eb1
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 16 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/region_infer/mod.rs
Expand Up @@ -2154,7 +2154,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// appears to be the most interesting point to report to the
// user via an even more ad-hoc guess.
categorized_path.sort_by(|p0, p1| p0.category.cmp(&p1.category));
debug!("`: sorted_path={:#?}", categorized_path);
debug!("best_blame_constraint: sorted_path={:#?}", categorized_path);

categorized_path.remove(0)
}
Expand Down
20 changes: 11 additions & 9 deletions compiler/rustc_borrowck/src/region_infer/values.rs
Expand Up @@ -174,17 +174,19 @@ impl<N: Idx> LivenessValues<N> {
self.points.contains(row, index)
}

/// Returns an iterator of all the elements contained by the region `r`
crate fn get_elements(&self, row: N) -> impl Iterator<Item = Location> + '_ {
self.points
.row(row)
.into_iter()
.flat_map(|set| set.iter())
.take_while(move |&p| self.elements.point_in_range(p))
.map(move |p| self.elements.to_location(p))
}

/// Returns a "pretty" string value of the region. Meant for debugging.
crate fn region_value_str(&self, r: N) -> String {
region_value_str(
self.points
.row(r)
.into_iter()
.flat_map(|set| set.iter())
.take_while(|&p| self.elements.point_in_range(p))
.map(|p| self.elements.to_location(p))
.map(RegionElement::Location),
)
region_value_str(self.get_elements(r).map(RegionElement::Location))
}
}

Expand Down
17 changes: 11 additions & 6 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Expand Up @@ -663,12 +663,17 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
}
self.cx.borrowck_context.constraints.outlives_constraints.push(constraint)
}
for live_region in liveness_constraints.rows() {
self.cx
.borrowck_context
.constraints
.liveness_constraints
.add_element(live_region, location);
for region in liveness_constraints.rows() {
// If the region is live at at least one location in the promoted MIR,
// then add a liveness constraint to the main MIR for this region
// at the location provided as an argument to this method
if let Some(_) = liveness_constraints.get_elements(region).next() {
self.cx
.borrowck_context
.constraints
.liveness_constraints
.add_element(region, location);
}
}

if !closure_bounds.is_empty() {
Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/borrowck/issue-88434-minimal-example.rs
@@ -0,0 +1,13 @@
#![feature(const_fn_trait_bound)]
// Regression test related to issue 88434

const _CONST: &() = &f(&|_| {});

const fn f<F>(_: &F)
where
F: FnMut(&u8),
{
panic!() //~ ERROR evaluation of constant value failed
}

fn main() { }
17 changes: 17 additions & 0 deletions src/test/ui/borrowck/issue-88434-minimal-example.stderr
@@ -0,0 +1,17 @@
error[E0080]: evaluation of constant value failed
--> $DIR/issue-88434-minimal-example.rs:10:5
|
LL | const _CONST: &() = &f(&|_| {});
| ---------- inside `_CONST` at $DIR/issue-88434-minimal-example.rs:4:22
...
LL | panic!()
| ^^^^^^^^
| |
| the evaluated program panicked at 'explicit panic', $DIR/issue-88434-minimal-example.rs:10:5
| inside `f::<[closure@$DIR/issue-88434-minimal-example.rs:4:25: 4:31]>` at $SRC_DIR/std/src/panic.rs:LL:COL
|
= note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.
13 changes: 13 additions & 0 deletions src/test/ui/borrowck/issue-88434-removal-index-should-be-less.rs
@@ -0,0 +1,13 @@
#![feature(const_fn_trait_bound)]
// Regression test for issue 88434

const _CONST: &[u8] = &f(&[], |_| {});

const fn f<F>(_: &[u8], _: F) -> &[u8]
where
F: FnMut(&u8),
{
panic!() //~ ERROR evaluation of constant value failed
}

fn main() { }
@@ -0,0 +1,17 @@
error[E0080]: evaluation of constant value failed
--> $DIR/issue-88434-removal-index-should-be-less.rs:10:5
|
LL | const _CONST: &[u8] = &f(&[], |_| {});
| -------------- inside `_CONST` at $DIR/issue-88434-removal-index-should-be-less.rs:4:24
...
LL | panic!()
| ^^^^^^^^
| |
| the evaluated program panicked at 'explicit panic', $DIR/issue-88434-removal-index-should-be-less.rs:10:5
| inside `f::<[closure@$DIR/issue-88434-removal-index-should-be-less.rs:4:31: 4:37]>` at $SRC_DIR/std/src/panic.rs:LL:COL
|
= note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.

0 comments on commit 0a56eb1

Please sign in to comment.