Skip to content

Rollup of 4 pull requests #123795

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

Merged
merged 9 commits into from
Apr 11, 2024
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/intrinsicck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
};
assert!(
ty.is_manually_drop(),
"expected first field of `MaybeUnit` to be `ManuallyDrop`"
"expected first field of `MaybeUninit` to be `ManuallyDrop`"
);
let fields = &ty.non_enum_variant().fields;
let ty = fields[FieldIdx::ZERO].ty(self.tcx, args);
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,11 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
}
}

#[instrument(level = "debug", skip(self))]
fn visit_pattern_type_pattern(&mut self, p: &'tcx hir::Pat<'tcx>) {
intravisit::walk_pat(self, p)
}

#[instrument(level = "debug", skip(self))]
fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem<'tcx>) {
use self::hir::TraitItemKind::*;
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2234,11 +2234,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
.type_of(def_id)
.no_bound_vars()
.expect("const parameter types cannot be generic");
let item_def_id = tcx.parent(def_id);
let generics = tcx.generics_of(item_def_id);
let index = generics.param_def_id_to_index[&def_id];
let name = tcx.item_name(def_id);
ty::Const::new_param(tcx, ty::ParamConst::new(index, name), ty)
self.lower_const_param(expr.hir_id, ty)
}

_ => {
Expand Down
114 changes: 91 additions & 23 deletions compiler/rustc_hir_typeck/src/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,37 +367,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty::INNERMOST,
ty::BoundRegion { var: ty::BoundVar::ZERO, kind: ty::BoundRegionKind::BrEnv },
);

let num_args = args
.as_coroutine_closure()
.coroutine_closure_sig()
.skip_binder()
.tupled_inputs_ty
.tuple_fields()
.len();
let typeck_results = self.typeck_results.borrow();

let tupled_upvars_ty_for_borrow = Ty::new_tup_from_iter(
self.tcx,
self.typeck_results
.borrow()
.closure_min_captures_flattened(
self.tcx.coroutine_for_closure(closure_def_id).expect_local(),
)
// Skip the captures that are just moving the closure's args
// into the coroutine. These are always by move, and we append
// those later in the `CoroutineClosureSignature` helper functions.
.skip(
args.as_coroutine_closure()
.coroutine_closure_sig()
.skip_binder()
.tupled_inputs_ty
.tuple_fields()
.len(),
)
.map(|captured_place| {
let upvar_ty = captured_place.place.ty();
let capture = captured_place.info.capture_kind;
ty::analyze_coroutine_closure_captures(
typeck_results.closure_min_captures_flattened(closure_def_id),
typeck_results
.closure_min_captures_flattened(
self.tcx.coroutine_for_closure(closure_def_id).expect_local(),
)
// Skip the captures that are just moving the closure's args
// into the coroutine. These are always by move, and we append
// those later in the `CoroutineClosureSignature` helper functions.
.skip(num_args),
|(_, parent_capture), (_, child_capture)| {
// This is subtle. See documentation on function.
let needs_ref = should_reborrow_from_env_of_parent_coroutine_closure(
parent_capture,
child_capture,
);

let upvar_ty = child_capture.place.ty();
let capture = child_capture.info.capture_kind;
// Not all upvars are captured by ref, so use
// `apply_capture_kind_on_capture_ty` to ensure that we
// compute the right captured type.
apply_capture_kind_on_capture_ty(
return apply_capture_kind_on_capture_ty(
self.tcx,
upvar_ty,
capture,
Some(closure_env_region),
)
}),
if needs_ref { Some(closure_env_region) } else { child_capture.region },
);
},
),
);
let coroutine_captures_by_ref_ty = Ty::new_fn_ptr(
self.tcx,
Expand Down Expand Up @@ -1761,6 +1772,63 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

/// Determines whether a child capture that is derived from a parent capture
/// should be borrowed with the lifetime of the parent coroutine-closure's env.
///
/// There are two cases when this needs to happen:
///
/// (1.) Are we borrowing data owned by the parent closure? We can determine if
/// that is the case by checking if the parent capture is by move, EXCEPT if we
/// apply a deref projection, which means we're reborrowing a reference that we
/// captured by move.
///
/// ```rust
/// #![feature(async_closure)]
/// let x = &1i32; // Let's call this lifetime `'1`.
/// let c = async move || {
/// println!("{:?}", *x);
/// // Even though the inner coroutine borrows by ref, we're only capturing `*x`,
/// // not `x`, so the inner closure is allowed to reborrow the data for `'1`.
/// };
/// ```
///
/// (2.) If a coroutine is mutably borrowing from a parent capture, then that
/// mutable borrow cannot live for longer than either the parent *or* the borrow
/// that we have on the original upvar. Therefore we always need to borrow the
/// child capture with the lifetime of the parent coroutine-closure's env.
///
/// ```rust
/// #![feature(async_closure)]
/// let mut x = 1i32;
/// let c = async || {
/// x = 1;
/// // The parent borrows `x` for some `&'1 mut i32`.
/// // However, when we call `c()`, we implicitly autoref for the signature of
/// // `AsyncFnMut::async_call_mut`. Let's call that lifetime `'call`. Since
/// // the maximum that `&'call mut &'1 mut i32` can be reborrowed is `&'call mut i32`,
/// // the inner coroutine should capture w/ the lifetime of the coroutine-closure.
/// };
/// ```
///
/// If either of these cases apply, then we should capture the borrow with the
/// lifetime of the parent coroutine-closure's env. Luckily, if this function is
/// not correct, then the program is not unsound, since we still borrowck and validate
/// the choices made from this function -- the only side-effect is that the user
/// may receive unnecessary borrowck errors.
fn should_reborrow_from_env_of_parent_coroutine_closure<'tcx>(
parent_capture: &ty::CapturedPlace<'tcx>,
child_capture: &ty::CapturedPlace<'tcx>,
) -> bool {
// (1.)
(!parent_capture.is_by_ref()
&& !matches!(
child_capture.place.projections.get(parent_capture.place.projections.len()),
Some(Projection { kind: ProjectionKind::Deref, .. })
))
// (2.)
|| matches!(child_capture.info.capture_kind, UpvarCapture::ByRef(ty::BorrowKind::MutBorrow))
}

