diff --git a/Cargo.lock b/Cargo.lock index 529e17b158fc8..cde44f96ce76f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4388,6 +4388,7 @@ dependencies = [ "rustc_attr", "rustc_data_structures", "rustc_errors", + "rustc_graphviz", "rustc_hir", "rustc_hir_pretty", "rustc_index", @@ -4395,6 +4396,7 @@ dependencies = [ "rustc_lint", "rustc_macros", "rustc_middle", + "rustc_serialize", "rustc_session", "rustc_span", "rustc_target", diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index 39ca41c92ff75..75dd223d014d4 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -308,7 +308,7 @@ pub struct ScopeTree { /// The reason is that semantically, until the `box` expression returns, /// the values are still owned by their containing expressions. So /// we'll see that `&x`. - pub yield_in_scope: FxHashMap, + pub yield_in_scope: FxHashMap>, /// The number of visit_expr and visit_pat calls done in the body. /// Used to sanity check visit_expr/visit_pat call count when @@ -423,8 +423,8 @@ impl ScopeTree { /// Checks whether the given scope contains a `yield`. If so, /// returns `Some(YieldData)`. If not, returns `None`. - pub fn yield_in_scope(&self, scope: Scope) -> Option { - self.yield_in_scope.get(&scope).cloned() + pub fn yield_in_scope(&self, scope: Scope) -> Option<&Vec> { + self.yield_in_scope.get(&scope) } /// Gives the number of expressions visited in a body. diff --git a/compiler/rustc_passes/src/region.rs b/compiler/rustc_passes/src/region.rs index db699a56645c2..fdf93e5893247 100644 --- a/compiler/rustc_passes/src/region.rs +++ b/compiler/rustc_passes/src/region.rs @@ -366,7 +366,8 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h let target_scopes = visitor.fixup_scopes.drain(start_point..); for scope in target_scopes { - let mut yield_data = visitor.scope_tree.yield_in_scope.get_mut(&scope).unwrap(); + let mut yield_data = + visitor.scope_tree.yield_in_scope.get_mut(&scope).unwrap().last_mut().unwrap(); let count = yield_data.expr_and_pat_count; let span = yield_data.span; @@ -429,7 +430,13 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h }; let data = YieldData { span, expr_and_pat_count: visitor.expr_and_pat_count, source: *source }; - visitor.scope_tree.yield_in_scope.insert(scope, data); + match visitor.scope_tree.yield_in_scope.get_mut(&scope) { + Some(yields) => yields.push(data), + None => { + visitor.scope_tree.yield_in_scope.insert(scope, vec![data]); + } + } + if visitor.pessimistic_yield { debug!("resolve_expr in pessimistic_yield - marking scope {:?} for fixup", scope); visitor.fixup_scopes.push(scope); diff --git a/compiler/rustc_typeck/Cargo.toml b/compiler/rustc_typeck/Cargo.toml index 7e570e151c529..57930a28a35a1 100644 --- a/compiler/rustc_typeck/Cargo.toml +++ b/compiler/rustc_typeck/Cargo.toml @@ -15,6 +15,7 @@ rustc_middle = { path = "../rustc_middle" } rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } +rustc_graphviz = { path = "../rustc_graphviz" } rustc_hir = { path = "../rustc_hir" } rustc_hir_pretty = { path = "../rustc_hir_pretty" } rustc_target = { path = "../rustc_target" } @@ -27,3 +28,4 @@ rustc_infer = { path = "../rustc_infer" } rustc_trait_selection = { path = "../rustc_trait_selection" } rustc_ty_utils = { path = "../rustc_ty_utils" } rustc_lint = { path = "../rustc_lint" } +rustc_serialize = { path = "../rustc_serialize" } diff --git a/compiler/rustc_typeck/src/check/generator_interior.rs b/compiler/rustc_typeck/src/check/generator_interior.rs index fb6e11dbfb738..56b6dd9a28446 100644 --- a/compiler/rustc_typeck/src/check/generator_interior.rs +++ b/compiler/rustc_typeck/src/check/generator_interior.rs @@ -3,6 +3,7 @@ //! is calculated in `rustc_const_eval::transform::generator` and may be a subset of the //! types computed here. +use self::drop_ranges::DropRanges; use super::FnCtxt; use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; use rustc_errors::pluralize; @@ -19,6 +20,8 @@ use rustc_span::Span; use smallvec::SmallVec; use tracing::debug; +mod drop_ranges; + struct InteriorVisitor<'a, 'tcx> { fcx: &'a FnCtxt<'a, 'tcx>, types: FxIndexSet>, @@ -34,6 +37,7 @@ struct InteriorVisitor<'a, 'tcx> { guard_bindings: SmallVec<[SmallVec<[HirId; 4]>; 1]>, guard_bindings_set: HirIdSet, linted_values: HirIdSet, + drop_ranges: DropRanges, } impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { @@ -48,9 +52,11 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { ) { use rustc_span::DUMMY_SP; + let ty = self.fcx.resolve_vars_if_possible(ty); + debug!( - "generator_interior: attempting to record type {:?} {:?} {:?} {:?}", - ty, scope, expr, source_span + "attempting to record type ty={:?}; hir_id={:?}; scope={:?}; expr={:?}; source_span={:?}; expr_count={:?}", + ty, hir_id, scope, expr, source_span, self.expr_count, ); let live_across_yield = scope @@ -63,21 +69,27 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { // // See the mega-comment at `yield_in_scope` for a proof. - debug!( - "comparing counts yield: {} self: {}, source_span = {:?}", - yield_data.expr_and_pat_count, self.expr_count, source_span - ); + yield_data + .iter() + .find(|yield_data| { + debug!( + "comparing counts yield: {} self: {}, source_span = {:?}", + yield_data.expr_and_pat_count, self.expr_count, source_span + ); + + if self.drop_ranges.is_dropped_at(hir_id, yield_data.expr_and_pat_count) + { + debug!("value is dropped at yield point; not recording"); + return false; + } - // If it is a borrowing happening in the guard, - // it needs to be recorded regardless because they - // do live across this yield point. - if guard_borrowing_from_pattern - || yield_data.expr_and_pat_count >= self.expr_count - { - Some(yield_data) - } else { - None - } + // If it is a borrowing happening in the guard, + // it needs to be recorded regardless because they + // do live across this yield point. + guard_borrowing_from_pattern + || yield_data.expr_and_pat_count >= self.expr_count + }) + .cloned() }) }) .unwrap_or_else(|| { @@ -85,7 +97,6 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { }); if let Some(yield_data) = live_across_yield { - let ty = self.fcx.resolve_vars_if_possible(ty); debug!( "type in expr = {:?}, scope = {:?}, type = {:?}, count = {}, yield_span = {:?}", expr, scope, ty, self.expr_count, yield_data.span @@ -154,7 +165,6 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { self.expr_count, expr.map(|e| e.span) ); - let ty = self.fcx.resolve_vars_if_possible(ty); if let Some((unresolved_type, unresolved_type_span)) = self.fcx.unresolved_type_vars(&ty) { @@ -186,6 +196,7 @@ pub fn resolve_interior<'a, 'tcx>( guard_bindings: <_>::default(), guard_bindings_set: <_>::default(), linted_values: <_>::default(), + drop_ranges: drop_ranges::compute_drop_ranges(fcx, def_id, body), }; intravisit::walk_body(&mut visitor, body); @@ -313,6 +324,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { let mut guard_borrowing_from_pattern = false; + match &expr.kind { ExprKind::Call(callee, args) => match &callee.kind { ExprKind::Path(qpath) => { diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs new file mode 100644 index 0000000000000..21a8d7b563456 --- /dev/null +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs @@ -0,0 +1,269 @@ +//! Drop range analysis finds the portions of the tree where a value is guaranteed to be dropped +//! (i.e. moved, uninitialized, etc.). This is used to exclude the types of those values from the +//! generator type. See `InteriorVisitor::record` for where the results of this analysis are used. +//! +//! There are three phases to this analysis: +//! 1. Use `ExprUseVisitor` to identify the interesting values that are consumed and borrowed. +//! 2. Use `DropRangeVisitor` to find where the interesting values are dropped or reinitialized, +//! and also build a control flow graph. +//! 3. Use `DropRanges::propagate_to_fixpoint` to flow the dropped/reinitialized information through +//! the CFG and find the exact points where we know a value is definitely dropped. +//! +//! The end result is a data structure that maps the post-order index of each node in the HIR tree +//! to a set of values that are known to be dropped at that location. + +use self::cfg_build::build_control_flow_graph; +use self::record_consumed_borrow::find_consumed_and_borrowed; +use crate::check::FnCtxt; +use hir::def_id::DefId; +use hir::{Body, HirId, HirIdMap, Node}; +use rustc_data_structures::fx::FxHashMap; +use rustc_hir as hir; +use rustc_index::bit_set::BitSet; +use rustc_index::vec::IndexVec; +use rustc_middle::hir::map::Map; +use rustc_middle::hir::place::{PlaceBase, PlaceWithHirId}; +use rustc_middle::ty; +use std::collections::BTreeMap; +use std::fmt::Debug; + +mod cfg_build; +mod cfg_propagate; +mod cfg_visualize; +mod record_consumed_borrow; + +pub fn compute_drop_ranges<'a, 'tcx>( + fcx: &'a FnCtxt<'a, 'tcx>, + def_id: DefId, + body: &'tcx Body<'tcx>, +) -> DropRanges { + 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); + let mut drop_ranges = build_control_flow_graph( + fcx.tcx.hir(), + fcx.tcx, + &fcx.typeck_results.borrow(), + consumed_borrowed_places, + body, + num_exprs, + ); + + drop_ranges.propagate_to_fixpoint(); + + DropRanges { tracked_value_map: drop_ranges.tracked_value_map, nodes: drop_ranges.nodes } +} + +/// Applies `f` to consumable node in the HIR subtree pointed to by `place`. +/// +/// This includes the place itself, and if the place is a reference to a local +/// variable then `f` is also called on the HIR node for that variable as well. +/// +/// For example, if `place` points to `foo()`, then `f` is called once for the +/// result of `foo`. On the other hand, if `place` points to `x` then `f` will +/// be called both on the `ExprKind::Path` node that represents the expression +/// as well as the HirId of the local `x` itself. +fn for_each_consumable<'tcx>(hir: Map<'tcx>, place: TrackedValue, mut f: impl FnMut(TrackedValue)) { + f(place); + let node = hir.find(place.hir_id()); + if let Some(Node::Expr(expr)) = node { + match expr.kind { + hir::ExprKind::Path(hir::QPath::Resolved( + _, + hir::Path { res: hir::def::Res::Local(hir_id), .. }, + )) => { + f(TrackedValue::Variable(*hir_id)); + } + _ => (), + } + } +} + +rustc_index::newtype_index! { + pub struct PostOrderId { + DEBUG_FORMAT = "id({})", + } +} + +rustc_index::newtype_index! { + pub struct TrackedValueIndex { + DEBUG_FORMAT = "hidx({})", + } +} + +/// Identifies a value whose drop state we need to track. +#[derive(PartialEq, Eq, Hash, Debug, Clone, Copy)] +enum TrackedValue { + /// Represents a named variable, such as a let binding, parameter, or upvar. + /// + /// The HirId points to the variable's definition site. + Variable(HirId), + /// A value produced as a result of an expression. + /// + /// The HirId points to the expression that returns this value. + Temporary(HirId), +} + +impl TrackedValue { + fn hir_id(&self) -> HirId { + match self { + TrackedValue::Variable(hir_id) | TrackedValue::Temporary(hir_id) => *hir_id, + } + } +} + +/// Represents a reason why we might not be able to convert a HirId or Place +/// into a tracked value. +#[derive(Debug)] +enum TrackedValueConversionError { + /// Place projects are not currently supported. + /// + /// The reasoning around these is kind of subtle, so we choose to be more + /// conservative around these for now. There is not reason in theory we + /// cannot support these, we just have not implemented it yet. + PlaceProjectionsNotSupported, +} + +impl TryFrom<&PlaceWithHirId<'_>> for TrackedValue { + type Error = TrackedValueConversionError; + + fn try_from(place_with_id: &PlaceWithHirId<'_>) -> Result { + if !place_with_id.place.projections.is_empty() { + debug!( + "TrackedValue from PlaceWithHirId: {:?} has projections, which are not supported.", + place_with_id + ); + 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)) + } + } + } +} + +pub struct DropRanges { + tracked_value_map: FxHashMap, + nodes: IndexVec, +} + +impl DropRanges { + pub fn is_dropped_at(&self, hir_id: HirId, location: usize) -> bool { + self.tracked_value_map + .get(&TrackedValue::Temporary(hir_id)) + .or(self.tracked_value_map.get(&TrackedValue::Variable(hir_id))) + .cloned() + .map_or(false, |tracked_value_id| { + self.expect_node(location.into()).drop_state.contains(tracked_value_id) + }) + } + + /// Returns a reference to the NodeInfo for a node, panicking if it does not exist + fn expect_node(&self, id: PostOrderId) -> &NodeInfo { + &self.nodes[id] + } +} + +/// Tracks information needed to compute drop ranges. +struct DropRangesBuilder { + /// The core of DropRangesBuilder is a set of nodes, which each represent + /// one expression. We primarily refer to them by their index in a + /// post-order traversal of the HIR tree, since this is what + /// generator_interior uses to talk about yield positions. + /// + /// This IndexVec keeps the relevant details for each node. See the + /// NodeInfo struct for more details, but this information includes things + /// such as the set of control-flow successors, which variables are dropped + /// or reinitialized, and whether each variable has been inferred to be + /// known-dropped or potentially reintiialized at each point. + nodes: IndexVec, + /// We refer to values whose drop state we are tracking by the HirId of + /// where they are defined. Within a NodeInfo, however, we store the + /// drop-state in a bit vector indexed by a HirIdIndex + /// (see NodeInfo::drop_state). The hir_id_map field stores the mapping + /// from HirIds to the HirIdIndex that is used to represent that value in + /// bitvector. + tracked_value_map: FxHashMap, + + /// When building the control flow graph, we don't always know the + /// post-order index of the target node at the point we encounter it. + /// For example, this happens with break and continue. In those cases, + /// we store a pair of the PostOrderId of the source and the HirId + /// of the target. Once we have gathered all of these edges, we make a + /// pass over the set of deferred edges (see process_deferred_edges in + /// cfg_build.rs), look up the PostOrderId for the target (since now the + /// post-order index for all nodes is known), and add missing control flow + /// edges. + deferred_edges: Vec<(PostOrderId, HirId)>, + /// This maps HirIds of expressions to their post-order index. It is + /// used in process_deferred_edges to correctly add back-edges. + post_order_map: HirIdMap, +} + +impl Debug for DropRangesBuilder { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DropRanges") + .field("hir_id_map", &self.tracked_value_map) + .field("post_order_maps", &self.post_order_map) + .field("nodes", &self.nodes.iter_enumerated().collect::>()) + .finish() + } +} + +/// DropRanges keeps track of what values are definitely dropped at each point in the code. +/// +/// Values of interest are defined by the hir_id of their place. Locations in code are identified +/// by their index in the post-order traversal. At its core, DropRanges maps +/// (hir_id, post_order_id) -> bool, where a true value indicates that the value is definitely +/// dropped at the point of the node identified by post_order_id. +impl DropRangesBuilder { + /// Returns the number of values (hir_ids) that are tracked + fn num_values(&self) -> usize { + self.tracked_value_map.len() + } + + fn node_mut(&mut self, id: PostOrderId) -> &mut NodeInfo { + let size = self.num_values(); + self.nodes.ensure_contains_elem(id, || NodeInfo::new(size)); + &mut self.nodes[id] + } + + fn add_control_edge(&mut self, from: PostOrderId, to: PostOrderId) { + trace!("adding control edge from {:?} to {:?}", from, to); + self.node_mut(from.into()).successors.push(to.into()); + } +} + +#[derive(Debug)] +struct NodeInfo { + /// IDs of nodes that can follow this one in the control flow + /// + /// If the vec is empty, then control proceeds to the next node. + successors: Vec, + + /// List of hir_ids that are dropped by this node. + drops: Vec, + + /// List of hir_ids that are reinitialized by this node. + reinits: Vec, + + /// Set of values that are definitely dropped at this point. + drop_state: BitSet, +} + +impl NodeInfo { + fn new(num_values: usize) -> Self { + Self { + successors: vec![], + drops: vec![], + reinits: vec![], + drop_state: BitSet::new_filled(num_values), + } + } +} diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs new file mode 100644 index 0000000000000..fc957b899909d --- /dev/null +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs @@ -0,0 +1,473 @@ +use super::{ + for_each_consumable, record_consumed_borrow::ConsumedAndBorrowedPlaces, DropRangesBuilder, + NodeInfo, PostOrderId, TrackedValue, TrackedValueIndex, +}; +use hir::{ + intravisit::{self, Visitor}, + Body, Expr, ExprKind, Guard, HirId, +}; +use rustc_data_structures::fx::FxHashMap; +use rustc_hir as hir; +use rustc_index::vec::IndexVec; +use rustc_middle::{ + hir::map::Map, + ty::{TyCtxt, TypeckResults}, +}; +use std::mem::swap; + +/// Traverses the body to find the control flow graph and locations for the +/// relevant places are dropped or reinitialized. +/// +/// The resulting structure still needs to be iterated to a fixed point, which +/// can be done with propagate_to_fixpoint in cfg_propagate. +pub(super) fn build_control_flow_graph<'tcx>( + hir: Map<'tcx>, + tcx: TyCtxt<'tcx>, + typeck_results: &TypeckResults<'tcx>, + consumed_borrowed_places: ConsumedAndBorrowedPlaces, + body: &'tcx Body<'tcx>, + num_exprs: usize, +) -> DropRangesBuilder { + let mut drop_range_visitor = + DropRangeVisitor::new(hir, tcx, typeck_results, consumed_borrowed_places, num_exprs); + intravisit::walk_body(&mut drop_range_visitor, body); + + drop_range_visitor.drop_ranges.process_deferred_edges(); + + drop_range_visitor.drop_ranges +} + +/// This struct is used to gather the information for `DropRanges` to determine the regions of the +/// HIR tree for which a value is dropped. +/// +/// We are interested in points where a variables is dropped or initialized, and the control flow +/// of the code. We identify locations in code by their post-order traversal index, so it is +/// important for this traversal to match that in `RegionResolutionVisitor` and `InteriorVisitor`. +/// +/// We make several simplifying assumptions, with the goal of being more conservative than +/// necessary rather than less conservative (since being less conservative is unsound, but more +/// conservative is still safe). These assumptions are: +/// +/// 1. Moving a variable `a` counts as a move of the whole variable. +/// 2. Moving a partial path like `a.b.c` is ignored. +/// 3. Reinitializing through a field (e.g. `a.b.c = 5`) counds as a reinitialization of all of +/// `a`. +/// +/// Some examples: +/// +/// Rule 1: +/// ```rust +/// let mut a = (vec![0], vec![0]); +/// drop(a); +/// // `a` is not considered initialized. +/// ``` +/// +/// Rule 2: +/// ```rust +/// let mut a = (vec![0], vec![0]); +/// drop(a.0); +/// drop(a.1); +/// // `a` is still considered initialized. +/// ``` +/// +/// Rule 3: +/// ```rust +/// let mut a = (vec![0], vec![0]); +/// drop(a); +/// a.1 = vec![1]; +/// // all of `a` is considered initialized +/// ``` + +struct DropRangeVisitor<'a, 'tcx> { + hir: Map<'tcx>, + places: ConsumedAndBorrowedPlaces, + drop_ranges: DropRangesBuilder, + expr_index: PostOrderId, + tcx: TyCtxt<'tcx>, + typeck_results: &'a TypeckResults<'tcx>, +} + +impl<'a, 'tcx> DropRangeVisitor<'a, 'tcx> { + fn new( + hir: Map<'tcx>, + tcx: TyCtxt<'tcx>, + typeck_results: &'a TypeckResults<'tcx>, + places: ConsumedAndBorrowedPlaces, + num_exprs: usize, + ) -> Self { + debug!("consumed_places: {:?}", places.consumed); + let drop_ranges = DropRangesBuilder::new( + places.consumed.iter().flat_map(|(_, places)| places.iter().cloned()), + hir, + num_exprs, + ); + Self { hir, places, drop_ranges, expr_index: PostOrderId::from_u32(0), typeck_results, tcx } + } + + fn record_drop(&mut self, value: TrackedValue) { + if self.places.borrowed.contains(&value) { + debug!("not marking {:?} as dropped because it is borrowed at some point", value); + } else { + debug!("marking {:?} as dropped at {:?}", value, self.expr_index); + let count = self.expr_index; + self.drop_ranges.drop_at(value, count); + } + } + + /// ExprUseVisitor's consume callback doesn't go deep enough for our purposes in all + /// expressions. This method consumes a little deeper into the expression when needed. + fn consume_expr(&mut self, expr: &hir::Expr<'_>) { + debug!("consuming expr {:?}, count={:?}", expr.hir_id, self.expr_index); + let places = self + .places + .consumed + .get(&expr.hir_id) + .map_or(vec![], |places| places.iter().cloned().collect()); + for place in places { + for_each_consumable(self.hir, place, |value| self.record_drop(value)); + } + } + + /// Marks an expression as being reinitialized. + /// + /// Note that we always approximated on the side of things being more + /// initialized than they actually are, as opposed to less. In cases such + /// as `x.y = ...`, we would consider all of `x` as being initialized + /// instead of just the `y` field. + /// + /// This is because it is always safe to consider something initialized + /// even when it is not, but the other way around will cause problems. + /// + /// In the future, we will hopefully tighten up these rules to be more + /// precise. + fn reinit_expr(&mut self, expr: &hir::Expr<'_>) { + // Walk the expression to find the base. For example, in an expression + // like `*a[i].x`, we want to find the `a` and mark that as + // reinitialized. + match expr.kind { + ExprKind::Path(hir::QPath::Resolved( + _, + hir::Path { res: hir::def::Res::Local(hir_id), .. }, + )) => { + // This is the base case, where we have found an actual named variable. + + let location = self.expr_index; + debug!("reinitializing {:?} at {:?}", hir_id, location); + self.drop_ranges.reinit_at(TrackedValue::Variable(*hir_id), location); + } + + ExprKind::Field(base, _) => self.reinit_expr(base), + + // Most expressions do not refer to something where we need to track + // reinitializations. + // + // Some of these may be interesting in the future + ExprKind::Path(..) + | ExprKind::Box(..) + | ExprKind::ConstBlock(..) + | ExprKind::Array(..) + | ExprKind::Call(..) + | ExprKind::MethodCall(..) + | ExprKind::Tup(..) + | ExprKind::Binary(..) + | ExprKind::Unary(..) + | ExprKind::Lit(..) + | ExprKind::Cast(..) + | ExprKind::Type(..) + | ExprKind::DropTemps(..) + | ExprKind::Let(..) + | ExprKind::If(..) + | ExprKind::Loop(..) + | ExprKind::Match(..) + | ExprKind::Closure(..) + | ExprKind::Block(..) + | ExprKind::Assign(..) + | ExprKind::AssignOp(..) + | ExprKind::Index(..) + | ExprKind::AddrOf(..) + | ExprKind::Break(..) + | ExprKind::Continue(..) + | ExprKind::Ret(..) + | ExprKind::InlineAsm(..) + | ExprKind::Struct(..) + | ExprKind::Repeat(..) + | ExprKind::Yield(..) + | ExprKind::Err => (), + } + } + + /// For an expression with an uninhabited return type (e.g. a function that returns !), + /// this adds a self edge to to the CFG to model the fact that the function does not + /// return. + fn handle_uninhabited_return(&mut self, expr: &Expr<'tcx>) { + let ty = self.typeck_results.expr_ty(expr); + let ty = self.tcx.erase_regions(ty); + let m = self.tcx.parent_module(expr.hir_id).to_def_id(); + let param_env = self.tcx.param_env(m.expect_local()); + if self.tcx.is_ty_uninhabited_from(m, ty, param_env) { + // This function will not return. We model this fact as an infinite loop. + self.drop_ranges.add_control_edge(self.expr_index + 1, self.expr_index + 1); + } + } +} + +impl<'a, 'tcx> Visitor<'tcx> for DropRangeVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + let mut reinit = None; + match expr.kind { + ExprKind::Assign(lhs, rhs, _) => { + self.visit_expr(lhs); + self.visit_expr(rhs); + + reinit = Some(lhs); + } + + ExprKind::If(test, if_true, if_false) => { + self.visit_expr(test); + + let fork = self.expr_index; + + self.drop_ranges.add_control_edge(fork, self.expr_index + 1); + self.visit_expr(if_true); + let true_end = self.expr_index; + + self.drop_ranges.add_control_edge(fork, self.expr_index + 1); + if let Some(if_false) = if_false { + self.visit_expr(if_false); + } + + self.drop_ranges.add_control_edge(true_end, self.expr_index + 1); + } + ExprKind::Match(scrutinee, arms, ..) => { + // We walk through the match expression almost like a chain of if expressions. + // Here's a diagram to follow along with: + // + // ┌─┐ + // match │A│ { + // ┌───┴─┘ + // │ + // ┌▼┌───►┌─┐ ┌─┐ + // │B│ if │C│ =>│D│, + // └─┘ ├─┴──►└─┴──────┐ + // ┌──┘ │ + // ┌──┘ │ + // │ │ + // ┌▼┌───►┌─┐ ┌─┐ │ + // │E│ if │F│ =>│G│, │ + // └─┘ ├─┴──►└─┴┐ │ + // │ │ │ + // } ▼ ▼ │ + // ┌─┐◄───────────────────┘ + // │H│ + // └─┘ + // + // The order we want is that the scrutinee (A) flows into the first pattern (B), + // which flows into the guard (C). Then the guard either flows into the arm body + // (D) or into the start of the next arm (E). Finally, the body flows to the end + // of the match block (H). + // + // The subsequent arms follow the same ordering. First we go to the pattern, then + // the guard (if present, otherwise it flows straight into the body), then into + // the body and then to the end of the match expression. + // + // The comments below show which edge is being added. + self.visit_expr(scrutinee); + + let (guard_exit, arm_end_ids) = arms.iter().fold( + (self.expr_index, vec![]), + |(incoming_edge, mut arm_end_ids), hir::Arm { pat, body, guard, .. }| { + // A -> B, or C -> E + self.drop_ranges.add_control_edge(incoming_edge, self.expr_index + 1); + self.visit_pat(pat); + // B -> C and E -> F are added implicitly due to the traversal order. + match guard { + Some(Guard::If(expr)) => self.visit_expr(expr), + Some(Guard::IfLet(pat, expr)) => { + self.visit_pat(pat); + self.visit_expr(expr); + } + None => (), + } + // Likewise, C -> D and F -> G are added implicitly. + + // Save C, F, so we can add the other outgoing edge. + let to_next_arm = self.expr_index; + + // The default edge does not get added since we also have an explicit edge, + // so we also need to add an edge to the next node as well. + // + // This adds C -> D, F -> G + self.drop_ranges.add_control_edge(self.expr_index, self.expr_index + 1); + self.visit_expr(body); + + // Save the end of the body so we can add the exit edge once we know where + // the exit is. + arm_end_ids.push(self.expr_index); + + // Pass C to the next iteration, as well as vec![D] + // + // On the last round through, we pass F and vec![D, G] so that we can + // add all the exit edges. + (to_next_arm, arm_end_ids) + }, + ); + // F -> H + self.drop_ranges.add_control_edge(guard_exit, self.expr_index + 1); + + arm_end_ids.into_iter().for_each(|arm_end| { + // D -> H, G -> H + self.drop_ranges.add_control_edge(arm_end, self.expr_index + 1) + }); + } + + ExprKind::Loop(body, ..) => { + let loop_begin = self.expr_index + 1; + if body.stmts.is_empty() && body.expr.is_none() { + // For empty loops we won't have updated self.expr_index after visiting the + // body, meaning we'd get an edge from expr_index to expr_index + 1, but + // instead we want an edge from expr_index + 1 to expr_index + 1. + self.drop_ranges.add_control_edge(loop_begin, loop_begin); + } else { + self.visit_block(body); + self.drop_ranges.add_control_edge(self.expr_index, loop_begin); + } + } + ExprKind::Break(hir::Destination { target_id: Ok(target), .. }, ..) + | ExprKind::Continue(hir::Destination { target_id: Ok(target), .. }, ..) => { + self.drop_ranges.add_control_edge_hir_id(self.expr_index, target); + } + + ExprKind::Call(f, args) => { + self.visit_expr(f); + for arg in args { + self.visit_expr(arg); + } + + self.handle_uninhabited_return(expr); + } + ExprKind::MethodCall(_, _, exprs, _) => { + for expr in exprs { + self.visit_expr(expr); + } + + self.handle_uninhabited_return(expr); + } + + ExprKind::AddrOf(..) + | ExprKind::Array(..) + | ExprKind::AssignOp(..) + | ExprKind::Binary(..) + | ExprKind::Block(..) + | ExprKind::Box(..) + | ExprKind::Break(..) + | ExprKind::Cast(..) + | ExprKind::Closure(..) + | ExprKind::ConstBlock(..) + | ExprKind::Continue(..) + | ExprKind::DropTemps(..) + | ExprKind::Err + | ExprKind::Field(..) + | ExprKind::Index(..) + | ExprKind::InlineAsm(..) + | ExprKind::Let(..) + | ExprKind::Lit(..) + | ExprKind::Path(..) + | ExprKind::Repeat(..) + | ExprKind::Ret(..) + | ExprKind::Struct(..) + | ExprKind::Tup(..) + | ExprKind::Type(..) + | ExprKind::Unary(..) + | ExprKind::Yield(..) => intravisit::walk_expr(self, expr), + } + + self.expr_index = self.expr_index + 1; + self.drop_ranges.add_node_mapping(expr.hir_id, self.expr_index); + self.consume_expr(expr); + if let Some(expr) = reinit { + self.reinit_expr(expr); + } + } + + fn visit_pat(&mut self, pat: &'tcx hir::Pat<'tcx>) { + intravisit::walk_pat(self, pat); + + // Increment expr_count here to match what InteriorVisitor expects. + self.expr_index = self.expr_index + 1; + } +} + +impl DropRangesBuilder { + fn new( + tracked_values: impl Iterator, + hir: Map<'_>, + num_exprs: usize, + ) -> Self { + let mut tracked_value_map = FxHashMap::<_, TrackedValueIndex>::default(); + let mut next = <_>::from(0u32); + for value in tracked_values { + for_each_consumable(hir, value, |value| { + if !tracked_value_map.contains_key(&value) { + tracked_value_map.insert(value, next); + next = next + 1; + } + }); + } + debug!("hir_id_map: {:?}", tracked_value_map); + let num_values = tracked_value_map.len(); + Self { + tracked_value_map, + nodes: IndexVec::from_fn_n(|_| NodeInfo::new(num_values), num_exprs + 1), + deferred_edges: <_>::default(), + post_order_map: <_>::default(), + } + } + + fn tracked_value_index(&self, tracked_value: TrackedValue) -> TrackedValueIndex { + *self.tracked_value_map.get(&tracked_value).unwrap() + } + + /// Adds an entry in the mapping from HirIds to PostOrderIds + /// + /// Needed so that `add_control_edge_hir_id` can work. + fn add_node_mapping(&mut self, node_hir_id: HirId, post_order_id: PostOrderId) { + self.post_order_map.insert(node_hir_id, post_order_id); + } + + /// Like add_control_edge, but uses a hir_id as the target. + /// + /// This can be used for branches where we do not know the PostOrderId of the target yet, + /// such as when handling `break` or `continue`. + fn add_control_edge_hir_id(&mut self, from: PostOrderId, to: HirId) { + self.deferred_edges.push((from, to)); + } + + fn drop_at(&mut self, value: TrackedValue, location: PostOrderId) { + let value = self.tracked_value_index(value); + self.node_mut(location.into()).drops.push(value); + } + + fn reinit_at(&mut self, value: TrackedValue, location: PostOrderId) { + let value = match self.tracked_value_map.get(&value) { + Some(value) => *value, + // If there's no value, this is never consumed and therefore is never dropped. We can + // ignore this. + None => return, + }; + self.node_mut(location.into()).reinits.push(value); + } + + /// Looks up PostOrderId for any control edges added by HirId and adds a proper edge for them. + /// + /// Should be called after visiting the HIR but before solving the control flow, otherwise some + /// edges will be missed. + fn process_deferred_edges(&mut self) { + let mut edges = vec![]; + swap(&mut edges, &mut self.deferred_edges); + edges.into_iter().for_each(|(from, to)| { + let to = *self.post_order_map.get(&to).expect("Expression ID not found"); + trace!("Adding deferred edge from {:?} to {:?}", from, to); + self.add_control_edge(from, to) + }); + } +} diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_propagate.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_propagate.rs new file mode 100644 index 0000000000000..139d17d2e1ca1 --- /dev/null +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_propagate.rs @@ -0,0 +1,92 @@ +use super::{DropRangesBuilder, PostOrderId}; +use rustc_index::{bit_set::BitSet, vec::IndexVec}; +use std::collections::BTreeMap; + +impl DropRangesBuilder { + pub fn propagate_to_fixpoint(&mut self) { + trace!("before fixpoint: {:#?}", self); + let preds = self.compute_predecessors(); + + trace!("predecessors: {:#?}", preds.iter_enumerated().collect::>()); + + let mut new_state = BitSet::new_empty(self.num_values()); + let mut changed_nodes = BitSet::new_empty(self.nodes.len()); + let mut unchanged_mask = BitSet::new_filled(self.nodes.len()); + changed_nodes.insert(0u32.into()); + + let mut propagate = || { + let mut changed = false; + unchanged_mask.insert_all(); + for id in self.nodes.indices() { + trace!("processing {:?}, changed_nodes: {:?}", id, changed_nodes); + // Check if any predecessor has changed, and if not then short-circuit. + // + // We handle the start node specially, since it doesn't have any predecessors, + // but we need to start somewhere. + if match id.index() { + 0 => !changed_nodes.contains(id), + _ => !preds[id].iter().any(|pred| changed_nodes.contains(*pred)), + } { + trace!("short-circuiting because none of {:?} have changed", preds[id]); + unchanged_mask.remove(id); + continue; + } + + if id.index() == 0 { + new_state.clear(); + } else { + // If we are not the start node and we have no predecessors, treat + // everything as dropped because there's no way to get here anyway. + new_state.insert_all(); + }; + + for pred in &preds[id] { + new_state.intersect(&self.nodes[*pred].drop_state); + } + + for drop in &self.nodes[id].drops { + new_state.insert(*drop); + } + + for reinit in &self.nodes[id].reinits { + new_state.remove(*reinit); + } + + if self.nodes[id].drop_state.intersect(&new_state) { + changed_nodes.insert(id); + changed = true; + } else { + unchanged_mask.remove(id); + } + } + + changed_nodes.intersect(&unchanged_mask); + changed + }; + + while propagate() { + trace!("drop_state changed, re-running propagation"); + } + + trace!("after fixpoint: {:#?}", self); + } + + fn compute_predecessors(&self) -> IndexVec> { + let mut preds = IndexVec::from_fn_n(|_| vec![], self.nodes.len()); + for (id, node) in self.nodes.iter_enumerated() { + // If the node has no explicit successors, we assume that control + // will from this node into the next one. + // + // If there are successors listed, then we assume that all + // possible successors are given and we do not include the default. + if node.successors.len() == 0 && id.index() != self.nodes.len() - 1 { + preds[id + 1].push(id); + } else { + for succ in &node.successors { + preds[*succ].push(id); + } + } + } + preds + } +} diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs new file mode 100644 index 0000000000000..20aad7aedf775 --- /dev/null +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs @@ -0,0 +1,77 @@ +//! Implementation of GraphWalk for DropRanges so we can visualize the control +//! flow graph when needed for debugging. + +use rustc_graphviz as dot; + +use super::{DropRangesBuilder, PostOrderId}; + +/// Writes the CFG for DropRangesBuilder to a .dot file for visualization. +/// +/// It is not normally called, but is kept around to easily add debugging +/// code when needed. +#[allow(dead_code)] +pub(super) fn write_graph_to_file(drop_ranges: &DropRangesBuilder, filename: &str) { + dot::render(drop_ranges, &mut std::fs::File::create(filename).unwrap()).unwrap(); +} + +impl<'a> dot::GraphWalk<'a> for DropRangesBuilder { + type Node = PostOrderId; + + type Edge = (PostOrderId, PostOrderId); + + fn nodes(&'a self) -> dot::Nodes<'a, Self::Node> { + self.nodes.iter_enumerated().map(|(i, _)| i).collect() + } + + fn edges(&'a self) -> dot::Edges<'a, Self::Edge> { + self.nodes + .iter_enumerated() + .flat_map(|(i, node)| { + if node.successors.len() == 0 { + vec![(i, i + 1)] + } else { + node.successors.iter().map(move |&s| (i, s)).collect() + } + }) + .collect() + } + + fn source(&'a self, edge: &Self::Edge) -> Self::Node { + edge.0 + } + + fn target(&'a self, edge: &Self::Edge) -> Self::Node { + edge.1 + } +} + +impl<'a> dot::Labeller<'a> for DropRangesBuilder { + type Node = PostOrderId; + + type Edge = (PostOrderId, PostOrderId); + + fn graph_id(&'a self) -> dot::Id<'a> { + dot::Id::new("drop_ranges").unwrap() + } + + fn node_id(&'a self, n: &Self::Node) -> dot::Id<'a> { + dot::Id::new(format!("id{}", n.index())).unwrap() + } + + fn node_label(&'a self, n: &Self::Node) -> dot::LabelText<'a> { + dot::LabelText::LabelStr( + format!( + "{:?}, local_id: {}", + n, + self.post_order_map + .iter() + .find(|(_hir_id, &post_order_id)| post_order_id == *n) + .map_or("".into(), |(hir_id, _)| format!( + "{}", + hir_id.local_id.index() + )) + ) + .into(), + ) + } +} diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs new file mode 100644 index 0000000000000..059a135a6fb65 --- /dev/null +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs @@ -0,0 +1,118 @@ +use super::TrackedValue; +use crate::{ + check::FnCtxt, + expr_use_visitor::{self, ExprUseVisitor}, +}; +use hir::{def_id::DefId, Body, HirId, HirIdMap}; +use rustc_data_structures::stable_set::FxHashSet; +use rustc_hir as hir; +use rustc_middle::hir::map::Map; + +pub(super) fn find_consumed_and_borrowed<'a, 'tcx>( + fcx: &'a FnCtxt<'a, 'tcx>, + def_id: DefId, + body: &'tcx Body<'tcx>, +) -> ConsumedAndBorrowedPlaces { + let mut expr_use_visitor = ExprUseDelegate::new(fcx.tcx.hir()); + expr_use_visitor.consume_body(fcx, def_id, body); + expr_use_visitor.places +} + +pub(super) struct ConsumedAndBorrowedPlaces { + /// Records the variables/expressions that are dropped by a given expression. + /// + /// The key is the hir-id of the expression, and the value is a set or hir-ids for variables + /// or values that are consumed by that expression. + /// + /// Note that this set excludes "partial drops" -- for example, a statement like `drop(x.y)` is + /// not considered a drop of `x`, although it would be a drop of `x.y`. + pub(super) consumed: HirIdMap>, + /// A set of hir-ids of values or variables that are borrowed at some point within the body. + pub(super) borrowed: FxHashSet, +} + +/// Works with ExprUseVisitor to find interesting values for the drop range analysis. +/// +/// Interesting values are those that are either dropped or borrowed. For dropped values, we also +/// record the parent expression, which is the point where the drop actually takes place. +struct ExprUseDelegate<'tcx> { + hir: Map<'tcx>, + places: ConsumedAndBorrowedPlaces, +} + +impl<'tcx> ExprUseDelegate<'tcx> { + fn new(hir: Map<'tcx>) -> Self { + Self { + hir, + places: ConsumedAndBorrowedPlaces { + consumed: <_>::default(), + borrowed: <_>::default(), + }, + } + } + + fn consume_body(&mut self, fcx: &'_ FnCtxt<'_, 'tcx>, def_id: DefId, body: &'tcx Body<'tcx>) { + // Run ExprUseVisitor to find where values are consumed. + ExprUseVisitor::new( + self, + &fcx.infcx, + def_id.expect_local(), + fcx.param_env, + &fcx.typeck_results.borrow(), + ) + .consume_body(body); + } + + fn mark_consumed(&mut self, consumer: HirId, target: TrackedValue) { + if !self.places.consumed.contains_key(&consumer) { + self.places.consumed.insert(consumer, <_>::default()); + } + self.places.consumed.get_mut(&consumer).map(|places| places.insert(target)); + } +} + +impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> { + fn consume( + &mut self, + place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>, + diag_expr_id: HirId, + ) { + let parent = match self.hir.find_parent_node(place_with_id.hir_id) { + Some(parent) => parent, + None => place_with_id.hir_id, + }; + debug!( + "consume {:?}; diag_expr_id={:?}, using parent {:?}", + place_with_id, diag_expr_id, parent + ); + place_with_id + .try_into() + .map_or((), |tracked_value| self.mark_consumed(parent, tracked_value)); + } + + fn borrow( + &mut self, + place_with_id: &expr_use_visitor::PlaceWithHirId<'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)); + } + + fn mutate( + &mut self, + _assignee_place: &expr_use_visitor::PlaceWithHirId<'tcx>, + _diag_expr_id: HirId, + ) { + } + + fn fake_read( + &mut self, + _place: expr_use_visitor::Place<'tcx>, + _cause: rustc_middle::mir::FakeReadCause, + _diag_expr_id: HirId, + ) { + } +} diff --git a/src/test/ui/async-await/async-fn-nonsend.rs b/src/test/ui/async-await/async-fn-nonsend.rs index 845941200fc95..c5453b67ef5b6 100644 --- a/src/test/ui/async-await/async-fn-nonsend.rs +++ b/src/test/ui/async-await/async-fn-nonsend.rs @@ -18,7 +18,7 @@ async fn fut() {} async fn fut_arg(_: T) {} async fn local_dropped_before_await() { - // FIXME: it'd be nice for this to be allowed in a `Send` `async fn` + // this is okay now because of the drop let x = non_send(); drop(x); fut().await; @@ -35,21 +35,40 @@ async fn non_send_temporary_in_match() { } } +fn get_formatter() -> std::fmt::Formatter<'static> { + panic!() +} + async fn non_sync_with_method_call() { - // FIXME: it'd be nice for this to work. + let f: &mut std::fmt::Formatter = &mut get_formatter(); + // It would by nice for this to work. + if non_sync().fmt(f).unwrap() == () { + fut().await; + } +} + +async fn non_sync_with_method_call_panic() { let f: &mut std::fmt::Formatter = panic!(); if non_sync().fmt(f).unwrap() == () { fut().await; } } +async fn non_sync_with_method_call_infinite_loop() { + let f: &mut std::fmt::Formatter = loop {}; + if non_sync().fmt(f).unwrap() == () { + fut().await; + } +} + fn assert_send(_: impl Send) {} pub fn pass_assert() { assert_send(local_dropped_before_await()); - //~^ ERROR future cannot be sent between threads safely assert_send(non_send_temporary_in_match()); //~^ ERROR future cannot be sent between threads safely assert_send(non_sync_with_method_call()); //~^ ERROR future cannot be sent between threads safely + assert_send(non_sync_with_method_call_panic()); + assert_send(non_sync_with_method_call_infinite_loop()); } diff --git a/src/test/ui/async-await/async-fn-nonsend.stderr b/src/test/ui/async-await/async-fn-nonsend.stderr index bff282085735c..40ad46b48620d 100644 --- a/src/test/ui/async-await/async-fn-nonsend.stderr +++ b/src/test/ui/async-await/async-fn-nonsend.stderr @@ -1,28 +1,5 @@ error: future cannot be sent between threads safely - --> $DIR/async-fn-nonsend.rs:49:17 - | -LL | assert_send(local_dropped_before_await()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `local_dropped_before_await` is not `Send` - | - = help: within `impl Future`, the trait `Send` is not implemented for `Rc<()>` -note: future is not `Send` as this value is used across an await - --> $DIR/async-fn-nonsend.rs:24:10 - | -LL | let x = non_send(); - | - has type `impl Debug` which is not `Send` -LL | drop(x); -LL | fut().await; - | ^^^^^^ await occurs here, with `x` maybe used later -LL | } - | - `x` is later dropped here -note: required by a bound in `assert_send` - --> $DIR/async-fn-nonsend.rs:46:24 - | -LL | fn assert_send(_: impl Send) {} - | ^^^^ required by this bound in `assert_send` - -error: future cannot be sent between threads safely - --> $DIR/async-fn-nonsend.rs:51:17 + --> $DIR/async-fn-nonsend.rs:68:17 | LL | assert_send(non_send_temporary_in_match()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `non_send_temporary_in_match` is not `Send` @@ -32,41 +9,41 @@ note: future is not `Send` as this value is used across an await --> $DIR/async-fn-nonsend.rs:33:25 | LL | match Some(non_send()) { - | ---------- has type `impl Debug` which is not `Send` + | ---------------- has type `Option` which is not `Send` LL | Some(_) => fut().await, - | ^^^^^^ await occurs here, with `non_send()` maybe used later + | ^^^^^^ await occurs here, with `Some(non_send())` maybe used later ... LL | } - | - `non_send()` is later dropped here + | - `Some(non_send())` is later dropped here note: required by a bound in `assert_send` - --> $DIR/async-fn-nonsend.rs:46:24 + --> $DIR/async-fn-nonsend.rs:64:24 | LL | fn assert_send(_: impl Send) {} | ^^^^ required by this bound in `assert_send` error: future cannot be sent between threads safely - --> $DIR/async-fn-nonsend.rs:53:17 + --> $DIR/async-fn-nonsend.rs:70:17 | LL | assert_send(non_sync_with_method_call()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send` | = help: the trait `Send` is not implemented for `dyn std::fmt::Write` note: future is not `Send` as this value is used across an await - --> $DIR/async-fn-nonsend.rs:42:14 + --> $DIR/async-fn-nonsend.rs:46:14 | -LL | let f: &mut std::fmt::Formatter = panic!(); - | - has type `&mut Formatter<'_>` which is not `Send` -LL | if non_sync().fmt(f).unwrap() == () { +LL | let f: &mut std::fmt::Formatter = &mut get_formatter(); + | --------------- has type `Formatter<'_>` which is not `Send` +... LL | fut().await; - | ^^^^^^ await occurs here, with `f` maybe used later + | ^^^^^^ await occurs here, with `get_formatter()` maybe used later LL | } LL | } - | - `f` is later dropped here + | - `get_formatter()` is later dropped here note: required by a bound in `assert_send` - --> $DIR/async-fn-nonsend.rs:46:24 + --> $DIR/async-fn-nonsend.rs:64:24 | LL | fn assert_send(_: impl Send) {} | ^^^^ required by this bound in `assert_send` -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors diff --git a/src/test/ui/async-await/partial-drop-partial-reinit.rs b/src/test/ui/async-await/partial-drop-partial-reinit.rs new file mode 100644 index 0000000000000..73f0ca8153cb9 --- /dev/null +++ b/src/test/ui/async-await/partial-drop-partial-reinit.rs @@ -0,0 +1,29 @@ +// edition:2021 +#![feature(negative_impls)] +#![allow(unused)] + +fn main() { + gimme_send(foo()); + //~^ ERROR cannot be sent between threads safely +} + +fn gimme_send(t: T) { + drop(t); +} + +struct NotSend {} + +impl Drop for NotSend { + fn drop(&mut self) {} +} + +impl !Send for NotSend {} + +async fn foo() { + let mut x = (NotSend {},); + drop(x.0); + x.0 = NotSend {}; + bar().await; +} + +async fn bar() {} diff --git a/src/test/ui/async-await/partial-drop-partial-reinit.stderr b/src/test/ui/async-await/partial-drop-partial-reinit.stderr new file mode 100644 index 0000000000000..2097642eb24ab --- /dev/null +++ b/src/test/ui/async-await/partial-drop-partial-reinit.stderr @@ -0,0 +1,27 @@ +error[E0277]: `NotSend` cannot be sent between threads safely + --> $DIR/partial-drop-partial-reinit.rs:6:16 + | +LL | gimme_send(foo()); + | ---------- ^^^^^ `NotSend` cannot be sent between threads safely + | | + | required by a bound introduced by this call +... +LL | async fn foo() { + | - within this `impl Future` + | + = help: within `impl Future`, the trait `Send` is not implemented for `NotSend` + = note: required because it appears within the type `(NotSend,)` + = note: required because it appears within the type `{ResumeTy, (NotSend,), impl Future, ()}` + = note: required because it appears within the type `[static generator@$DIR/partial-drop-partial-reinit.rs:22:16: 27:2]` + = note: required because it appears within the type `from_generator::GenFuture<[static generator@$DIR/partial-drop-partial-reinit.rs:22:16: 27:2]>` + = note: required because it appears within the type `impl Future` + = note: required because it appears within the type `impl Future` +note: required by a bound in `gimme_send` + --> $DIR/partial-drop-partial-reinit.rs:10:18 + | +LL | fn gimme_send(t: T) { + | ^^^^ required by this bound in `gimme_send` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/async-await/unresolved_type_param.rs b/src/test/ui/async-await/unresolved_type_param.rs index 85d868c27032e..d313691b38857 100644 --- a/src/test/ui/async-await/unresolved_type_param.rs +++ b/src/test/ui/async-await/unresolved_type_param.rs @@ -10,20 +10,12 @@ async fn foo() { //~^ ERROR type inside `async fn` body must be known in this context //~| ERROR type inside `async fn` body must be known in this context //~| ERROR type inside `async fn` body must be known in this context - //~| ERROR type inside `async fn` body must be known in this context - //~| ERROR type inside `async fn` body must be known in this context - //~| NOTE cannot infer type for type parameter `T` - //~| NOTE cannot infer type for type parameter `T` //~| NOTE cannot infer type for type parameter `T` //~| NOTE cannot infer type for type parameter `T` //~| NOTE cannot infer type for type parameter `T` //~| NOTE the type is part of the `async fn` body because of this `await` //~| NOTE the type is part of the `async fn` body because of this `await` //~| NOTE the type is part of the `async fn` body because of this `await` - //~| NOTE the type is part of the `async fn` body because of this `await` - //~| NOTE the type is part of the `async fn` body because of this `await` - //~| NOTE in this expansion of desugaring of `await` - //~| NOTE in this expansion of desugaring of `await` //~| NOTE in this expansion of desugaring of `await` //~| NOTE in this expansion of desugaring of `await` //~| NOTE in this expansion of desugaring of `await` diff --git a/src/test/ui/async-await/unresolved_type_param.stderr b/src/test/ui/async-await/unresolved_type_param.stderr index 8c0ecb8785d33..d19a3226ef9a4 100644 --- a/src/test/ui/async-await/unresolved_type_param.stderr +++ b/src/test/ui/async-await/unresolved_type_param.stderr @@ -34,30 +34,6 @@ note: the type is part of the `async fn` body because of this `await` LL | bar().await; | ^^^^^^ -error[E0698]: type inside `async fn` body must be known in this context - --> $DIR/unresolved_type_param.rs:9:5 - | -LL | bar().await; - | ^^^ cannot infer type for type parameter `T` declared on the function `bar` - | -note: the type is part of the `async fn` body because of this `await` - --> $DIR/unresolved_type_param.rs:9:10 - | -LL | bar().await; - | ^^^^^^ - -error[E0698]: type inside `async fn` body must be known in this context - --> $DIR/unresolved_type_param.rs:9:5 - | -LL | bar().await; - | ^^^ cannot infer type for type parameter `T` declared on the function `bar` - | -note: the type is part of the `async fn` body because of this `await` - --> $DIR/unresolved_type_param.rs:9:10 - | -LL | bar().await; - | ^^^^^^ - -error: aborting due to 5 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0698`. diff --git a/src/test/ui/generator/drop-control-flow.rs b/src/test/ui/generator/drop-control-flow.rs new file mode 100644 index 0000000000000..6319a29f5b7d0 --- /dev/null +++ b/src/test/ui/generator/drop-control-flow.rs @@ -0,0 +1,121 @@ +// build-pass + +// A test to ensure generators capture values that were conditionally dropped, +// and also that values that are dropped along all paths to a yield do not get +// included in the generator type. + +#![feature(generators, negative_impls)] +#![allow(unused_assignments, dead_code)] + +struct Ptr; +impl<'a> Drop for Ptr { + fn drop(&mut self) {} +} + +struct NonSend; +impl !Send for NonSend {} + +fn assert_send(_: T) {} + +// This test case is reduced from src/test/ui/drop/dynamic-drop-async.rs +fn one_armed_if(arg: bool) { + let _ = || { + let arr = [Ptr]; + if arg { + drop(arr); + } + yield; + }; +} + +fn two_armed_if(arg: bool) { + assert_send(|| { + let arr = [Ptr]; + if arg { + drop(arr); + } else { + drop(arr); + } + yield; + }) +} + +fn if_let(arg: Option) { + let _ = || { + let arr = [Ptr]; + if let Some(_) = arg { + drop(arr); + } + yield; + }; +} + +fn init_in_if(arg: bool) { + assert_send(|| { + let mut x = NonSend; + drop(x); + if arg { + x = NonSend; + } else { + yield; + } + }) +} + +fn init_in_match_arm(arg: Option) { + assert_send(|| { + let mut x = NonSend; + drop(x); + match arg { + Some(_) => x = NonSend, + None => yield, + } + }) +} + +fn reinit() { + let _ = || { + let mut arr = [Ptr]; + drop(arr); + arr = [Ptr]; + yield; + }; +} + +fn loop_uninit() { + let _ = || { + let mut arr = [Ptr]; + let mut count = 0; + drop(arr); + while count < 3 { + yield; + arr = [Ptr]; + count += 1; + } + }; +} + +fn nested_loop() { + let _ = || { + let mut arr = [Ptr]; + let mut count = 0; + drop(arr); + while count < 3 { + for _ in 0..3 { + yield; + } + arr = [Ptr]; + count += 1; + } + }; +} + +fn main() { + one_armed_if(true); + if_let(Some(41)); + init_in_if(true); + init_in_match_arm(Some(41)); + reinit(); + loop_uninit(); + nested_loop(); +} diff --git a/src/test/ui/generator/drop-yield-twice.rs b/src/test/ui/generator/drop-yield-twice.rs new file mode 100644 index 0000000000000..f484cbb8d67d5 --- /dev/null +++ b/src/test/ui/generator/drop-yield-twice.rs @@ -0,0 +1,15 @@ +#![feature(negative_impls, generators)] + +struct Foo(i32); +impl !Send for Foo {} + +fn main() { + assert_send(|| { //~ ERROR generator cannot be sent between threads safely + let guard = Foo(42); + yield; + drop(guard); + yield; + }) +} + +fn assert_send(_: T) {} diff --git a/src/test/ui/generator/drop-yield-twice.stderr b/src/test/ui/generator/drop-yield-twice.stderr new file mode 100644 index 0000000000000..f821f2f40055f --- /dev/null +++ b/src/test/ui/generator/drop-yield-twice.stderr @@ -0,0 +1,25 @@ +error: generator cannot be sent between threads safely + --> $DIR/drop-yield-twice.rs:7:5 + | +LL | assert_send(|| { + | ^^^^^^^^^^^ generator is not `Send` + | + = help: within `[generator@$DIR/drop-yield-twice.rs:7:17: 12:6]`, the trait `Send` is not implemented for `Foo` +note: generator is not `Send` as this value is used across a yield + --> $DIR/drop-yield-twice.rs:9:9 + | +LL | let guard = Foo(42); + | ----- has type `Foo` which is not `Send` +LL | yield; + | ^^^^^ yield occurs here, with `guard` maybe used later +... +LL | }) + | - `guard` is later dropped here +note: required by a bound in `assert_send` + --> $DIR/drop-yield-twice.rs:15:19 + | +LL | fn assert_send(_: T) {} + | ^^^^ required by this bound in `assert_send` + +error: aborting due to previous error + diff --git a/src/test/ui/generator/issue-57478.rs b/src/test/ui/generator/issue-57478.rs new file mode 100644 index 0000000000000..39710febdb95c --- /dev/null +++ b/src/test/ui/generator/issue-57478.rs @@ -0,0 +1,16 @@ +// check-pass + +#![feature(negative_impls, generators)] + +struct Foo; +impl !Send for Foo {} + +fn main() { + assert_send(|| { + let guard = Foo; + drop(guard); + yield; + }) +} + +fn assert_send(_: T) {} diff --git a/src/test/ui/generator/partial-drop.rs b/src/test/ui/generator/partial-drop.rs new file mode 100644 index 0000000000000..36f6e78cb3bfe --- /dev/null +++ b/src/test/ui/generator/partial-drop.rs @@ -0,0 +1,40 @@ +#![feature(negative_impls, generators)] + +struct Foo; +impl !Send for Foo {} + +struct Bar { + foo: Foo, + x: i32, +} + +fn main() { + assert_send(|| { + //~^ ERROR generator cannot be sent between threads safely + // FIXME: it would be nice to make this work. + let guard = Bar { foo: Foo, x: 42 }; + drop(guard.foo); + yield; + }); + + assert_send(|| { + //~^ ERROR generator cannot be sent between threads safely + // FIXME: it would be nice to make this work. + let guard = Bar { foo: Foo, x: 42 }; + drop(guard); + guard.foo = Foo; + guard.x = 23; + yield; + }); + + assert_send(|| { + //~^ ERROR generator cannot be sent between threads safely + // FIXME: it would be nice to make this work. + let guard = Bar { foo: Foo, x: 42 }; + let Bar { foo, x } = guard; + drop(foo); + yield; + }); +} + +fn assert_send(_: T) {} diff --git a/src/test/ui/generator/partial-drop.stderr b/src/test/ui/generator/partial-drop.stderr new file mode 100644 index 0000000000000..9a1b0734d8c86 --- /dev/null +++ b/src/test/ui/generator/partial-drop.stderr @@ -0,0 +1,71 @@ +error: generator cannot be sent between threads safely + --> $DIR/partial-drop.rs:12:5 + | +LL | assert_send(|| { + | ^^^^^^^^^^^ generator is not `Send` + | + = help: within `[generator@$DIR/partial-drop.rs:12:17: 18:6]`, the trait `Send` is not implemented for `Foo` +note: generator is not `Send` as this value is used across a yield + --> $DIR/partial-drop.rs:17:9 + | +LL | let guard = Bar { foo: Foo, x: 42 }; + | ----- has type `Bar` which is not `Send` +LL | drop(guard.foo); +LL | yield; + | ^^^^^ yield occurs here, with `guard` maybe used later +LL | }); + | - `guard` is later dropped here +note: required by a bound in `assert_send` + --> $DIR/partial-drop.rs:40:19 + | +LL | fn assert_send(_: T) {} + | ^^^^ required by this bound in `assert_send` + +error: generator cannot be sent between threads safely + --> $DIR/partial-drop.rs:20:5 + | +LL | assert_send(|| { + | ^^^^^^^^^^^ generator is not `Send` + | + = help: within `[generator@$DIR/partial-drop.rs:20:17: 28:6]`, the trait `Send` is not implemented for `Foo` +note: generator is not `Send` as this value is used across a yield + --> $DIR/partial-drop.rs:27:9 + | +LL | let guard = Bar { foo: Foo, x: 42 }; + | ----- has type `Bar` which is not `Send` +... +LL | yield; + | ^^^^^ yield occurs here, with `guard` maybe used later +LL | }); + | - `guard` is later dropped here +note: required by a bound in `assert_send` + --> $DIR/partial-drop.rs:40:19 + | +LL | fn assert_send(_: T) {} + | ^^^^ required by this bound in `assert_send` + +error: generator cannot be sent between threads safely + --> $DIR/partial-drop.rs:30:5 + | +LL | assert_send(|| { + | ^^^^^^^^^^^ generator is not `Send` + | + = help: within `[generator@$DIR/partial-drop.rs:30:17: 37:6]`, the trait `Send` is not implemented for `Foo` +note: generator is not `Send` as this value is used across a yield + --> $DIR/partial-drop.rs:36:9 + | +LL | let guard = Bar { foo: Foo, x: 42 }; + | ----- has type `Bar` which is not `Send` +... +LL | yield; + | ^^^^^ yield occurs here, with `guard` maybe used later +LL | }); + | - `guard` is later dropped here +note: required by a bound in `assert_send` + --> $DIR/partial-drop.rs:40:19 + | +LL | fn assert_send(_: T) {} + | ^^^^ required by this bound in `assert_send` + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/generator/reinit-in-match-guard.rs b/src/test/ui/generator/reinit-in-match-guard.rs new file mode 100644 index 0000000000000..260b341a52525 --- /dev/null +++ b/src/test/ui/generator/reinit-in-match-guard.rs @@ -0,0 +1,25 @@ +// build-pass + +#![feature(generators)] + +#![allow(unused_assignments, dead_code)] + +fn main() { + let _ = || { + let mut x = vec![22_usize]; + std::mem::drop(x); + match y() { + true if { + x = vec![]; + false + } => {} + _ => { + yield; + } + } + }; +} + +fn y() -> bool { + true +} diff --git a/src/test/ui/lint/must_not_suspend/dedup.rs b/src/test/ui/lint/must_not_suspend/dedup.rs index 040fff5a5a5a8..81a08579bb7bc 100644 --- a/src/test/ui/lint/must_not_suspend/dedup.rs +++ b/src/test/ui/lint/must_not_suspend/dedup.rs @@ -13,7 +13,7 @@ async fn wheeee(t: T) { } async fn yes() { - wheeee(No {}).await; //~ ERROR `No` held across + wheeee(&No {}).await; //~ ERROR `No` held across } fn main() { diff --git a/src/test/ui/lint/must_not_suspend/dedup.stderr b/src/test/ui/lint/must_not_suspend/dedup.stderr index bc1b611299a2b..13fa3ae3008d2 100644 --- a/src/test/ui/lint/must_not_suspend/dedup.stderr +++ b/src/test/ui/lint/must_not_suspend/dedup.stderr @@ -1,8 +1,8 @@ error: `No` held across a suspend point, but should not be - --> $DIR/dedup.rs:16:12 + --> $DIR/dedup.rs:16:13 | -LL | wheeee(No {}).await; - | ^^^^^ ------ the value is held across this suspend point +LL | wheeee(&No {}).await; + | ^^^^^ ------ the value is held across this suspend point | note: the lint level is defined here --> $DIR/dedup.rs:3:9 @@ -10,10 +10,10 @@ note: the lint level is defined here LL | #![deny(must_not_suspend)] | ^^^^^^^^^^^^^^^^ help: consider using a block (`{ ... }`) to shrink the value's scope, ending before the suspend point - --> $DIR/dedup.rs:16:12 + --> $DIR/dedup.rs:16:13 | -LL | wheeee(No {}).await; - | ^^^^^ +LL | wheeee(&No {}).await; + | ^^^^^ error: aborting due to previous error