Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Handle patterns within closures #45

Merged
merged 8 commits into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use rustc_hir::{
};
use rustc_index::vec::{Idx, IndexVec};
use rustc_macros::HashStable;
use rustc_middle::mir::FakeReadCause;
use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
use rustc_session::config::{BorrowckMode, CrateType, OutputFilenames};
use rustc_session::lint::{Level, Lint};
Expand Down Expand Up @@ -430,6 +431,30 @@ pub struct TypeckResults<'tcx> {
/// see `MinCaptureInformationMap` for more details.
pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>,

/// Tracks the fake reads required for a closure and the reason for the fake read.
/// When performing pattern matching for closures, there are times we don't end up
/// reading places that are mentioned in a closure (because of _ patterns). However,
/// to ensure the places are initialized, we introduce fake reads.
/// Consider these two examples:
/// ``` (discriminant matching with only wildcard arm)
/// let x: u8;
/// let c = || match x { _ => () };
/// ```
/// In this example, we don't need to actually read/borrow `x` in `c`, and so we don't
/// want to capture it. However, we do still want an error here, because `x` should have
/// to be initialized at the point where c is created. Therefore, we add a "fake read"
/// instead.
/// ``` (destructured assignments)
/// let c = || {
/// let (t1, t2) = t;
/// }
/// ```
/// In the second example, we capture the disjoint fields of `t` (`t.0` & `t.1`), but
/// we never capture `t`. This becomes an issue when we build MIR as we require
/// information on `t` in order to create place `t.0` and `t.1`. We can solve this
/// issue by fake reading `t`.
pub closure_fake_reads: FxHashMap<DefId, Vec<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>>,

/// Stores the type, expression, span and optional scope span of all types
/// that are live across the yield of this generator (if a generator).
pub generator_interior_types: ty::Binder<Vec<GeneratorInteriorTypeCause<'tcx>>>,
Expand Down Expand Up @@ -464,6 +489,7 @@ impl<'tcx> TypeckResults<'tcx> {
concrete_opaque_types: Default::default(),
closure_captures: Default::default(),
closure_min_captures: Default::default(),
closure_fake_reads: Default::default(),
generator_interior_types: ty::Binder::dummy(Default::default()),
treat_byte_string_as_slice: Default::default(),
}
Expand Down Expand Up @@ -715,6 +741,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
ref concrete_opaque_types,
ref closure_captures,
ref closure_min_captures,
ref closure_fake_reads,
ref generator_interior_types,
ref treat_byte_string_as_slice,
} = *self;
Expand Down Expand Up @@ -750,6 +777,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
concrete_opaque_types.hash_stable(hcx, hasher);
closure_captures.hash_stable(hcx, hasher);
closure_min_captures.hash_stable(hcx, hasher);
closure_fake_reads.hash_stable(hcx, hasher);
generator_interior_types.hash_stable(hcx, hasher);
treat_byte_string_as_slice.hash_stable(hcx, hasher);
})
Expand Down
80 changes: 54 additions & 26 deletions compiler/rustc_mir_build/src/build/expr/as_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ use rustc_middle::hir::place::ProjectionKind as HirProjectionKind;
use rustc_middle::middle::region;
use rustc_middle::mir::AssertKind::BoundsCheck;
use rustc_middle::mir::*;
use rustc_middle::ty::AdtDef;
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty, TyCtxt, Variance};
use rustc_span::Span;
use rustc_target::abi::VariantIdx;

use rustc_index::vec::Idx;