/// Truncate the capture so that the place being borrowed is in accordance with RFC 1240,
/// which states that it's unsafe to take a reference into a struct marked `repr(packed)`.
fn restrict_repr_packed_field_ref_capture<'tcx>(
Expand Down
67 changes: 67 additions & 0 deletions compiler/rustc_middle/src/ty/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::{mir, ty};
use std::fmt::Write;

use crate::query::Providers;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxIndexMap;
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
Expand Down Expand Up @@ -415,6 +416,72 @@ impl BorrowKind {
}
}

pub fn analyze_coroutine_closure_captures<'a, 'tcx: 'a, T>(
parent_captures: impl IntoIterator<Item = &'a CapturedPlace<'tcx>>,
child_captures: impl IntoIterator<Item = &'a CapturedPlace<'tcx>>,
mut for_each: impl FnMut((usize, &'a CapturedPlace<'tcx>), (usize, &'a CapturedPlace<'tcx>)) -> T,
) -> impl Iterator<Item = T> + Captures<'a> + Captures<'tcx> {
std::iter::from_coroutine(move || {
let mut child_captures = child_captures.into_iter().enumerate().peekable();

// One parent capture may correspond to several child captures if we end up
// refining the set of captures via edition-2021 precise captures. We want to
// match up any number of child captures with one parent capture, so we keep
// peeking off this `Peekable` until the child doesn't match anymore.
for (parent_field_idx, parent_capture) in parent_captures.into_iter().enumerate() {
// Make sure we use every field at least once, b/c why are we capturing something
// if it's not used in the inner coroutine.
let mut field_used_at_least_once = false;

// A parent matches a child if they share the same prefix of projections.
// The child may have more, if it is capturing sub-fields out of
// something that is captured by-move in the parent closure.
while child_captures.peek().map_or(false, |(_, child_capture)| {
child_prefix_matches_parent_projections(parent_capture, child_capture)
}) {
let (child_field_idx, child_capture) = child_captures.next().unwrap();
// This analysis only makes sense if the parent capture is a
// prefix of the child capture.
assert!(
child_capture.place.projections.len() >= parent_capture.place.projections.len(),
"parent capture ({parent_capture:#?}) expected to be prefix of \
child capture ({child_capture:#?})"
);

yield for_each(
(parent_field_idx, parent_capture),
(child_field_idx, child_capture),
);

field_used_at_least_once = true;
}

// Make sure the field was used at least once.
assert!(
field_used_at_least_once,
"we captured {parent_capture:#?} but it was not used in the child coroutine?"
);
}
assert_eq!(child_captures.next(), None, "leftover child captures?");
})
}

fn child_prefix_matches_parent_projections(
parent_capture: &ty::CapturedPlace<'_>,
child_capture: &ty::CapturedPlace<'_>,
) -> bool {
let HirPlaceBase::Upvar(parent_base) = parent_capture.place.base else {
bug!("expected capture to be an upvar");
};
let HirPlaceBase::Upvar(child_base) = child_capture.place.base else {
bug!("expected capture to be an upvar");
};

parent_base.var_path.hir_id == child_base.var_path.hir_id
&& std::iter::zip(&child_capture.place.projections, &parent_capture.place.projections)
.all(|(child, parent)| child.kind == parent.kind)
}

