Skip to content

Commit df7daa8

Browse files
committed
Auto merge of #123795 - matthiaskrgr:rollup-deesd9c, r=matthiaskrgr
Rollup of 4 pull requests Successful merges: - #123660 (Make the computation of `coroutine_captures_by_ref_ty` more sophisticated) - #123738 (Call lower_const_param instead of duplicating the code) - #123774 (Fix typo MaybeUnit -> MaybeUninit) - #123790 (correct the handling of `bootstrap-cache-path` option) r? `@ghost` `@rustbot` modify labels: rollup
2 parents 72fe8a0 + 1620f33 commit df7daa8

File tree

15 files changed

+470
-113
lines changed

15 files changed

+470
-113
lines changed

compiler/rustc_hir_analysis/src/check/intrinsicck.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
143143
};
144144
assert!(
145145
ty.is_manually_drop(),
146-
"expected first field of `MaybeUnit` to be `ManuallyDrop`"
146+
"expected first field of `MaybeUninit` to be `ManuallyDrop`"
147147
);
148148
let fields = &ty.non_enum_variant().fields;
149149
let ty = fields[FieldIdx::ZERO].ty(self.tcx, args);

compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

+5
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,11 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
717717
}
718718
}
719719

720+
#[instrument(level = "debug", skip(self))]
721+
fn visit_pattern_type_pattern(&mut self, p: &'tcx hir::Pat<'tcx>) {
722+
intravisit::walk_pat(self, p)
723+
}
724+
720725
#[instrument(level = "debug", skip(self))]
721726
fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem<'tcx>) {
722727
use self::hir::TraitItemKind::*;

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -2234,11 +2234,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
22342234
.type_of(def_id)
22352235
.no_bound_vars()
22362236
.expect("const parameter types cannot be generic");
2237-
let item_def_id = tcx.parent(def_id);
2238-
let generics = tcx.generics_of(item_def_id);
2239-
let index = generics.param_def_id_to_index[&def_id];
2240-
let name = tcx.item_name(def_id);
2241-
ty::Const::new_param(tcx, ty::ParamConst::new(index, name), ty)
2237+
self.lower_const_param(expr.hir_id, ty)
22422238
}
22432239

22442240
_ => {

compiler/rustc_hir_typeck/src/upvar.rs

+91-23
Original file line numberDiff line numberDiff line change
@@ -367,37 +367,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
367367
ty::INNERMOST,
368368
ty::BoundRegion { var: ty::BoundVar::ZERO, kind: ty::BoundRegionKind::BrEnv },
369369
);
370+
371+
let num_args = args
372+
.as_coroutine_closure()
373+
.coroutine_closure_sig()
374+
.skip_binder()
375+
.tupled_inputs_ty
376+
.tuple_fields()
377+
.len();
378+
let typeck_results = self.typeck_results.borrow();
379+
370380
let tupled_upvars_ty_for_borrow = Ty::new_tup_from_iter(
371381
self.tcx,
372-
self.typeck_results
373-
.borrow()
374-
.closure_min_captures_flattened(
375-
self.tcx.coroutine_for_closure(closure_def_id).expect_local(),
376-
)
377-
// Skip the captures that are just moving the closure's args
378-
// into the coroutine. These are always by move, and we append
379-
// those later in the `CoroutineClosureSignature` helper functions.
380-
.skip(
381-
args.as_coroutine_closure()
382-
.coroutine_closure_sig()
383-
.skip_binder()
384-
.tupled_inputs_ty
385-
.tuple_fields()
386-
.len(),
387-
)
388-
.map(|captured_place| {
389-
let upvar_ty = captured_place.place.ty();
390-
let capture = captured_place.info.capture_kind;
382+
ty::analyze_coroutine_closure_captures(
383+
typeck_results.closure_min_captures_flattened(closure_def_id),
384+
typeck_results
385+
.closure_min_captures_flattened(
386+
self.tcx.coroutine_for_closure(closure_def_id).expect_local(),
387+
)
388+
// Skip the captures that are just moving the closure's args
389+
// into the coroutine. These are always by move, and we append
390+
// those later in the `CoroutineClosureSignature` helper functions.
391+
.skip(num_args),
392+
|(_, parent_capture), (_, child_capture)| {
393+
// This is subtle. See documentation on function.
394+
let needs_ref = should_reborrow_from_env_of_parent_coroutine_closure(
395+
parent_capture,
396+
child_capture,
397+
);
398+
399+
let upvar_ty = child_capture.place.ty();
400+
let capture = child_capture.info.capture_kind;
391401
// Not all upvars are captured by ref, so use
392402
// `apply_capture_kind_on_capture_ty` to ensure that we
393403
// compute the right captured type.
394-
apply_capture_kind_on_capture_ty(
404+
return apply_capture_kind_on_capture_ty(
395405
self.tcx,
396406
upvar_ty,
397407
capture,
398-
Some(closure_env_region),
399-
)
400-
}),
408+
if needs_ref { Some(closure_env_region) } else { child_capture.region },
409+
);
410+
},
411+
),
401412
);
402413
let coroutine_captures_by_ref_ty = Ty::new_fn_ptr(
403414
self.tcx,
@@ -1761,6 +1772,63 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17611772
}
17621773
}
17631774