/// The "outermost" place that holds this value.
#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug, PartialEq)]
crate enum PlaceBase {
/// Denotes the start of a `Place`.
Local(Local),
Expand Down Expand Up @@ -67,7 +68,7 @@ crate enum PlaceBase {
///
/// This is used internally when building a place for an expression like `a.b.c`. The fields `b`
/// and `c` can be progressively pushed onto the place builder that is created when converting `a`.
#[derive(Clone)]
#[derive(Clone, Debug, PartialEq)]
crate struct PlaceBuilder<'tcx> {
base: PlaceBase,
projection: Vec<PlaceElem<'tcx>>,
Expand All @@ -83,20 +84,23 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>(
mir_projections: &[PlaceElem<'tcx>],
) -> Vec<HirProjectionKind> {
let mut hir_projections = Vec::new();
let mut variant = None;

for mir_projection in mir_projections {
let hir_projection = match mir_projection {
ProjectionElem::Deref => HirProjectionKind::Deref,
ProjectionElem::Field(field, _) => {
// We will never encouter this for multivariant enums,
// read the comment for `Downcast`.
HirProjectionKind::Field(field.index() as u32, VariantIdx::new(0))
let variant = variant.unwrap_or(VariantIdx::new(0));
HirProjectionKind::Field(field.index() as u32, variant)
}
ProjectionElem::Downcast(..) => {
// This projections exist only for enums that have
// multiple variants. Since such enums that are captured
// completely, we can stop here.
break;
ProjectionElem::Downcast(.., idx) => {
// We don't expect to see multi-variant enums here, as earlier
// phases will have truncated them already. However, there can
// still be downcasts, thanks to single-variant enums.
// We keep track of VariantIdx so we can use this information
// if the next ProjectionElem is a Field.
variant = Some(*idx);
continue;
}
ProjectionElem::Index(..)
| ProjectionElem::ConstantIndex { .. }
Expand All @@ -106,7 +110,7 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>(
break;
}
};

variant = None;
hir_projections.push(hir_projection);
}

Expand Down Expand Up @@ -194,12 +198,12 @@ fn find_capture_matching_projections<'a, 'tcx>(
/// Takes a PlaceBuilder and resolves the upvar (if any) within it, so that the
/// `PlaceBuilder` now starts from `PlaceBase::Local`.
///
/// Returns a Result with the error being the HirId of the Upvar that was not found.
/// Returns a Result with the error being the PlaceBuilder (`from_builder`) that was not found.
fn to_upvars_resolved_place_builder<'a, 'tcx>(
from_builder: PlaceBuilder<'tcx>,
tcx: TyCtxt<'tcx>,
typeck_results: &'a ty::TypeckResults<'tcx>,
) -> Result<PlaceBuilder<'tcx>, HirId> {
) -> Result<PlaceBuilder<'tcx>, PlaceBuilder<'tcx>> {
match from_builder.base {
PlaceBase::Local(_) => Ok(from_builder),
PlaceBase::Upvar { var_hir_id, closure_def_id, closure_kind } => {
Expand Down Expand Up @@ -230,13 +234,12 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>(
from_builder.projection
)
} else {
// FIXME(project-rfc-2229#24): Handle this case properly
debug!(
"No associated capture found for {:?}[{:#?}]",
var_hir_id, from_builder.projection,
);
}
return Err(var_hir_id);
return Err(from_builder);
};

let closure_ty = typeck_results
Expand Down Expand Up @@ -300,6 +303,25 @@ impl<'tcx> PlaceBuilder<'tcx> {
to_upvars_resolved_place_builder(self, tcx, typeck_results).unwrap()
}

/// Attempts to resolve the `PlaceBuilder`.
/// On success, it will return the resolved `PlaceBuilder`.
/// On failure, it will return itself.
///
/// Upvars resolve may fail for a `PlaceBuilder` when attempting to
/// resolve a disjoint field whose root variable is not captured
/// (destructured assignments) or when attempting to resolve a root
/// variable (discriminant matching with only wildcard arm) that is
/// not captured. This can happen because the final mir that will be
/// generated doesn't require a read for this place. Failures will only
/// happen inside closures.
crate fn try_upvars_resolved<'a>(
self,
tcx: TyCtxt<'tcx>,
typeck_results: &'a ty::TypeckResults<'tcx>,
) -> Result<PlaceBuilder<'tcx>, PlaceBuilder<'tcx>> {
to_upvars_resolved_place_builder(self, tcx, typeck_results)
}

crate fn base(&self) -> PlaceBase {
self.base
}
Expand All @@ -308,15 +330,22 @@ impl<'tcx> PlaceBuilder<'tcx> {
self.project(PlaceElem::Field(f, ty))
}

fn deref(self) -> Self {
crate fn deref(self) -> Self {
self.project(PlaceElem::Deref)
}

crate fn downcast(self, adt_def: &'tcx AdtDef, variant_index: VariantIdx) -> Self {
self.project(PlaceElem::Downcast(
Some(adt_def.variants[variant_index].ident.name),
variant_index,
))
}

fn index(self, index: Local) -> Self {
self.project(PlaceElem::Index(index))
}

fn project(mut self, elem: PlaceElem<'tcx>) -> Self {
crate fn project(mut self, elem: PlaceElem<'tcx>) -> Self {
self.projection.push(elem);
self
}
Expand Down Expand Up @@ -602,13 +631,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// The "retagging" transformation (for Stacked Borrows) relies on this.
let idx = unpack!(block = self.as_temp(block, temp_lifetime, index, Mutability::Not,));

block = self.bounds_check(
block,
base_place.clone().into_place(self.tcx, self.typeck_results),
idx,
expr_span,
source_info,
);
block = self.bounds_check(block, base_place.clone(), idx, expr_span, source_info);

if is_outermost_index {
self.read_fake_borrows(block, fake_borrow_temps, source_info)
Expand All @@ -629,7 +652,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn bounds_check(
&mut self,
block: BasicBlock,
slice: Place<'tcx>,
slice: PlaceBuilder<'tcx>,
index: Local,
expr_span: Span,
source_info: SourceInfo,
Expand All @@ -641,7 +664,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let lt = self.temp(bool_ty, expr_span);

// len = len(slice)
self.cfg.push_assign(block, source_info, len, Rvalue::Len(slice));
self.cfg.push_assign(
block,
source_info,
len,
Rvalue::Len(slice.into_place(self.tcx, self.typeck_results)),
);
// lt = idx < len
self.cfg.push_assign(
block,
Expand Down
38 changes: 37 additions & 1 deletion compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::build::{BlockAnd, BlockAndExtension, Builder};
use crate::thir::*;
use rustc_middle::middle::region;
use rustc_middle::mir::AssertKind;
use rustc_middle::mir::Place;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, Ty, UpvarSubsts};
use rustc_span::Span;
Expand Down Expand Up @@ -164,7 +165,41 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields))
}
ExprKind::Closure { closure_id, substs, upvars, movability } => {
ExprKind::Closure { closure_id, substs, upvars, movability, ref fake_reads } => {
// Convert the closure fake reads, if any, from `ExprRef` to mir `Place`
// and push the fake reads.
// This must come before creating the operands. This is required in case
// there is a fake read and a borrow of the same path, since otherwise the
// fake read might interfere with the borrow. Consider an example like this
// one:
// ```
// let mut x = 0;
// let c = || {
// &mut x; // mutable borrow of `x`
// match x { _ => () } // fake read of `x`
// };
// ```
// FIXME(RFC2229): Remove feature gate once diagnostics are improved
if this.tcx.features().capture_disjoint_fields {
for (thir_place, cause, hir_id) in fake_reads.into_iter() {
let place_builder =
unpack!(block = this.as_place_builder(block, thir_place));

if let Ok(place_builder_resolved) =
place_builder.try_upvars_resolved(this.tcx, this.typeck_results)
{
let mir_place =
place_builder_resolved.into_place(this.tcx, this.typeck_results);
this.cfg.push_fake_read(
block,
this.source_info(this.tcx.hir().span(*hir_id)),
*cause,
mir_place,
);
}
}
}

// see (*) above
let operands: Vec<_> = upvars
.into_iter()
Expand Down Expand Up @@ -203,6 +238,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
})
.collect();

let result = match substs {
UpvarSubsts::Generator(substs) => {
// We implicitly set the discriminant to 0. See
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@

mod as_constant;
mod as_operand;
mod as_place;
pub mod as_place;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes,

error[E0603]: module `as_place` is private
  --> compiler/rustc_mir_build/src/build/mod.rs:2:25
   |
2  | use crate::build::expr::as_place::PlaceBuilder;
   |                         ^^^^^^^^ private module
   |
note: the module `as_place` is defined here
  --> compiler/rustc_mir_build/src/build/expr/mod.rs:65:1
   |
65 | mod as_place;
   | ^^^^^^^^^^^^^

error[E0603]: module `as_place` is private
  --> compiler/rustc_mir_build/src/build/matches/mod.rs:8:25
   |
8  | use crate::build::expr::as_place::PlaceBuilder;
   |                         ^^^^^^^^ private module
   |
note: the module `as_place` is defined here
  --> compiler/rustc_mir_build/src/build/expr/mod.rs:65:1
   |
65 | mod as_place;
   | ^^^^^^^^^^^^^

error[E0603]: module `as_place` is private
  --> compiler/rustc_mir_build/src/build/matches/simplify.rs:15:25
   |
15 | use crate::build::expr::as_place::PlaceBuilder;
   |                         ^^^^^^^^ private module
   |
note: the module `as_place` is defined here
  --> compiler/rustc_mir_build/src/build/expr/mod.rs:65:1
   |
65 | mod as_place;
   | ^^^^^^^^^^^^^

error[E0603]: module `as_place` is private
  --> compiler/rustc_mir_build/src/build/matches/test.rs:8:25
   |
8  | use crate::build::expr::as_place::PlaceBuilder;
   |                         ^^^^^^^^ private module
   |
note: the module `as_place` is defined here
  --> compiler/rustc_mir_build/src/build/expr/mod.rs:65:1
   |
65 | mod as_place;
   | ^^^^^^^^^^^^^

error[E0603]: module `as_place` is private
  --> compiler/rustc_mir_build/src/build/matches/util.rs:1:25
   |
1  | use crate::build::expr::as_place::PlaceBuilder;
   |                         ^^^^^^^^ private module
   |
note: the module `as_place` is defined here
  --> compiler/rustc_mir_build/src/build/expr/mod.rs:65:1
   |
65 | mod as_place;
   | ^^^^^^^^^^^^^

error: aborting due to 5 previous errors

mod as_rvalue;
mod as_temp;
mod category;
Expand Down
Loading