Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC-2229: Implement Precise Capture Analysis #78801

Merged
merged 16 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,9 @@ declare_features! (
/// Enables `#[cfg(panic = "...")]` config key.
(active, cfg_panic, "1.49.0", Some(77443), None),

/// Allows capturing disjoint fields in a closure/generator (RFC 2229).
(active, capture_disjoint_fields, "1.49.0", Some(53488), None),

// -------------------------------------------------------------------------
// feature-group-end: actual feature gates
// -------------------------------------------------------------------------
Expand All @@ -639,6 +642,7 @@ pub const INCOMPLETE_FEATURES: &[Symbol] = &[
sym::inline_const,
sym::repr128,
sym::unsized_locals,
sym::capture_disjoint_fields,
];

/// Some features are not allowed to be used together at the same time, if
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
// ==========================================================================

rustc_attr!(TEST, rustc_outlives, Normal, template!(Word)),
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word)),
rustc_attr!(TEST, rustc_variance, Normal, template!(Word)),
rustc_attr!(TEST, rustc_layout, Normal, template!(List: "field1, field2, ...")),
rustc_attr!(TEST, rustc_regions, Normal, template!(Word)),
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,10 @@ pub struct TypeckResults<'tcx> {
/// entire variable.
pub closure_captures: ty::UpvarListMap,

/// Tracks the minimum captures required for a closure;
/// see `MinCaptureInformationMap` for more details.
pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>,

/// Stores the type, expression, span and optional scope span of all types
/// that are live across the yield of this generator (if a generator).
pub generator_interior_types: Vec<GeneratorInteriorTypeCause<'tcx>>,
Expand Down Expand Up @@ -442,6 +446,7 @@ impl<'tcx> TypeckResults<'tcx> {
tainted_by_errors: None,
concrete_opaque_types: Default::default(),
closure_captures: Default::default(),
closure_min_captures: Default::default(),
generator_interior_types: Default::default(),
}
}
Expand Down Expand Up @@ -676,6 +681,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
tainted_by_errors,
ref concrete_opaque_types,
ref closure_captures,
ref closure_min_captures,
ref generator_interior_types,
} = *self;

Expand Down Expand Up @@ -709,6 +715,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
tainted_by_errors.hash_stable(hcx, hasher);
concrete_opaque_types.hash_stable(hcx, hasher);
closure_captures.hash_stable(hcx, hasher);
closure_min_captures.hash_stable(hcx, hasher);
generator_interior_types.hash_stable(hcx, hasher);
})
}
Expand Down
57 changes: 57 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub use self::IntVarValue::*;
pub use self::Variance::*;

use crate::hir::exports::ExportMap;
use crate::hir::place::Place as HirPlace;
use crate::ich::StableHashingContext;
use crate::middle::cstore::CrateStoreDyn;
use crate::middle::resolve_lifetime::ObjectLifetimeDefault;
Expand Down Expand Up @@ -674,6 +675,12 @@ pub struct UpvarId {
pub closure_expr_id: LocalDefId,
}

impl UpvarId {
pub fn new(var_hir_id: hir::HirId, closure_def_id: LocalDefId) -> UpvarId {
UpvarId { var_path: UpvarPath { hir_id: var_hir_id }, closure_expr_id: closure_def_id }
}
}

#[derive(Clone, PartialEq, Debug, TyEncodable, TyDecodable, Copy, HashStable)]
pub enum BorrowKind {
/// Data must be immutable and is aliasable.
Expand Down Expand Up @@ -756,6 +763,56 @@ pub struct UpvarBorrow<'tcx> {
pub region: ty::Region<'tcx>,
}

/// Given the closure DefId this map provides a map of root variables to minimum
/// set of `CapturedPlace`s that need to be tracked to support all captures of that closure.
pub type MinCaptureInformationMap<'tcx> = FxHashMap<DefId, RootVariableMinCaptureList<'tcx>>;

/// Part of `MinCaptureInformationMap`; Maps a root variable to the list of `CapturedPlace`.
/// Used to track the minimum set of `Place`s that need to be captured to support all
/// Places captured by the closure starting at a given root variable.
///
/// This provides a convenient and quick way of checking if a variable being used within
/// a closure is a capture of a local variable.
pub type RootVariableMinCaptureList<'tcx> = FxIndexMap<hir::HirId, MinCaptureList<'tcx>>;

/// Part of `MinCaptureInformationMap`; List of `CapturePlace`s.
pub type MinCaptureList<'tcx> = Vec<CapturedPlace<'tcx>>;

/// A `Place` and the corresponding `CaptureInfo`.
#[derive(PartialEq, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
pub struct CapturedPlace<'tcx> {
pub place: HirPlace<'tcx>,
pub info: CaptureInfo<'tcx>,
}

