Skip to content

Commit 60f5cad

Browse files
nikomatsakiseholk
andcommitted
try to fix issue 57017, but not quite there yet
Co-authored-by: Eric Holk <eric@theincredibleholk.org>
1 parent d137c3a commit 60f5cad

File tree

5 files changed

+70
-9
lines changed

5 files changed

+70
-9
lines changed

compiler/rustc_typeck/src/check/generator_interior.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_hir::def_id::DefId;
1313
use rustc_hir::hir_id::HirIdSet;
1414
use rustc_hir::intravisit::{self, Visitor};
1515
use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind};
16-
use rustc_middle::middle::region::{self, YieldData};
16+
use rustc_middle::middle::region::{self, Scope, ScopeData, YieldData};
1717
use rustc_middle::ty::{self, Ty, TyCtxt};
1818
use rustc_span::symbol::sym;
1919
use rustc_span::Span;
@@ -369,7 +369,17 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
369369

370370
self.expr_count += 1;
371371

372-
let scope = self.region_scope_tree.temporary_scope(expr.hir_id.local_id);
372+
debug!("is_borrowed_temporary: {:?}", self.drop_ranges.is_borrowed_temporary(expr));
373+
374+
let scope = if self.drop_ranges.is_borrowed_temporary(expr) {
375+
self.region_scope_tree.temporary_scope(expr.hir_id.local_id)
376+
} else {
377+
debug!("parent_node: {:?}", self.fcx.tcx.hir().find_parent_node(expr.hir_id));
378+
match self.fcx.tcx.hir().find_parent_node(expr.hir_id) {
379+
Some(parent) => Some(Scope { id: parent.local_id, data: ScopeData::Node }),
380+
None => self.region_scope_tree.temporary_scope(expr.hir_id.local_id),
381+
}
382+
};
373383

374384
// If there are adjustments, then record the final type --
375385
// this is the actual value that is being produced.

compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use crate::check::FnCtxt;
1818
use hir::def_id::DefId;
1919
use hir::{Body, HirId, HirIdMap, Node};
2020
use rustc_data_structures::fx::FxHashMap;
21+
use rustc_data_structures::stable_set::FxHashSet;
2122
use rustc_hir as hir;
2223
use rustc_index::bit_set::BitSet;
2324
use rustc_index::vec::IndexVec;
@@ -41,7 +42,7 @@ pub fn compute_drop_ranges<'a, 'tcx>(
4142
let consumed_borrowed_places = find_consumed_and_borrowed(fcx, def_id, body);
4243

4344
let num_exprs = fcx.tcx.region_scope_tree(def_id).body_expr_count(body.id()).unwrap_or(0);
44-
let mut drop_ranges = build_control_flow_graph(
45+
let (mut drop_ranges, borrowed_temporaries) = build_control_flow_graph(
4546
fcx.tcx.hir(),
4647
fcx.tcx,
4748
&fcx.typeck_results.borrow(),
@@ -52,11 +53,20 @@ pub fn compute_drop_ranges<'a, 'tcx>(
5253

5354
drop_ranges.propagate_to_fixpoint();
5455

55-
DropRanges { tracked_value_map: drop_ranges.tracked_value_map, nodes: drop_ranges.nodes }
56+
debug!("borrowed_temporaries = {borrowed_temporaries:?}");
57+
DropRanges {
58+
tracked_value_map: drop_ranges.tracked_value_map,
59+
nodes: drop_ranges.nodes,
60+
borrowed_temporaries: Some(borrowed_temporaries),
61+
}
5662
} else {
5763
// If drop range tracking is not enabled, skip all the analysis and produce an
5864
// empty set of DropRanges.
59-
DropRanges { tracked_value_map: FxHashMap::default(), nodes: IndexVec::new() }
65+
DropRanges {
66+
tracked_value_map: FxHashMap::default(),
67+
nodes: IndexVec::new(),
68+
borrowed_temporaries: None,
69+
}
6070
}
6171
}
6272

@@ -161,6 +171,7 @@ impl TryFrom<&PlaceWithHirId<'_>> for TrackedValue {
161171
pub struct DropRanges {
162172
tracked_value_map: FxHashMap<TrackedValue, TrackedValueIndex>,
163173
nodes: IndexVec<PostOrderId, NodeInfo>,
174+
borrowed_temporaries: Option<FxHashSet<HirId>>,
164175
}
165176

166177
impl DropRanges {
@@ -174,6 +185,10 @@ impl DropRanges {
174185
})
175186
}
176187

188+
pub fn is_borrowed_temporary(&self, expr: &hir::Expr<'_>) -> bool {
189+
if let Some(b) = &self.borrowed_temporaries { b.contains(&expr.hir_id) } else { true }
190+
}
191+
177192
/// Returns a reference to the NodeInfo for a node, panicking if it does not exist
178193
fn expect_node(&self, id: PostOrderId) -> &NodeInfo {
179194
&self.nodes[id]

compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use hir::{
66
intravisit::{self, Visitor},
77
Body, Expr, ExprKind, Guard, HirId, LoopIdError,
88
};
9-
use rustc_data_structures::fx::FxHashMap;
9+
use rustc_data_structures::{fx::FxHashMap, stable_set::FxHashSet};
1010
use rustc_hir as hir;
1111
use rustc_index::vec::IndexVec;
1212
use rustc_middle::{
@@ -27,14 +27,14 @@ pub(super) fn build_control_flow_graph<'tcx>(
2727
consumed_borrowed_places: ConsumedAndBorrowedPlaces,
2828
body: &'tcx Body<'tcx>,
2929
num_exprs: usize,
30-
) -> DropRangesBuilder {
30+
) -> (DropRangesBuilder, FxHashSet<HirId>) {
3131
let mut drop_range_visitor =
3232
DropRangeVisitor::new(hir, tcx, typeck_results, consumed_borrowed_places, num_exprs);
3333
intravisit::walk_body(&mut drop_range_visitor, body);
3434

3535
drop_range_visitor.drop_ranges.process_deferred_edges();
3636

37-
drop_range_visitor.drop_ranges
37+
(drop_range_visitor.drop_ranges, drop_range_visitor.places.borrowed_temporaries)
3838
}
3939

4040
/// This struct is used to gather the information for `DropRanges` to determine the regions of the

compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::{
66
use hir::{def_id::DefId, Body, HirId, HirIdMap};
77
use rustc_data_structures::stable_set::FxHashSet;
88
use rustc_hir as hir;
9+
use rustc_middle::hir::place::PlaceBase;
910
use rustc_middle::ty::{ParamEnv, TyCtxt};
1011

1112
pub(super) fn find_consumed_and_borrowed<'a, 'tcx>(
@@ -27,8 +28,12 @@ pub(super) struct ConsumedAndBorrowedPlaces {
2728
/// Note that this set excludes "partial drops" -- for example, a statement like `drop(x.y)` is
2829
/// not considered a drop of `x`, although it would be a drop of `x.y`.
2930
pub(super) consumed: HirIdMap<FxHashSet<TrackedValue>>,
31+
3032
/// A set of hir-ids of values or variables that are borrowed at some point within the body.
3133
pub(super) borrowed: FxHashSet<TrackedValue>,
34+
35+
/// A set of hir-ids of values or variables that are borrowed at some point within the body.
36+
pub(super) borrowed_temporaries: FxHashSet<HirId>,
3237
}
3338

3439
/// Works with ExprUseVisitor to find interesting values for the drop range analysis.
@@ -49,6 +54,7 @@ impl<'tcx> ExprUseDelegate<'tcx> {
4954
places: ConsumedAndBorrowedPlaces {
5055
consumed: <_>::default(),
5156
borrowed: <_>::default(),
57+
borrowed_temporaries: <_>::default(),
5258
},
5359
}
5460
}
@@ -98,10 +104,19 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
98104
diag_expr_id: HirId,
99105
_bk: rustc_middle::ty::BorrowKind,
100106
) {
101-
debug!("borrow {:?}; diag_expr_id={:?}", place_with_id, diag_expr_id);
107+
debug!("borrow: place_with_id = {place_with_id:?}, diag_expr_id={diag_expr_id:?}");
108+
102109
self.places
103110
.borrowed
104111
.insert(TrackedValue::from_place_with_projections_allowed(place_with_id));
112+
113+
// XXX -- we need to distinguish "consuming a copy" from other borrows
114+
//
115+
// XXX -- we need to distinguish `&*E` where `E: &T` which is not creating a temporary
116+
// even though the place-base E is an rvalue
117+
if let PlaceBase::Rvalue = place_with_id.place.base {
118+
self.places.borrowed_temporaries.insert(place_with_id.hir_id);
119+
}
105120
}
106121

107122
fn mutate(

foo.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// check-pass
2+
#![feature(generators, negative_impls)]
3+
4+
struct Client;
5+
6+
impl !Sync for Client {}
7+
8+
fn status(_client_status: &Client) -> i16 {
9+
200
10+
}
11+
12+
fn assert_send<T: Send>(_thing: T) {}
13+
14+
// This is the same bug as issue 57017, but using yield instead of await
15+
fn main() {
16+
let client = Client;
17+
let g = move || match status(&client) {
18+
_status => yield,
19+
};
20+
assert_send(g);
21+
}

0 commit comments

Comments
 (0)