Skip to content

Commit

Permalink
extract method to read scrutinee conditionally
Browse files Browse the repository at this point in the history
  • Loading branch information
@dingxiangfei2009
dingxiangfei2009 committed Jul 11, 2022
1 parent 1cd30e7 commit 8e4a971
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 103 deletions.
18 changes: 2 additions & 16 deletions compiler/rustc_ast_lowering/src/block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
match s.kind {
StmtKind::Local(ref local) => {
let hir_id = self.lower_node_id(s.id);
let els = if let LocalKind::InitElse(_, els) = &local.kind {
if !self.tcx.features().let_else {
feature_err(
&self.tcx.sess.parse_sess,
sym::let_else,
s.span,
"`let...else` statements are unstable",
)
.emit();
}
Some(self.lower_block(els, false))
} else {
None
};
let local = self.lower_local(local);
self.alias_attrs(hir_id, local.hir_id);
let kind = hir::StmtKind::Local(local);
Expand Down Expand Up @@ -106,9 +92,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let hir_id = self.lower_node_id(l.id);
let pat = self.lower_pat(&l.pat);
let els = if let LocalKind::InitElse(_, els) = &l.kind {
if !self.sess.features_untracked().let_else {
if !self.tcx.features().let_else {
feature_err(
&self.sess.parse_sess,
&self.tcx.sess.parse_sess,
sym::let_else,
l.span,
"`let...else` statements are unstable",
Expand Down
190 changes: 103 additions & 87 deletions compiler/rustc_typeck/src/expr_use_visitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
//! normal visitor, which just walks the entire body in one shot, the
//! `ExprUseVisitor` determines how expressions are being used.

use std::slice::from_ref;

use hir::def::DefKind;
use hir::Expr;
// Export these here so that Clippy can use them.
pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection};

Expand Down Expand Up @@ -257,91 +260,11 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {

hir::ExprKind::Match(ref discr, arms, _) => {
let discr_place = return_if_err!(self.mc.cat_expr(discr));

// Matching should not always be considered a use of the place, hence
// discr does not necessarily need to be borrowed.
// We only want to borrow discr if the pattern contain something other
// than wildcards.
let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self;
let mut needs_to_be_read = false;
for arm in arms.iter() {
return_if_err!(mc.cat_pattern(discr_place.clone(), arm.pat, |place, pat| {
match &pat.kind {
PatKind::Binding(.., opt_sub_pat) => {
// If the opt_sub_pat is None, than the binding does not count as
// a wildcard for the purpose of borrowing discr.
if opt_sub_pat.is_none() {
needs_to_be_read = true;
}
}
PatKind::Path(qpath) => {
// A `Path` pattern is just a name like `Foo`. This is either a
// named constant or else it refers to an ADT variant

let res = self.mc.typeck_results.qpath_res(qpath, pat.hir_id);
match res {
Res::Def(DefKind::Const, _)
| Res::Def(DefKind::AssocConst, _) => {
// Named constants have to be equated with the value
// being matched, so that's a read of the value being matched.
//
// FIXME: We don't actually reads for ZSTs.
needs_to_be_read = true;
}
_ => {
// Otherwise, this is a struct/enum variant, and so it's
// only a read if we need to read the discriminant.
needs_to_be_read |= is_multivariant_adt(place.place.ty());
}
}
}
PatKind::TupleStruct(..) | PatKind::Struct(..) | PatKind::Tuple(..) => {
// For `Foo(..)`, `Foo { ... }` and `(...)` patterns, check if we are matching
// against a multivariant enum or struct. In that case, we have to read
// the discriminant. Otherwise this kind of pattern doesn't actually
// read anything (we'll get invoked for the `...`, which may indeed
// perform some reads).

let place_ty = place.place.ty();
needs_to_be_read |= is_multivariant_adt(place_ty);
}
PatKind::Lit(_) | PatKind::Range(..) => {
// If the PatKind is a Lit or a Range then we want
// to borrow discr.
needs_to_be_read = true;
}
PatKind::Or(_)
| PatKind::Box(_)
| PatKind::Slice(..)
| PatKind::Ref(..)
| PatKind::Wild => {
// If the PatKind is Or, Box, Slice or Ref, the decision is made later
// as these patterns contains subpatterns
// If the PatKind is Wild, the decision is made based on the other patterns being
// examined
}
}
}));
}

if needs_to_be_read {
self.borrow_expr(discr, ty::ImmBorrow);
} else {
let closure_def_id = match discr_place.place.base {
PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
_ => None,
};

self.delegate.fake_read(
&discr_place,
FakeReadCause::ForMatchedPlace(closure_def_id),
discr_place.hir_id,
);

// We always want to walk the discriminant. We want to make sure, for instance,
// that the discriminant has been initialized.
self.walk_expr(discr);
}
self.maybe_read_scrutinee(
discr,
discr_place.clone(),
arms.iter().map(|arm| arm.pat),
);

// treatment of the discriminant is handled while walking the arms.
for arm in arms {
Expand Down Expand Up @@ -470,6 +393,97 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
}
}

fn maybe_read_scrutinee<'t>(
&mut self,
discr: &Expr<'_>,
discr_place: PlaceWithHirId<'tcx>,
pats: impl Iterator<Item = &'t hir::Pat<'t>>,
) {
// Matching should not always be considered a use of the place, hence
// discr does not necessarily need to be borrowed.
// We only want to borrow discr if the pattern contain something other
// than wildcards.
let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self;
let mut needs_to_be_read = false;
for pat in pats {
return_if_err!(mc.cat_pattern(discr_place.clone(), pat, |place, pat| {
match &pat.kind {
PatKind::Binding(.., opt_sub_pat) => {
// If the opt_sub_pat is None, than the binding does not count as
// a wildcard for the purpose of borrowing discr.
if opt_sub_pat.is_none() {
needs_to_be_read = true;
}
}
PatKind::Path(qpath) => {
// A `Path` pattern is just a name like `Foo`. This is either a
// named constant or else it refers to an ADT variant

let res = self.mc.typeck_results.qpath_res(qpath, pat.hir_id);
match res {
Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) => {
// Named constants have to be equated with the value
// being matched, so that's a read of the value being matched.
//
// FIXME: We don't actually reads for ZSTs.
needs_to_be_read = true;
}
_ => {
// Otherwise, this is a struct/enum variant, and so it's
// only a read if we need to read the discriminant.
needs_to_be_read |= is_multivariant_adt(place.place.ty());
}
}
}
PatKind::TupleStruct(..) | PatKind::Struct(..) | PatKind::Tuple(..) => {
// For `Foo(..)`, `Foo { ... }` and `(...)` patterns, check if we are matching
// against a multivariant enum or struct. In that case, we have to read
// the discriminant. Otherwise this kind of pattern doesn't actually
// read anything (we'll get invoked for the `...`, which may indeed
// perform some reads).

let place_ty = place.place.ty();
needs_to_be_read |= is_multivariant_adt(place_ty);
}
PatKind::Lit(_) | PatKind::Range(..) => {
// If the PatKind is a Lit or a Range then we want
// to borrow discr.
needs_to_be_read = true;
}
PatKind::Or(_)
| PatKind::Box(_)
| PatKind::Slice(..)
| PatKind::Ref(..)
| PatKind::Wild => {
// If the PatKind is Or, Box, Slice or Ref, the decision is made later
// as these patterns contains subpatterns
// If the PatKind is Wild, the decision is made based on the other patterns being
// examined
}
}
}));
}

if needs_to_be_read {
self.borrow_expr(discr, ty::ImmBorrow);
} else {
let closure_def_id = match discr_place.place.base {
PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
_ => None,
};

self.delegate.fake_read(
&discr_place,
FakeReadCause::ForMatchedPlace(closure_def_id),
discr_place.hir_id,
);

// We always want to walk the discriminant. We want to make sure, for instance,
// that the discriminant has been initialized.
self.walk_expr(discr);
}
}

fn walk_local<F>(
&mut self,
expr: &hir::Expr<'_>,
Expand All @@ -484,10 +498,12 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
f(self);
if let Some(els) = els {
// borrowing because we need to test the descriminant
self.borrow_expr(expr, ImmBorrow);
// self.borrow_expr(expr, ImmBorrow);
self.maybe_read_scrutinee(expr, expr_place, from_ref(pat).iter());
self.walk_block(els)
} else {
self.walk_irrefutable_pat(&expr_place, &pat);
}
self.walk_irrefutable_pat(&expr_place, &pat);
}

/// Indicates that the value of `blk` will be consumed, meaning either copied or moved
Expand Down

0 comments on commit 8e4a971

Please sign in to comment.