Skip to content

Commit 0c0e8c4

Browse files
authored
Rollup merge of rust-lang#78801 - sexxi-goose:min_capture, r=nikomatsakis
RFC-2229: Implement Precise Capture Analysis ### This PR introduces - Feature gate for RFC-2229 (incomplete) `capture_disjoint_field` - Rustc Attribute to print out the capture analysis `rustc_capture_analysis` - Precise capture analysis ### Description of the analysis 1. If the feature gate is not set then all variables that are not local to the closure will be added to the list of captures. (This is for backcompat) 2. The rest of the analysis is based entirely on how the captured `Place`s are used within the closure. Precise information (i.e. projections) about the `Place` is maintained throughout. 3. To reduce the amount of information we need to keep track of, we do a minimization step. In this step, we determine a list such that no Place within this list represents an ancestor path to another entry in the list. Check rust-lang/project-rfc-2229#9 for more detailed examples. 4. To keep the compiler functional as before we implement a Bridge between the results of this new analysis to existing data structures used for closure captures. Note the new capture analysis results are only part of MaybeTypeckTables that is the information is only available during typeck-ing. ### Known issues - Statements like `let _ = x` will make the compiler ICE when used within a closure with the feature enabled. More generally speaking the issue is caused by `let` statements that create no bindings and are init'ed using a Place expression. ### Testing We removed the code that would handle the case where the feature gate is not set, to enable the feature as default and did a bors try and perf run. More information here: rust-lang#78762 ### Thanks This has been slowly in the works for a while now. I want to call out `@Azhng` `@ChrisPardy` `@null-sleep` `@jenniferwills` `@logmosier` `@roxelo` for working on this and the previous PRs that led up to this, `@nikomatsakis` for guiding us. Closes rust-lang/project-rfc-2229#7 Closes rust-lang/project-rfc-2229#9 Closes rust-lang/project-rfc-2229#6 Closes rust-lang/project-rfc-2229#19 r? `@nikomatsakis`
2 parents 6272915 + c50e57f commit 0c0e8c4

37 files changed

+2445
-190
lines changed

compiler/rustc_feature/src/active.rs

+4
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,9 @@ declare_features! (
616616
/// Enables `#[cfg(panic = "...")]` config key.
617617
(active, cfg_panic, "1.49.0", Some(77443), None),
618618

619+
/// Allows capturing disjoint fields in a closure/generator (RFC 2229).
620+
(active, capture_disjoint_fields, "1.49.0", Some(53488), None),
621+
619622
// -------------------------------------------------------------------------
620623
// feature-group-end: actual feature gates
621624
// -------------------------------------------------------------------------
@@ -639,6 +642,7 @@ pub const INCOMPLETE_FEATURES: &[Symbol] = &[
639642
sym::inline_const,
640643
sym::repr128,
641644
sym::unsized_locals,
645+
sym::capture_disjoint_fields,
642646
];
643647

644648
/// Some features are not allowed to be used together at the same time, if

compiler/rustc_feature/src/builtin_attrs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
547547
// ==========================================================================
548548

549549
rustc_attr!(TEST, rustc_outlives, Normal, template!(Word)),
550+
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word)),
550551
rustc_attr!(TEST, rustc_variance, Normal, template!(Word)),
551552
rustc_attr!(TEST, rustc_layout, Normal, template!(List: "field1, field2, ...")),
552553
rustc_attr!(TEST, rustc_regions, Normal, template!(Word)),

compiler/rustc_middle/src/ty/context.rs

+7
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,10 @@ pub struct TypeckResults<'tcx> {
415415
/// entire variable.
416416
pub closure_captures: ty::UpvarListMap,
417417

418+
/// Tracks the minimum captures required for a closure;
419+
/// see `MinCaptureInformationMap` for more details.
420+
pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>,
421+
418422
/// Stores the type, expression, span and optional scope span of all types
419423
/// that are live across the yield of this generator (if a generator).
420424
pub generator_interior_types: Vec<GeneratorInteriorTypeCause<'tcx>>,
@@ -442,6 +446,7 @@ impl<'tcx> TypeckResults<'tcx> {
442446
tainted_by_errors: None,
443447
concrete_opaque_types: Default::default(),
444448
closure_captures: Default::default(),
449+
closure_min_captures: Default::default(),
445450
generator_interior_types: Default::default(),
446451
}
447452
}
@@ -676,6 +681,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
676681
tainted_by_errors,
677682
ref concrete_opaque_types,
678683
ref closure_captures,
684+
ref closure_min_captures,
679685
ref generator_interior_types,
680686
} = *self;
681687