1775+
/// Determines whether a child capture that is derived from a parent capture
1776+
/// should be borrowed with the lifetime of the parent coroutine-closure's env.
1777+
///
1778+
/// There are two cases when this needs to happen:
1779+
///
1780+
/// (1.) Are we borrowing data owned by the parent closure? We can determine if
1781+
/// that is the case by checking if the parent capture is by move, EXCEPT if we
1782+
/// apply a deref projection, which means we're reborrowing a reference that we
1783+
/// captured by move.
1784+
///
1785+
/// ```rust
1786+
/// #![feature(async_closure)]
1787+
/// let x = &1i32; // Let's call this lifetime `'1`.
1788+
/// let c = async move || {
1789+
/// println!("{:?}", *x);
1790+
/// // Even though the inner coroutine borrows by ref, we're only capturing `*x`,
1791+
/// // not `x`, so the inner closure is allowed to reborrow the data for `'1`.
1792+
/// };
1793+
/// ```
1794+
///
1795+
/// (2.) If a coroutine is mutably borrowing from a parent capture, then that
1796+
/// mutable borrow cannot live for longer than either the parent *or* the borrow
1797+
/// that we have on the original upvar. Therefore we always need to borrow the
1798+
/// child capture with the lifetime of the parent coroutine-closure's env.
1799+
///
1800+
/// ```rust
1801+
/// #![feature(async_closure)]
1802+
/// let mut x = 1i32;
1803+
/// let c = async || {
1804+
/// x = 1;
1805+
/// // The parent borrows `x` for some `&'1 mut i32`.
1806+
/// // However, when we call `c()`, we implicitly autoref for the signature of
1807+
/// // `AsyncFnMut::async_call_mut`. Let's call that lifetime `'call`. Since
1808+
/// // the maximum that `&'call mut &'1 mut i32` can be reborrowed is `&'call mut i32`,
1809+
/// // the inner coroutine should capture w/ the lifetime of the coroutine-closure.
1810+
/// };
1811+
/// ```
1812+
///
1813+
/// If either of these cases apply, then we should capture the borrow with the
1814+
/// lifetime of the parent coroutine-closure's env. Luckily, if this function is
1815+
/// not correct, then the program is not unsound, since we still borrowck and validate
1816+
/// the choices made from this function -- the only side-effect is that the user
1817+
/// may receive unnecessary borrowck errors.
1818+
fn should_reborrow_from_env_of_parent_coroutine_closure<'tcx>(
1819+
parent_capture: &ty::CapturedPlace<'tcx>,
1820+
child_capture: &ty::CapturedPlace<'tcx>,
1821+
) -> bool {
1822+
// (1.)
1823+
(!parent_capture.is_by_ref()
1824+
&& !matches!(
1825+
child_capture.place.projections.get(parent_capture.place.projections.len()),
1826+
Some(Projection { kind: ProjectionKind::Deref, .. })
1827+
))
1828+
// (2.)
1829+
|| matches!(child_capture.info.capture_kind, UpvarCapture::ByRef(ty::BorrowKind::MutBorrow))
1830+
}
1831+
17641832
/// Truncate the capture so that the place being borrowed is in accordance with RFC 1240,
17651833
/// which states that it's unsafe to take a reference into a struct marked `repr(packed)`.
17661834
fn restrict_repr_packed_field_ref_capture<'tcx>(

compiler/rustc_middle/src/ty/closure.rs

+67
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::{mir, ty};
66
use std::fmt::Write;
77

88
use crate::query::Providers;
9+
use rustc_data_structures::captures::Captures;
910
use rustc_data_structures::fx::FxIndexMap;
1011
use rustc_hir as hir;
1112
use rustc_hir::def_id::LocalDefId;
@@ -415,6 +416,72 @@ impl BorrowKind {
415416
}
416417
}
417418

