Skip to content

Commit

Permalink
Track drop points in generator_interior
Browse files Browse the repository at this point in the history
This change adds the basic infrastructure for tracking drop ranges in
generator interior analysis, which allows us to exclude dropped types
from the generator type.

Not yet complete, but many of the async/await and generator tests pass.
The main missing piece is tracking branching control flow (e.g. around
an `if` expression). The patch does include support, however, for
multiple yields in th e same block.

Issue rust-lang#57478
  • Loading branch information
eholk committed Jan 18, 2022
1 parent 4be32f8 commit f712df8
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 99 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_passes/src/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,6 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h
hir::ExprKind::AssignOp(..)
| hir::ExprKind::Index(..)
| hir::ExprKind::Unary(..)
| hir::ExprKind::Call(..)
| hir::ExprKind::MethodCall(..) => {
// FIXME(https://github.com/rust-lang/rfcs/issues/811) Nested method calls
//
Expand Down
190 changes: 161 additions & 29 deletions compiler/rustc_typeck/src/check/generator_interior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
//! is calculated in `rustc_const_eval::transform::generator` and may be a subset of the
//! types computed here.

use crate::expr_use_visitor::{self, ExprUseVisitor};

use super::FnCtxt;
use hir::HirIdMap;
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_errors::pluralize;
use rustc_hir as hir;
Expand Down Expand Up @@ -34,6 +37,7 @@ struct InteriorVisitor<'a, 'tcx> {
guard_bindings: SmallVec<[SmallVec<[HirId; 4]>; 1]>,
guard_bindings_set: HirIdSet,
linted_values: HirIdSet,
drop_ranges: HirIdMap<DropRange>,
}

impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
Expand All @@ -48,9 +52,11 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
) {
use rustc_span::DUMMY_SP;

let ty = self.fcx.resolve_vars_if_possible(ty);

debug!(
"generator_interior: attempting to record type {:?} {:?} {:?} {:?}",
ty, scope, expr, source_span
"attempting to record type ty={:?}; hir_id={:?}; scope={:?}; expr={:?}; source_span={:?}; expr_count={:?}",
ty, hir_id, scope, expr, source_span, self.expr_count,
);

let live_across_yield = scope
Expand All @@ -68,6 +74,14 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
yield_data.expr_and_pat_count, self.expr_count, source_span
);

match self.drop_ranges.get(&hir_id) {
Some(range) if range.contains(yield_data.expr_and_pat_count) => {
debug!("value is dropped at yield point; not recording");
return None
}
_ => (),
}

// If it is a borrowing happening in the guard,
// it needs to be recorded regardless because they
// do live across this yield point.
Expand All @@ -85,7 +99,6 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
});

if let Some(yield_data) = live_across_yield {
let ty = self.fcx.resolve_vars_if_possible(ty);
debug!(
"type in expr = {:?}, scope = {:?}, type = {:?}, count = {}, yield_span = {:?}",
expr, scope, ty, self.expr_count, yield_data.span
Expand Down Expand Up @@ -154,7 +167,6 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
self.expr_count,
expr.map(|e| e.span)
);
let ty = self.fcx.resolve_vars_if_possible(ty);
if let Some((unresolved_type, unresolved_type_span)) =
self.fcx.unresolved_type_vars(&ty)
{
Expand All @@ -166,6 +178,39 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
}
}
}

fn visit_call(
&mut self,
call_expr: &'tcx Expr<'tcx>,
callee: &'tcx Expr<'tcx>,
args: &'tcx [Expr<'tcx>],
) {
match &callee.kind {
ExprKind::Path(qpath) => {
let res = self.fcx.typeck_results.borrow().qpath_res(qpath, callee.hir_id);
match res {
// Direct calls never need to keep the callee `ty::FnDef`
// ZST in a temporary, so skip its type, just in case it
// can significantly complicate the generator type.
Res::Def(
DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(_, CtorKind::Fn),
_,
) => {
// NOTE(eddyb) this assumes a path expression has
// no nested expressions to keep track of.
self.expr_count += 1;

// Record the rest of the call expression normally.
for arg in args {
self.visit_expr(arg);
}
}
_ => intravisit::walk_expr(self, call_expr),
}
}
_ => intravisit::walk_expr(self, call_expr),
}
}
}