@@ -709,6 +715,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
709715
tainted_by_errors.hash_stable(hcx, hasher);
710716
concrete_opaque_types.hash_stable(hcx, hasher);
711717
closure_captures.hash_stable(hcx, hasher);
718+
closure_min_captures.hash_stable(hcx, hasher);
712719
generator_interior_types.hash_stable(hcx, hasher);
713720
})
714721
}

compiler/rustc_middle/src/ty/mod.rs

+57
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ pub use self::IntVarValue::*;
66
pub use self::Variance::*;
77

88
use crate::hir::exports::ExportMap;
9+
use crate::hir::place::Place as HirPlace;
910
use crate::ich::StableHashingContext;
1011
use crate::middle::cstore::CrateStoreDyn;
1112
use crate::middle::resolve_lifetime::ObjectLifetimeDefault;
@@ -674,6 +675,12 @@ pub struct UpvarId {
674675
pub closure_expr_id: LocalDefId,
675676
}
676677

678+
impl UpvarId {
679+
pub fn new(var_hir_id: hir::HirId, closure_def_id: LocalDefId) -> UpvarId {
680+
UpvarId { var_path: UpvarPath { hir_id: var_hir_id }, closure_expr_id: closure_def_id }
681+
}
682+
}
683+
677684
#[derive(Clone, PartialEq, Debug, TyEncodable, TyDecodable, Copy, HashStable)]
678685
pub enum BorrowKind {
679686
/// Data must be immutable and is aliasable.
@@ -756,6 +763,56 @@ pub struct UpvarBorrow<'tcx> {
756763
pub region: ty::Region<'tcx>,
757764
}
758765

766+
/// Given the closure DefId this map provides a map of root variables to minimum
767+
/// set of `CapturedPlace`s that need to be tracked to support all captures of that closure.
768+
pub type MinCaptureInformationMap<'tcx> = FxHashMap<DefId, RootVariableMinCaptureList<'tcx>>;
769+
770+
/// Part of `MinCaptureInformationMap`; Maps a root variable to the list of `CapturedPlace`.
771+
/// Used to track the minimum set of `Place`s that need to be captured to support all
772+
/// Places captured by the closure starting at a given root variable.
773+
///
774+
/// This provides a convenient and quick way of checking if a variable being used within
775+
/// a closure is a capture of a local variable.
776+
pub type RootVariableMinCaptureList<'tcx> = FxIndexMap<hir::HirId, MinCaptureList<'tcx>>;
777+
778+
/// Part of `MinCaptureInformationMap`; List of `CapturePlace`s.
779+
pub type MinCaptureList<'tcx> = Vec<CapturedPlace<'tcx>>;
780+
781+
/// A `Place` and the corresponding `CaptureInfo`.
782+
#[derive(PartialEq, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
783+
pub struct CapturedPlace<'tcx> {
784+
pub place: HirPlace<'tcx>,
785+
pub info: CaptureInfo<'tcx>,
786+
}
787+
788+
/// Part of `MinCaptureInformationMap`; describes the capture kind (&, &mut, move)
789+
/// for a particular capture as well as identifying the part of the source code
790+
/// that triggered this capture to occur.
791+
#[derive(PartialEq, Clone, Debug, Copy, TyEncodable, TyDecodable, HashStable)]
792+
pub struct CaptureInfo<'tcx> {
793+
/// Expr Id pointing to use that resulted in selecting the current capture kind
794+
///
795+
/// If the user doesn't enable feature `capture_disjoint_fields` (RFC 2229) then, it is
796+
/// possible that we don't see the use of a particular place resulting in expr_id being
797+
/// None. In such case we fallback on uvpars_mentioned for span.
798+
///
799+
/// Eg:
800+
/// ```rust
801+
/// let x = ...;
802+
///
803+
/// let c = || {
804+
/// let _ = x
805+
/// }
806+
/// ```
807+
///
808+
/// In this example, if `capture_disjoint_fields` is **not** set, then x will be captured,
809+
/// but we won't see it being used during capture analysis, since it's essentially a discard.
810+
pub expr_id: Option<hir::HirId>,
811+
812+
/// Capture mode that was selected
813+
pub capture_kind: UpvarCapture<'tcx>,
814+
}
815+
759816
pub type UpvarListMap = FxHashMap<DefId, FxIndexMap<hir::HirId, UpvarId>>;
760817
pub type UpvarCaptureMap<'tcx> = FxHashMap<UpvarId, UpvarCapture<'tcx>>;
761818

compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs

+14-13
Original file line numberDiff line numberDiff line change
@@ -383,16 +383,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
383383
self.describe_field_from_ty(&ty, field, variant_index)
384384
}
385385
ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
386-
// `tcx.upvars_mentioned(def_id)` returns an `Option`, which is `None` in case
387-
// the closure comes from another crate. But in that case we wouldn't
388-
// be borrowck'ing it, so we can just unwrap:
389-
let (&var_id, _) = self
390-
.infcx
391-
.tcx
392-
.upvars_mentioned(def_id)
393-
.unwrap()
394-
.get_index(field.index())
395-
.unwrap();
386+
// We won't be borrowck'ing here if the closure came from another crate,
387+
// so it's safe to call `expect_local`.
388+
//
389+
// We know the field exists so it's safe to call operator[] and `unwrap` here.
390+
let (&var_id, _) =
391+
self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id]
392+
.get_index(field.index())
393+
.unwrap();
396394

397395
self.infcx.tcx.hir().name(var_id).to_string()
398396
}
@@ -967,9 +965,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
967965
let expr = &self.infcx.tcx.hir().expect_expr(hir_id).kind;
968966
debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr);
969967
if let hir::ExprKind::Closure(.., body_id, args_span, _) = expr {
970-
for ((upvar_hir_id, upvar), place) in
971-
self.infcx.tcx.upvars_mentioned(def_id)?.iter().zip(places)
968+
for (upvar_hir_id, place) in
969+
self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id]
970+
.keys()
971+
.zip(places)
972972
{
973+
let span = self.infcx.tcx.upvars_mentioned(local_did)?[upvar_hir_id].span;
973974
match place {
974975
Operand::Copy(place) | Operand::Move(place)
975976
if target_place == place.as_ref() =>
@@ -991,7 +992,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
991992
let usage_span =
992993
match self.infcx.tcx.typeck(local_did).upvar_capture(upvar_id) {
993994
ty::UpvarCapture::ByValue(Some(span)) => span,
994-
_ => upvar.span,
995+
_ => span,
995996
};
996997
return Some((*args_span, generator_kind, usage_span));
997998
}

compiler/rustc_mir_build/src/thir/cx/expr.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,9 @@ fn make_mirror_unadjusted<'a, 'tcx>(
387387
}
388388
};
389389
let upvars = cx
390-
.tcx
391-
.upvars_mentioned(def_id)
390+
.typeck_results()
391+
.closure_captures
392+
.get(&def_id)
392393
.iter()
393394
.flat_map(|upvars| upvars.iter())
394395
.zip(substs.upvar_tys())

compiler/rustc_passes/src/liveness.rs

+49-20
Original file line numberDiff line numberDiff line change
@@ -317,19 +317,20 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
317317
// swap in a new set of IR maps for this body
318318
let mut maps = IrMaps::new(self.tcx);
319319
let hir_id = maps.tcx.hir().body_owner(body.id());
320-
let def_id = maps.tcx.hir().local_def_id(hir_id);
320+
let local_def_id = maps.tcx.hir().local_def_id(hir_id);
321+
let def_id = local_def_id.to_def_id();
321322