pub fn provide(providers: &mut Providers) {
*providers = Providers { closure_typeinfo, ..*providers }
}
7 changes: 4 additions & 3 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ pub use rustc_type_ir::ConstKind::{
pub use rustc_type_ir::*;

pub use self::closure::{
is_ancestor_or_same_capture, place_to_string_for_capture, BorrowKind, CaptureInfo,
CapturedPlace, ClosureTypeInfo, MinCaptureInformationMap, MinCaptureList,
RootVariableMinCaptureList, UpvarCapture, UpvarId, UpvarPath, CAPTURE_STRUCT_LOCAL,
analyze_coroutine_closure_captures, is_ancestor_or_same_capture, place_to_string_for_capture,
BorrowKind, CaptureInfo, CapturedPlace, ClosureTypeInfo, MinCaptureInformationMap,
MinCaptureList, RootVariableMinCaptureList, UpvarCapture, UpvarId, UpvarPath,
CAPTURE_STRUCT_LOCAL,
};
pub use self::consts::{
Const, ConstData, ConstInt, ConstKind, Expr, ScalarInt, UnevaluatedConst, ValTree,
Expand Down
78 changes: 10 additions & 68 deletions compiler/rustc_mir_transform/src/coroutine/by_move_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@

use rustc_data_structures::unord::UnordMap;
use rustc_hir as hir;
use rustc_middle::hir::place::{PlaceBase, Projection, ProjectionKind};
use rustc_middle::hir::place::{Projection, ProjectionKind};
use rustc_middle::mir::visit::MutVisitor;
use rustc_middle::mir::{self, dump_mir, MirPass};
use rustc_middle::ty::{self, InstanceDef, Ty, TyCtxt, TypeVisitableExt};
Expand Down Expand Up @@ -124,44 +124,10 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
.tuple_fields()
.len();

let mut field_remapping = UnordMap::default();

let mut child_captures = tcx
.closure_captures(coroutine_def_id)
.iter()
.copied()
// By construction we capture all the args first.
.skip(num_args)
.enumerate()
.peekable();

// One parent capture may correspond to several child captures if we end up
// refining the set of captures via edition-2021 precise captures. We want to
// match up any number of child captures with one parent capture, so we keep
// peeking off this `Peekable` until the child doesn't match anymore.
for (parent_field_idx, parent_capture) in
tcx.closure_captures(parent_def_id).iter().copied().enumerate()
{
// Make sure we use every field at least once, b/c why are we capturing something
// if it's not used in the inner coroutine.
let mut field_used_at_least_once = false;

// A parent matches a child if they share the same prefix of projections.
// The child may have more, if it is capturing sub-fields out of
// something that is captured by-move in the parent closure.
while child_captures.peek().map_or(false, |(_, child_capture)| {
child_prefix_matches_parent_projections(parent_capture, child_capture)
}) {
let (child_field_idx, child_capture) = child_captures.next().unwrap();

// This analysis only makes sense if the parent capture is a
// prefix of the child capture.
assert!(
child_capture.place.projections.len() >= parent_capture.place.projections.len(),
"parent capture ({parent_capture:#?}) expected to be prefix of \
child capture ({child_capture:#?})"
);

let field_remapping: UnordMap<_, _> = ty::analyze_coroutine_closure_captures(
tcx.closure_captures(parent_def_id).iter().copied(),
tcx.closure_captures(coroutine_def_id).iter().skip(num_args).copied(),
|(parent_field_idx, parent_capture), (child_field_idx, child_capture)| {
// Store this set of additional projections (fields and derefs).
// We need to re-apply them later.
let child_precise_captures =
Expand Down Expand Up @@ -192,26 +158,18 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
),
};

field_remapping.insert(
(
FieldIdx::from_usize(child_field_idx + num_args),
(
FieldIdx::from_usize(parent_field_idx + num_args),
parent_capture_ty,
needs_deref,
child_precise_captures,
),
);

field_used_at_least_once = true;
}

// Make sure the field was used at least once.
assert!(
field_used_at_least_once,
"we captured {parent_capture:#?} but it was not used in the child coroutine?"
);
}
assert_eq!(child_captures.next(), None, "leftover child captures?");
)
},
)
.collect();

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

fn child_prefix_matches_parent_projections(
parent_capture: &ty::CapturedPlace<'_>,
child_capture: &ty::CapturedPlace<'_>,
) -> bool {
let PlaceBase::Upvar(parent_base) = parent_capture.place.base else {
bug!("expected capture to be an upvar");
};
let PlaceBase::Upvar(child_base) = child_capture.place.base else {
bug!("expected capture to be an upvar");
};

parent_base.var_path.hir_id == child_base.var_path.hir_id
&& std::iter::zip(&child_capture.place.projections, &parent_capture.place.projections)
.all(|(child, parent)| child.kind == parent.kind)
}

struct MakeByMoveBody<'tcx> {
tcx: TyCtxt<'tcx>,
field_remapping: UnordMap<FieldIdx, (FieldIdx, Ty<'tcx>, bool, &'tcx [Projection<'tcx>])>,
Expand Down
2 changes: 1 addition & 1 deletion config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@

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

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