pub fn resolve_interior<'a, 'tcx>(
Expand All @@ -176,6 +221,20 @@ pub fn resolve_interior<'a, 'tcx>(
kind: hir::GeneratorKind,
) {
let body = fcx.tcx.hir().body(body_id);

let mut drop_range_visitor = DropRangeVisitor::default();

// Run ExprUseVisitor to find where values are consumed.
ExprUseVisitor::new(
&mut drop_range_visitor,
&fcx.infcx,
def_id.expect_local(),
fcx.param_env,
&fcx.typeck_results.borrow(),
)
.consume_body(body);
intravisit::walk_body(&mut drop_range_visitor, body);

let mut visitor = InteriorVisitor {
fcx,
types: FxIndexSet::default(),
Expand All @@ -186,6 +245,7 @@ pub fn resolve_interior<'a, 'tcx>(
guard_bindings: <_>::default(),
guard_bindings_set: <_>::default(),
linted_values: <_>::default(),
drop_ranges: drop_range_visitor.drop_ranges,
};
intravisit::walk_body(&mut visitor, body);

Expand Down Expand Up @@ -313,32 +373,9 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {

fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
let mut guard_borrowing_from_pattern = false;

match &expr.kind {
ExprKind::Call(callee, args) => match &callee.kind {
ExprKind::Path(qpath) => {
let res = self.fcx.typeck_results.borrow().qpath_res(qpath, callee.hir_id);
match res {
// Direct calls never need to keep the callee `ty::FnDef`
// ZST in a temporary, so skip its type, just in case it
// can significantly complicate the generator type.
Res::Def(
DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(_, CtorKind::Fn),
_,
) => {
// NOTE(eddyb) this assumes a path expression has
// no nested expressions to keep track of.
self.expr_count += 1;

// Record the rest of the call expression normally.
for arg in *args {
self.visit_expr(arg);
}
}
_ => intravisit::walk_expr(self, expr),
}
}
_ => intravisit::walk_expr(self, expr),
},
ExprKind::Call(callee, args) => self.visit_call(expr, callee, args),
ExprKind::Path(qpath) => {
intravisit::walk_expr(self, expr);
let res = self.fcx.typeck_results.borrow().qpath_res(qpath, expr.hir_id);
Expand Down Expand Up @@ -617,3 +654,98 @@ fn check_must_not_suspend_def(
}
false
}

/// This struct facilitates computing the ranges for which a place is uninitialized.
#[derive(Default)]
struct DropRangeVisitor {
consumed_places: HirIdSet,
drop_ranges: HirIdMap<DropRange>,
expr_count: usize,
}

impl DropRangeVisitor {
fn record_drop(&mut self, hir_id: HirId) {
debug!("marking {:?} as dropped at {}", hir_id, self.expr_count);
self.drop_ranges.insert(hir_id, DropRange { dropped_at: self.expr_count });
}

/// ExprUseVisitor's consume callback doesn't go deep enough for our purposes in all
/// expressions. This method consumes a little deeper into the expression when needed.
fn consume_expr(&mut self, expr: &hir::Expr<'_>) {
self.record_drop(expr.hir_id);
match expr.kind {
hir::ExprKind::Path(hir::QPath::Resolved(
_,
hir::Path { res: hir::def::Res::Local(hir_id), .. },
)) => {
self.record_drop(*hir_id);
}
_ => (),
}
}
}

impl<'tcx> expr_use_visitor::Delegate<'tcx> for DropRangeVisitor {
fn consume(
&mut self,
place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>,
diag_expr_id: hir::HirId,
) {
debug!("consume {:?}; diag_expr_id={:?}", place_with_id, diag_expr_id);
self.consumed_places.insert(place_with_id.hir_id);
}

fn borrow(
&mut self,
_place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>,
_diag_expr_id: hir::HirId,
_bk: rustc_middle::ty::BorrowKind,
) {
}

fn mutate(
&mut self,
_assignee_place: &expr_use_visitor::PlaceWithHirId<'tcx>,
_diag_expr_id: hir::HirId,
) {
}

fn fake_read(
&mut self,
_place: expr_use_visitor::Place<'tcx>,
_cause: rustc_middle::mir::FakeReadCause,
_diag_expr_id: hir::HirId,
) {
}
}

impl<'tcx> Visitor<'tcx> for DropRangeVisitor {
type Map = intravisit::ErasedMap<'tcx>;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}

fn visit_expr(&mut self, expr: &Expr<'_>) {
intravisit::walk_expr(self, expr);

self.expr_count += 1;

if self.consumed_places.contains(&expr.hir_id) {
self.consume_expr(expr);
}
}
}

struct DropRange {
/// The post-order id of the point where this expression is dropped.
///
/// We can consider the value dropped at any post-order id greater than dropped_at.
dropped_at: usize,
}

impl DropRange {
fn contains(&self, id: usize) -> bool {
id >= self.dropped_at
}
}
8 changes: 4 additions & 4 deletions src/test/ui/async-await/async-fn-nonsend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ async fn fut() {}
async fn fut_arg<T>(_: T) {}

async fn local_dropped_before_await() {
// FIXME: it'd be nice for this to be allowed in a `Send` `async fn`
// this is okay now because of the drop
let x = non_send();
drop(x);
fut().await;
Expand All @@ -36,7 +36,7 @@ async fn non_send_temporary_in_match() {
}

async fn non_sync_with_method_call() {
// FIXME: it'd be nice for this to work.

let f: &mut std::fmt::Formatter = panic!();
if non_sync().fmt(f).unwrap() == () {
fut().await;
Expand All @@ -47,9 +47,9 @@ fn assert_send(_: impl Send) {}

pub fn pass_assert() {
assert_send(local_dropped_before_await());
//~^ ERROR future cannot be sent between threads safely

assert_send(non_send_temporary_in_match());
//~^ ERROR future cannot be sent between threads safely
assert_send(non_sync_with_method_call());
//~^ ERROR future cannot be sent between threads safely

}
16 changes: 0 additions & 16 deletions src/test/ui/async-await/unresolved_type_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,8 @@ async fn bar<T>() -> () {}
async fn foo() {
bar().await;
//~^ ERROR type inside `async fn` body must be known in this context
//~| ERROR type inside `async fn` body must be known in this context
//~| ERROR type inside `async fn` body must be known in this context
//~| ERROR type inside `async fn` body must be known in this context
//~| ERROR type inside `async fn` body must be known in this context
//~| NOTE cannot infer type for type parameter `T`
//~| NOTE cannot infer type for type parameter `T`
//~| NOTE cannot infer type for type parameter `T`
//~| NOTE cannot infer type for type parameter `T`
//~| NOTE cannot infer type for type parameter `T`
//~| NOTE the type is part of the `async fn` body because of this `await`
//~| NOTE the type is part of the `async fn` body because of this `await`
//~| NOTE the type is part of the `async fn` body because of this `await`
//~| NOTE the type is part of the `async fn` body because of this `await`
//~| NOTE the type is part of the `async fn` body because of this `await`
//~| NOTE in this expansion of desugaring of `await`
//~| NOTE in this expansion of desugaring of `await`
//~| NOTE in this expansion of desugaring of `await`
//~| NOTE in this expansion of desugaring of `await`
//~| NOTE in this expansion of desugaring of `await`
}
fn main() {}
50 changes: 1 addition & 49 deletions src/test/ui/async-await/unresolved_type_param.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,54 +10,6 @@ note: the type is part of the `async fn` body because of this `await`
LL | bar().await;
| ^^^^^^

error[E0698]: type inside `async fn` body must be known in this context
--> $DIR/unresolved_type_param.rs:9:5
|
LL | bar().await;
| ^^^ cannot infer type for type parameter `T` declared on the function `bar`
|
note: the type is part of the `async fn` body because of this `await`
--> $DIR/unresolved_type_param.rs:9:10
|
LL | bar().await;
| ^^^^^^

error[E0698]: type inside `async fn` body must be known in this context
--> $DIR/unresolved_type_param.rs:9:5
|
LL | bar().await;
| ^^^ cannot infer type for type parameter `T` declared on the function `bar`
|
note: the type is part of the `async fn` body because of this `await`
--> $DIR/unresolved_type_param.rs:9:10
|
LL | bar().await;
| ^^^^^^

error[E0698]: type inside `async fn` body must be known in this context
--> $DIR/unresolved_type_param.rs:9:5
|
LL | bar().await;
| ^^^ cannot infer type for type parameter `T` declared on the function `bar`
|
note: the type is part of the `async fn` body because of this `await`
--> $DIR/unresolved_type_param.rs:9:10
|
LL | bar().await;
| ^^^^^^

error[E0698]: type inside `async fn` body must be known in this context
--> $DIR/unresolved_type_param.rs:9:5
|
LL | bar().await;
| ^^^ cannot infer type for type parameter `T` declared on the function `bar`
|
note: the type is part of the `async fn` body because of this `await`
--> $DIR/unresolved_type_param.rs:9:10
|
LL | bar().await;
| ^^^^^^

error: aborting due to 5 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0698`.
2 changes: 2 additions & 0 deletions src/test/ui/generator/issue-57478.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// check-pass

#![feature(negative_impls, generators)]

struct Foo;
Expand Down

0 comments on commit f712df8

Please sign in to comment.