322323
// Don't run unused pass for #[derive()]
323-
if let Some(parent) = self.tcx.parent(def_id.to_def_id()) {
324+
if let Some(parent) = self.tcx.parent(def_id) {
324325
if let DefKind::Impl = self.tcx.def_kind(parent.expect_local()) {
325326
if self.tcx.has_attr(parent, sym::automatically_derived) {
326327
return;
327328
}
328329
}
329330
}
330331

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

342343
// compute liveness
343-
let mut lsets = Liveness::new(&mut maps, def_id);
344+
let mut lsets = Liveness::new(&mut maps, local_def_id);
344345
let entry_ln = lsets.compute(&body, hir_id);
345346
lsets.log_liveness(entry_ln, body.id().hir_id);
346347

@@ -397,10 +398,18 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
397398
// construction site.
398399
let mut call_caps = Vec::new();
399400
let closure_def_id = self.tcx.hir().local_def_id(expr.hir_id);
400-
if let Some(upvars) = self.tcx.upvars_mentioned(closure_def_id) {
401-
call_caps.extend(upvars.iter().map(|(&var_id, upvar)| {
401+
if let Some(captures) = self
402+
.tcx
403+
.typeck(closure_def_id)
404+
.closure_captures
405+
.get(&closure_def_id.to_def_id())
406+
{
407+
// If closure captures is Some, upvars_mentioned must also be Some
408+
let upvars = self.tcx.upvars_mentioned(closure_def_id).unwrap();
409+
call_caps.extend(captures.keys().map(|var_id| {
410+
let upvar = upvars[var_id];
402411
let upvar_ln = self.add_live_node(UpvarNode(upvar.span));
403-
CaptureInfo { ln: upvar_ln, var_hid: var_id }
412+
CaptureInfo { ln: upvar_ln, var_hid: *var_id }
404413
}));
405414
}
406415
self.set_captures(expr.hir_id, call_caps);
@@ -564,6 +573,7 @@ struct Liveness<'a, 'tcx> {
564573
typeck_results: &'a ty::TypeckResults<'tcx>,
565574
param_env: ty::ParamEnv<'tcx>,
566575
upvars: Option<&'tcx FxIndexMap<hir::HirId, hir::Upvar>>,
576+
closure_captures: Option<&'tcx FxIndexMap<hir::HirId, ty::UpvarId>>,
567577
successors: IndexVec<LiveNode, Option<LiveNode>>,
568578
rwu_table: RWUTable,
569579

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

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

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

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

872-
if self.upvars.is_none() {
883+
if self.closure_captures.is_none() {
873884
// Either not a closure, or closure without any captured variables.
874885
// No need to determine liveness of captured variables, since there
875886
// are none.
@@ -1341,7 +1352,21 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
13411352
acc: u32,
13421353
) -> LiveNode {
13431354
match path.res {
1344-
Res::Local(hid) => self.access_var(hir_id, hid, succ, acc, path.span),
1355+
Res::Local(hid) => {
1356+
let in_upvars = self.upvars.map_or(false, |u| u.contains_key(&hid));
1357+
let in_captures = self.closure_captures.map_or(false, |c| c.contains_key(&hid));
1358+
1359+
match (in_upvars, in_captures) {
1360+
(false, _) | (true, true) => self.access_var(hir_id, hid, succ, acc, path.span),
1361+
(true, false) => {
1362+
// This case is possible when with RFC-2229, a wild pattern
1363+
// is used within a closure.
1364+
// eg: `let _ = x`. The closure doesn't capture x here,
1365+
// even though it's mentioned in the closure.
1366+
succ
1367+
}
1368+
}
1369+
}
13451370
_ => succ,
13461371
}
13471372
}
@@ -1531,11 +1556,15 @@ impl<'tcx> Liveness<'_, 'tcx> {
15311556
}
15321557

15331558
fn warn_about_unused_upvars(&self, entry_ln: LiveNode) {
1534-
let upvars = match self.upvars {
1559+
let closure_captures = match self.closure_captures {
15351560
None => return,
1536-
Some(upvars) => upvars,
1561+
Some(closure_captures) => closure_captures,
15371562
};
1538-
for (&var_hir_id, upvar) in upvars.iter() {
1563+
1564+
// If closure_captures is Some(), upvars must be Some() too.
1565+
let upvars = self.upvars.unwrap();
1566+
for &var_hir_id in closure_captures.keys() {
1567+
let upvar = upvars[&var_hir_id];
15391568
let var = self.variable(var_hir_id, upvar.span);
15401569
let upvar_id = ty::UpvarId {
15411570
var_path: ty::UpvarPath { hir_id: var_hir_id },

compiler/rustc_span/src/symbol.rs

+2
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ symbols! {
318318
call_mut,
319319
call_once,
320320
caller_location,
321+
capture_disjoint_fields,
321322
cdylib,
322323
ceilf32,
323324
ceilf64,
@@ -909,6 +910,7 @@ symbols! {
909910
rustc_args_required_const,
910911
rustc_attrs,
911912
rustc_builtin_macro,
913+
rustc_capture_analysis,
912914
rustc_clean,
913915
rustc_const_stable,
914916
rustc_const_unstable,

0 commit comments

Comments
 (0)