/// Part of `MinCaptureInformationMap`; describes the capture kind (&, &mut, move)
/// for a particular capture as well as identifying the part of the source code
/// that triggered this capture to occur.
#[derive(PartialEq, Clone, Debug, Copy, TyEncodable, TyDecodable, HashStable)]
arora-aman marked this conversation as resolved.
Show resolved Hide resolved
pub struct CaptureInfo<'tcx> {
/// Expr Id pointing to use that resulted in selecting the current capture kind
///
/// If the user doesn't enable feature `capture_disjoint_fields` (RFC 2229) then, it is
/// possible that we don't see the use of a particular place resulting in expr_id being
/// None. In such case we fallback on uvpars_mentioned for span.
///
/// Eg:
/// ```rust,no_run
/// let x = 5;
///
/// let c = || {
/// let _ = x
/// };
/// ```
///
/// In this example, if `capture_disjoint_fields` is **not** set, then x will be captured,
/// but we won't see it being used during capture analysis, since it's essentially a discard.
pub expr_id: Option<hir::HirId>,

/// Capture mode that was selected
pub capture_kind: UpvarCapture<'tcx>,
}

pub type UpvarListMap = FxHashMap<DefId, FxIndexMap<hir::HirId, UpvarId>>;
pub type UpvarCaptureMap<'tcx> = FxHashMap<UpvarId, UpvarCapture<'tcx>>;

Expand Down
27 changes: 14 additions & 13 deletions compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,16 +383,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
self.describe_field_from_ty(&ty, field, variant_index)
}
ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
// `tcx.upvars_mentioned(def_id)` returns an `Option`, which is `None` in case
// the closure comes from another crate. But in that case we wouldn't
// be borrowck'ing it, so we can just unwrap:
let (&var_id, _) = self
.infcx
.tcx
.upvars_mentioned(def_id)
.unwrap()
.get_index(field.index())
.unwrap();
// We won't be borrowck'ing here if the closure came from another crate,
// so it's safe to call `expect_local`.
//
// We know the field exists so it's safe to call operator[] and `unwrap` here.
let (&var_id, _) =
self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id]
.get_index(field.index())
.unwrap();

