Skip to content

Commit

Permalink
Rollup merge of rust-lang#93751 - eholk:issue-93648-drop-tracking-pro…
Browse files Browse the repository at this point in the history
…jection, r=tmiasko

Drop tracking: track borrows of projections

Previous efforts to ignore partially consumed values meant we were also not considering borrows of a projection. This led to cases where we'd miss borrowed types which MIR expected to be there, leading to ICEs.

This PR also includes the `-Zdrop-tracking` flag from rust-lang#93313. If that PR lands first, I'll rebase to drop the commit from this one.

Fixes rust-lang#93648
  • Loading branch information
JohnTitor authored Feb 9, 2022
2 parents 68fa9b1 + 97b24f3 commit a07eed9
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 19 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,7 @@ fn test_debugging_options_tracking_hash() {
tracked!(debug_info_for_profiling, true);
tracked!(debug_macros, true);
tracked!(dep_info_omit_d_target, true);
tracked!(drop_tracking, true);
tracked!(dual_proc_macros, true);
tracked!(fewer_names, Some(true));
tracked!(force_unstable_if_unmarked, true);
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,8 @@ options! {
dont_buffer_diagnostics: bool = (false, parse_bool, [UNTRACKED],
"emit diagnostics rather than buffering (breaks NLL error downgrading, sorting) \
(default: no)"),
drop_tracking: bool = (false, parse_bool, [TRACKED],
"enables drop tracking in generators (default: no)"),
dual_proc_macros: bool = (false, parse_bool, [TRACKED],
"load proc macros for both target and host, but only link to the target (default: no)"),
dump_dep_graph: bool = (false, parse_bool, [UNTRACKED],
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_typeck/src/check/generator_interior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ use tracing::debug;

mod drop_ranges;

// FIXME(eholk): This flag is here to give a quick way to disable drop tracking in case we find
// unexpected breakages while it's still new. It should be removed before too long. For example,
// see #93161.
const ENABLE_DROP_TRACKING: bool = false;

struct InteriorVisitor<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,
types: FxIndexSet<ty::GeneratorInteriorTypeCause<'tcx>>,
Expand Down Expand Up @@ -82,7 +77,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
yield_data.expr_and_pat_count, self.expr_count, source_span
);

if ENABLE_DROP_TRACKING
if self.fcx.sess().opts.debugging_opts.drop_tracking
&& self
.drop_ranges
.is_dropped_at(hir_id, yield_data.expr_and_pat_count)
Expand Down
24 changes: 14 additions & 10 deletions compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn compute_drop_ranges<'a, 'tcx>(
def_id: DefId,
body: &'tcx Body<'tcx>,
) -> DropRanges {
if super::ENABLE_DROP_TRACKING {
if fcx.sess().opts.debugging_opts.drop_tracking {
let consumed_borrowed_places = find_consumed_and_borrowed(fcx, def_id, body);

let num_exprs = fcx.tcx.region_scope_tree(def_id).body_expr_count(body.id()).unwrap_or(0);
Expand Down Expand Up @@ -116,6 +116,18 @@ impl TrackedValue {
TrackedValue::Variable(hir_id) | TrackedValue::Temporary(hir_id) => *hir_id,
}
}

fn from_place_with_projections_allowed(place_with_id: &PlaceWithHirId<'_>) -> Self {
match place_with_id.place.base {
PlaceBase::Rvalue | PlaceBase::StaticItem => {
TrackedValue::Temporary(place_with_id.hir_id)
}
PlaceBase::Local(hir_id)
| PlaceBase::Upvar(ty::UpvarId { var_path: ty::UpvarPath { hir_id }, .. }) => {
TrackedValue::Variable(hir_id)
}
}
}
}

/// Represents a reason why we might not be able to convert a HirId or Place
Expand All @@ -142,15 +154,7 @@ impl TryFrom<&PlaceWithHirId<'_>> for TrackedValue {
return Err(TrackedValueConversionError::PlaceProjectionsNotSupported);
}

match place_with_id.place.base {
PlaceBase::Rvalue | PlaceBase::StaticItem => {
Ok(TrackedValue::Temporary(place_with_id.hir_id))
}
PlaceBase::Local(hir_id)
| PlaceBase::Upvar(ty::UpvarId { var_path: ty::UpvarPath { hir_id }, .. }) => {
Ok(TrackedValue::Variable(hir_id))
}
}
Ok(TrackedValue::from_place_with_projections_allowed(place_with_id))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
_diag_expr_id: HirId,
_bk: rustc_middle::ty::BorrowKind,
) {
place_with_id
.try_into()
.map_or(false, |tracked_value| self.places.borrowed.insert(tracked_value));
self.places
.borrowed
.insert(TrackedValue::from_place_with_projections_allowed(place_with_id));
}

fn mutate(
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/async-await/issue-93648.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// edition:2021
// build-pass
// compile-flags: -Zdrop-tracking

fn main() {
let _ = async {
let mut s = (String::new(),);
s.0.push_str("abc");
std::mem::drop(s);
async {}.await;
};
}

0 comments on commit a07eed9

Please sign in to comment.