419+
pub fn analyze_coroutine_closure_captures<'a, 'tcx: 'a, T>(
420+
parent_captures: impl IntoIterator<Item = &'a CapturedPlace<'tcx>>,
421+
child_captures: impl IntoIterator<Item = &'a CapturedPlace<'tcx>>,
422+
mut for_each: impl FnMut((usize, &'a CapturedPlace<'tcx>), (usize, &'a CapturedPlace<'tcx>)) -> T,
423+
) -> impl Iterator<Item = T> + Captures<'a> + Captures<'tcx> {
424+
std::iter::from_coroutine(move || {
425+
let mut child_captures = child_captures.into_iter().enumerate().peekable();
426+
427+
// One parent capture may correspond to several child captures if we end up
428+
// refining the set of captures via edition-2021 precise captures. We want to
429+
// match up any number of child captures with one parent capture, so we keep
430+
// peeking off this `Peekable` until the child doesn't match anymore.
431+
for (parent_field_idx, parent_capture) in parent_captures.into_iter().enumerate() {
432+
// Make sure we use every field at least once, b/c why are we capturing something
433+
// if it's not used in the inner coroutine.
434+
let mut field_used_at_least_once = false;
435+
436+
// A parent matches a child if they share the same prefix of projections.
437+
// The child may have more, if it is capturing sub-fields out of
438+
// something that is captured by-move in the parent closure.
439+
while child_captures.peek().map_or(false, |(_, child_capture)| {
440+
child_prefix_matches_parent_projections(parent_capture, child_capture)
441+
}) {
442+
let (child_field_idx, child_capture) = child_captures.next().unwrap();
443+
// This analysis only makes sense if the parent capture is a
444+
// prefix of the child capture.
445+
assert!(
446+
child_capture.place.projections.len() >= parent_capture.place.projections.len(),
447+
"parent capture ({parent_capture:#?}) expected to be prefix of \
448+
child capture ({child_capture:#?})"
449+
);
450+
451+
yield for_each(
452+
(parent_field_idx, parent_capture),
453+
(child_field_idx, child_capture),
454+
);
455+
456+
field_used_at_least_once = true;
457+
}
458+
459+
// Make sure the field was used at least once.
460+
assert!(
461+
field_used_at_least_once,
462+
"we captured {parent_capture:#?} but it was not used in the child coroutine?"
463+
);
464+
}
465+
assert_eq!(child_captures.next(), None, "leftover child captures?");
466+
})
467+
}
468+
469+
fn child_prefix_matches_parent_projections(
470+
parent_capture: &ty::CapturedPlace<'_>,
471+
child_capture: &ty::CapturedPlace<'_>,
472+
) -> bool {
473+
let HirPlaceBase::Upvar(parent_base) = parent_capture.place.base else {
474+
bug!("expected capture to be an upvar");
475+
};
476+
let HirPlaceBase::Upvar(child_base) = child_capture.place.base else {
477+
bug!("expected capture to be an upvar");
478+
};
479+
480+
parent_base.var_path.hir_id == child_base.var_path.hir_id
481+
&& std::iter::zip(&child_capture.place.projections, &parent_capture.place.projections)
482+
.all(|(child, parent)| child.kind == parent.kind)
483+
}
484+
418485
pub fn provide(providers: &mut Providers) {
419486
*providers = Providers { closure_typeinfo, ..*providers }
420487
}

compiler/rustc_middle/src/ty/mod.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,10 @@ pub use rustc_type_ir::ConstKind::{
7777
pub use rustc_type_ir::*;
7878

7979
pub use self::closure::{
80-
is_ancestor_or_same_capture, place_to_string_for_capture, BorrowKind, CaptureInfo,
81-
CapturedPlace, ClosureTypeInfo, MinCaptureInformationMap, MinCaptureList,
82-
RootVariableMinCaptureList, UpvarCapture, UpvarId, UpvarPath, CAPTURE_STRUCT_LOCAL,
80+
analyze_coroutine_closure_captures, is_ancestor_or_same_capture, place_to_string_for_capture,
81+
BorrowKind, CaptureInfo, CapturedPlace, ClosureTypeInfo, MinCaptureInformationMap,
82+
MinCaptureList, RootVariableMinCaptureList, UpvarCapture, UpvarId, UpvarPath,
83+
CAPTURE_STRUCT_LOCAL,
8384
};
8485
pub use self::consts::{
8586
Const, ConstData, ConstInt, ConstKind, Expr, ScalarInt, UnevaluatedConst, ValTree,

compiler/rustc_mir_transform/src/coroutine/by_move_body.rs

+10-68
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
7272
use rustc_data_structures::unord::UnordMap;
7373
use rustc_hir as hir;
74-
use rustc_middle::hir::place::{PlaceBase, Projection, ProjectionKind};
74+
use rustc_middle::hir::place::{Projection, ProjectionKind};
7575
use rustc_middle::mir::visit::MutVisitor;
7676
use rustc_middle::mir::{self, dump_mir, MirPass};
7777
use rustc_middle::ty::{self, InstanceDef, Ty, TyCtxt, TypeVisitableExt};
@@ -124,44 +124,10 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
124124
.tuple_fields()
125125
.len();
126126

127-
let mut field_remapping = UnordMap::default();
128-
129-
let mut child_captures = tcx
130-
.closure_captures(coroutine_def_id)
131-
.iter()
132-
.copied()
133-
// By construction we capture all the args first.
134-
.skip(num_args)
135-
.enumerate()
136-
.peekable();
137-
138-
// One parent capture may correspond to several child captures if we end up
139-
// refining the set of captures via edition-2021 precise captures. We want to
140-
// match up any number of child captures with one parent capture, so we keep
141-
// peeking off this `Peekable` until the child doesn't match anymore.
142-
for (parent_field_idx, parent_capture) in
143-
tcx.closure_captures(parent_def_id).iter().copied().enumerate()
144-
{
145-
// Make sure we use every field at least once, b/c why are we capturing something
146-
// if it's not used in the inner coroutine.
147-
let mut field_used_at_least_once = false;
148-
149-
// A parent matches a child if they share the same prefix of projections.
150-
// The child may have more, if it is capturing sub-fields out of
151-
// something that is captured by-move in the parent closure.
152-
while child_captures.peek().map_or(false, |(_, child_capture)| {
153-
child_prefix_matches_parent_projections(parent_capture, child_capture)
154-
}) {
155-
let (child_field_idx, child_capture) = child_captures.next().unwrap();
156-
157-
// This analysis only makes sense if the parent capture is a
158-
// prefix of the child capture.
159-
assert!(
160-
child_capture.place.projections.len() >= parent_capture.place.projections.len(),
161-
"parent capture ({parent_capture:#?}) expected to be prefix of \
162-
child capture ({child_capture:#?})"
163-
);
164-
127+
let field_remapping: UnordMap<_, _> = ty::analyze_coroutine_closure_captures(
128+
tcx.closure_captures(parent_def_id).iter().copied(),
129+
tcx.closure_captures(coroutine_def_id).iter().skip(num_args).copied(),
130+
|(parent_field_idx, parent_capture), (child_field_idx, child_capture)| {
165131
// Store this set of additional projections (fields and derefs).
166132
// We need to re-apply them later.
167133
let child_precise_captures =
@@ -192,26 +158,18 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
192158
),
193159
};
194160

195-
field_remapping.insert(
161+
(
196162
FieldIdx::from_usize(child_field_idx + num_args),
197163
(
198164
FieldIdx::from_usize(parent_field_idx + num_args),
199165
parent_capture_ty,
200166
needs_deref,
201167
child_precise_captures,
202168
),
203-
);
204-
205-
field_used_at_least_once = true;
206-
}
207-
208-
// Make sure the field was used at least once.
209-
assert!(
210-
field_used_at_least_once,
211-
"we captured {parent_capture:#?} but it was not used in the child coroutine?"
212-
);
213-
}
214-
assert_eq!(child_captures.next(), None, "leftover child captures?");
169+
)
170+
},
171+
)
172+
.collect();
215173

216174
if coroutine_kind == ty::ClosureKind::FnOnce {
217175
assert_eq!(field_remapping.len(), tcx.closure_captures(parent_def_id).len());
@@ -241,22 +199,6 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
241199
}
242200
}
243201

244-
fn child_prefix_matches_parent_projections(
245-
parent_capture: &ty::CapturedPlace<'_>,
246-
child_capture: &ty::CapturedPlace<'_>,
247-
) -> bool {
248-
let PlaceBase::Upvar(parent_base) = parent_capture.place.base else {
249-
bug!("expected capture to be an upvar");
250-
};
251-
let PlaceBase::Upvar(child_base) = child_capture.place.base else {
252-
bug!("expected capture to be an upvar");
253-
};
254-
255-
parent_base.var_path.hir_id == child_base.var_path.hir_id
256-
&& std::iter::zip(&child_capture.place.projections, &parent_capture.place.projections)
257-
.all(|(child, parent)| child.kind == parent.kind)
258-
}
259-
260202
struct MakeByMoveBody<'tcx> {
261203
tcx: TyCtxt<'tcx>,
262204
field_remapping: UnordMap<FieldIdx, (FieldIdx, Ty<'tcx>, bool, &'tcx [Projection<'tcx>])>,

config.example.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@
302302

303303
# Set the bootstrap/download cache path. It is useful when building rust
304304
# repeatedly in a CI invironment.
305-
# bootstrap-cache-path = /shared/cache
305+
#bootstrap-cache-path = /path/to/shared/cache
306306

307307
# Enable a build of the extended Rust tool set which is not only the compiler
308308
# but also tools such as Cargo. This will also produce "combined installers"

0 commit comments

Comments
 (0)