self.infcx.tcx.hir().name(var_id).to_string()
}
Expand Down Expand Up @@ -967,9 +965,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let expr = &self.infcx.tcx.hir().expect_expr(hir_id).kind;
debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr);
if let hir::ExprKind::Closure(.., body_id, args_span, _) = expr {
for ((upvar_hir_id, upvar), place) in
self.infcx.tcx.upvars_mentioned(def_id)?.iter().zip(places)
for (upvar_hir_id, place) in
self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id]
.keys()
.zip(places)
{
let span = self.infcx.tcx.upvars_mentioned(local_did)?[upvar_hir_id].span;
match place {
Operand::Copy(place) | Operand::Move(place)
if target_place == place.as_ref() =>
Expand All @@ -991,7 +992,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let usage_span =
match self.infcx.tcx.typeck(local_did).upvar_capture(upvar_id) {
ty::UpvarCapture::ByValue(Some(span)) => span,
_ => upvar.span,
_ => span,
};
return Some((*args_span, generator_kind, usage_span));
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,9 @@ fn make_mirror_unadjusted<'a, 'tcx>(
}
};
let upvars = cx
.tcx
.upvars_mentioned(def_id)
.typeck_results()
.closure_captures
.get(&def_id)
.iter()
.flat_map(|upvars| upvars.iter())
.zip(substs.upvar_tys())
Expand Down
69 changes: 49 additions & 20 deletions compiler/rustc_passes/src/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,19 +317,20 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
// swap in a new set of IR maps for this body
let mut maps = IrMaps::new(self.tcx);
let hir_id = maps.tcx.hir().body_owner(body.id());
let def_id = maps.tcx.hir().local_def_id(hir_id);
let local_def_id = maps.tcx.hir().local_def_id(hir_id);
let def_id = local_def_id.to_def_id();

// Don't run unused pass for #[derive()]
if let Some(parent) = self.tcx.parent(def_id.to_def_id()) {
if let Some(parent) = self.tcx.parent(def_id) {
if let DefKind::Impl = self.tcx.def_kind(parent.expect_local()) {
if self.tcx.has_attr(parent, sym::automatically_derived) {
return;
}
}
}

if let Some(upvars) = maps.tcx.upvars_mentioned(def_id) {
for (&var_hir_id, _upvar) in upvars {
if let Some(captures) = maps.tcx.typeck(local_def_id).closure_captures.get(&def_id) {
for &var_hir_id in captures.keys() {
let var_name = maps.tcx.hir().name(var_hir_id);
maps.add_variable(Upvar(var_hir_id, var_name));
}
Expand All @@ -340,7 +341,7 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
intravisit::walk_body(&mut maps, body);

// compute liveness
let mut lsets = Liveness::new(&mut maps, def_id);
let mut lsets = Liveness::new(&mut maps, local_def_id);
let entry_ln = lsets.compute(&body, hir_id);
lsets.log_liveness(entry_ln, body.id().hir_id);

Expand Down Expand Up @@ -397,10 +398,18 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
// construction site.
let mut call_caps = Vec::new();
let closure_def_id = self.tcx.hir().local_def_id(expr.hir_id);
if let Some(upvars) = self.tcx.upvars_mentioned(closure_def_id) {
call_caps.extend(upvars.iter().map(|(&var_id, upvar)| {
if let Some(captures) = self
.tcx
.typeck(closure_def_id)
.closure_captures
.get(&closure_def_id.to_def_id())
{
// If closure captures is Some, upvars_mentioned must also be Some
let upvars = self.tcx.upvars_mentioned(closure_def_id).unwrap();
call_caps.extend(captures.keys().map(|var_id| {
let upvar = upvars[var_id];
let upvar_ln = self.add_live_node(UpvarNode(upvar.span));
CaptureInfo { ln: upvar_ln, var_hid: var_id }
CaptureInfo { ln: upvar_ln, var_hid: *var_id }
}));
}
self.set_captures(expr.hir_id, call_caps);
Expand Down Expand Up @@ -564,6 +573,7 @@ struct Liveness<'a, 'tcx> {
typeck_results: &'a ty::TypeckResults<'tcx>,
param_env: ty::ParamEnv<'tcx>,
upvars: Option<&'tcx FxIndexMap<hir::HirId, hir::Upvar>>,
closure_captures: Option<&'tcx FxIndexMap<hir::HirId, ty::UpvarId>>,
successors: IndexVec<LiveNode, Option<LiveNode>>,
rwu_table: RWUTable,

Expand All @@ -587,6 +597,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
let typeck_results = ir.tcx.typeck(body_owner);
let param_env = ir.tcx.param_env(body_owner);
let upvars = ir.tcx.upvars_mentioned(body_owner);
let closure_captures = typeck_results.closure_captures.get(&body_owner.to_def_id());

let closure_ln = ir.add_live_node(ClosureNode);
let exit_ln = ir.add_live_node(ExitNode);
Expand All @@ -600,6 +611,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
typeck_results,
param_env,
upvars,
closure_captures,
successors: IndexVec::from_elem_n(None, num_live_nodes),
rwu_table: RWUTable::new(num_live_nodes * num_vars),
closure_ln,
Expand Down Expand Up @@ -850,14 +862,13 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
// if they are live on the entry to the closure, since only the closure
// itself can access them on subsequent calls.

if let Some(upvars) = self.upvars {
if let Some(closure_captures) = self.closure_captures {
// Mark upvars captured by reference as used after closure exits.
for (&var_hir_id, upvar) in upvars.iter().rev() {
let upvar_id = ty::UpvarId {
var_path: ty::UpvarPath { hir_id: var_hir_id },
closure_expr_id: self.body_owner,
};
match self.typeck_results.upvar_capture(upvar_id) {
// Since closure_captures is Some, upvars must exists too.
let upvars = self.upvars.unwrap();
for (&var_hir_id, upvar_id) in closure_captures {
let upvar = upvars[&var_hir_id];
match self.typeck_results.upvar_capture(*upvar_id) {
ty::UpvarCapture::ByRef(_) => {
let var = self.variable(var_hir_id, upvar.span);
self.acc(self.exit_ln, var, ACC_READ | ACC_USE);
Expand All @@ -869,7 +880,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {

let succ = self.propagate_through_expr(&body.value, self.exit_ln);

if self.upvars.is_none() {
if self.closure_captures.is_none() {
// Either not a closure, or closure without any captured variables.
// No need to determine liveness of captured variables, since there
// are none.
Expand Down Expand Up @@ -1341,7 +1352,21 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
acc: u32,
) -> LiveNode {
match path.res {
Res::Local(hid) => self.access_var(hir_id, hid, succ, acc, path.span),
Res::Local(hid) => {
let in_upvars = self.upvars.map_or(false, |u| u.contains_key(&hid));
let in_captures = self.closure_captures.map_or(false, |c| c.contains_key(&hid));

match (in_upvars, in_captures) {
(false, _) | (true, true) => self.access_var(hir_id, hid, succ, acc, path.span),
(true, false) => {
// This case is possible when with RFC-2229, a wild pattern
// is used within a closure.
// eg: `let _ = x`. The closure doesn't capture x here,
// even though it's mentioned in the closure.
succ
}
}
}
_ => succ,
}
}
Expand Down Expand Up @@ -1531,11 +1556,15 @@ impl<'tcx> Liveness<'_, 'tcx> {
}

fn warn_about_unused_upvars(&self, entry_ln: LiveNode) {
let upvars = match self.upvars {
let closure_captures = match self.closure_captures {
None => return,
Some(upvars) => upvars,
Some(closure_captures) => closure_captures,
};
for (&var_hir_id, upvar) in upvars.iter() {

// If closure_captures is Some(), upvars must be Some() too.
let upvars = self.upvars.unwrap();
for &var_hir_id in closure_captures.keys() {
let upvar = upvars[&var_hir_id];
let var = self.variable(var_hir_id, upvar.span);
let upvar_id = ty::UpvarId {
var_path: ty::UpvarPath { hir_id: var_hir_id },
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ symbols! {
call_mut,
call_once,
caller_location,
capture_disjoint_fields,
cdylib,
ceilf32,
ceilf64,
Expand Down Expand Up @@ -909,6 +910,7 @@ symbols! {
rustc_args_required_const,
rustc_attrs,
rustc_builtin_macro,
rustc_capture_analysis,
rustc_clean,
rustc_const_stable,
rustc_const_unstable,
Expand Down
Loading