Skip to content

Commit

Permalink
Auto merge of #90352 - camsteffen:for-loop-desugar, r=oli-obk
Browse files Browse the repository at this point in the history 
Simplify `for` loop desugar

Basically two intermediate bindings are inlined. I could have left one intermediate binding in place as this would simplify some diagnostic logic, but I think the difference in that regard would be negligible, so it is better to have a minimal HIR.

For checking that the pattern is irrefutable, I added a special case when the `match` is found to be non-exhaustive.

The reordering of the arms is purely stylistic. I don't *think* there are any perf implications.

```diff
  match IntoIterator::into_iter($head) {
      mut iter => {
          $label: loop {
-             let mut __next;
              match Iterator::next(&mut iter) {
-                 Some(val) => __next = val,
                  None => break,
+                 Some($pat) => $block,
              }
-             let $pat = __next;
-             $block
          }
      }
  }
```
  • Loading branch information
@bors
bors committed Nov 21, 2021
2 parents 65f3f8b + 66da8fa commit cebd2dd
Show file tree
Hide file tree
Showing 42 changed files with 321 additions and 569 deletions.
137 changes: 38 additions & 99 deletions compiler/rustc_ast_lowering/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_session::parse::feature_err;
use rustc_span::hygiene::ExpnId;
use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{hygiene::ForLoopLoc, DUMMY_SP};
use rustc_span::DUMMY_SP;

impl<'hir> LoweringContext<'_, 'hir> {
fn lower_exprs(&mut self, exprs: &[AstP<Expr>]) -> &'hir [hir::Expr<'hir>] {
Expand Down Expand Up @@ -1308,16 +1308,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
/// Desugar `ExprForLoop` from: `[opt_ident]: for <pat> in <head> <body>` into:
/// ```rust
/// {
/// let result = match ::std::iter::IntoIterator::into_iter(<head>) {
/// let result = match IntoIterator::into_iter(<head>) {
/// mut iter => {
/// [opt_ident]: loop {
/// let mut __next;
/// match ::std::iter::Iterator::next(&mut iter) {
/// ::std::option::Option::Some(val) => __next = val,
/// ::std::option::Option::None => break
/// match Iterator::next(&mut iter) {
/// None => break,
/// Some(<pat>) => <body>,
/// };
/// let <pat> = __next;
/// StmtKind::Expr(<body>);
/// }
/// }
/// };
Expand All @@ -1332,133 +1329,75 @@ impl<'hir> LoweringContext<'_, 'hir> {
body: &Block,
opt_label: Option<Label>,
) -> hir::Expr<'hir> {
// expand <head>
let head = self.lower_expr_mut(head);
let desugared_span =
self.mark_span_with_reason(DesugaringKind::ForLoop(ForLoopLoc::Head), head.span, None);
let e_span = self.lower_span(e.span);

let iter = Ident::with_dummy_span(sym::iter);

let next_ident = Ident::with_dummy_span(sym::__next);
let (next_pat, next_pat_hid) = self.pat_ident_binding_mode(
desugared_span,
next_ident,
hir::BindingAnnotation::Mutable,
);

// `::std::option::Option::Some(val) => __next = val`
let pat_arm = {
let val_ident = Ident::with_dummy_span(sym::val);
let pat_span = self.lower_span(pat.span);
let (val_pat, val_pat_hid) = self.pat_ident(pat_span, val_ident);
let val_expr = self.expr_ident(pat_span, val_ident, val_pat_hid);
let next_expr = self.expr_ident(pat_span, next_ident, next_pat_hid);
let assign = self.arena.alloc(self.expr(
pat_span,
hir::ExprKind::Assign(next_expr, val_expr, self.lower_span(pat_span)),
ThinVec::new(),
));
let some_pat = self.pat_some(pat_span, val_pat);
self.arm(some_pat, assign)
};
let pat = self.lower_pat(pat);
let for_span =
self.mark_span_with_reason(DesugaringKind::ForLoop, self.lower_span(e.span), None);
let head_span = self.mark_span_with_reason(DesugaringKind::ForLoop, head.span, None);
let pat_span = self.mark_span_with_reason(DesugaringKind::ForLoop, pat.span, None);

// `::std::option::Option::None => break`
let break_arm = {
// `None => break`
let none_arm = {
let break_expr =
self.with_loop_scope(e.id, |this| this.expr_break_alloc(e_span, ThinVec::new()));
let pat = self.pat_none(e_span);
self.with_loop_scope(e.id, |this| this.expr_break_alloc(for_span, ThinVec::new()));
let pat = self.pat_none(for_span);
self.arm(pat, break_expr)
};

// Some(<pat>) => <body>,
let some_arm = {
let some_pat = self.pat_some(pat_span, pat);
let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false));
let body_expr = self.arena.alloc(self.expr_block(body_block, ThinVec::new()));
self.arm(some_pat, body_expr)
};

// `mut iter`
let iter = Ident::with_dummy_span(sym::iter);
let (iter_pat, iter_pat_nid) =
self.pat_ident_binding_mode(desugared_span, iter, hir::BindingAnnotation::Mutable);
self.pat_ident_binding_mode(head_span, iter, hir::BindingAnnotation::Mutable);

// `match ::std::iter::Iterator::next(&mut iter) { ... }`
// `match Iterator::next(&mut iter) { ... }`
let match_expr = {
let iter = self.expr_ident(desugared_span, iter, iter_pat_nid);
let ref_mut_iter = self.expr_mut_addr_of(desugared_span, iter);
let iter = self.expr_ident(head_span, iter, iter_pat_nid);
let ref_mut_iter = self.expr_mut_addr_of(head_span, iter);
let next_expr = self.expr_call_lang_item_fn(
desugared_span,
head_span,
hir::LangItem::IteratorNext,
arena_vec![self; ref_mut_iter],
);
let arms = arena_vec![self; pat_arm, break_arm];
let arms = arena_vec![self; none_arm, some_arm];

self.expr_match(desugared_span, next_expr, arms, hir::MatchSource::ForLoopDesugar)
self.expr_match(head_span, next_expr, arms, hir::MatchSource::ForLoopDesugar)
};
let match_stmt = self.stmt_expr(desugared_span, match_expr);

let next_expr = self.expr_ident(desugared_span, next_ident, next_pat_hid);

// `let mut __next`
let next_let = self.stmt_let_pat(
None,
desugared_span,
None,
next_pat,
hir::LocalSource::ForLoopDesugar,
);
let match_stmt = self.stmt_expr(for_span, match_expr);

// `let <pat> = __next`
let pat = self.lower_pat(pat);
let pat_let = self.stmt_let_pat(
None,
desugared_span,
Some(next_expr),
pat,
hir::LocalSource::ForLoopDesugar,
);

let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false));
let body_expr = self.expr_block(body_block, ThinVec::new());
let body_stmt = self.stmt_expr(body_block.span, body_expr);

let loop_block = self.block_all(
e_span,
arena_vec![self; next_let, match_stmt, pat_let, body_stmt],
None,
);
let loop_block = self.block_all(for_span, arena_vec![self; match_stmt], None);

// `[opt_ident]: loop { ... }`
let kind = hir::ExprKind::Loop(
loop_block,
self.lower_label(opt_label),
hir::LoopSource::ForLoop,
self.lower_span(e_span.with_hi(head.span.hi())),
self.lower_span(for_span.with_hi(head.span.hi())),
);
let loop_expr = self.arena.alloc(hir::Expr {
hir_id: self.lower_node_id(e.id),
kind,
span: self.lower_span(e.span),
});
let loop_expr =
self.arena.alloc(hir::Expr { hir_id: self.lower_node_id(e.id), kind, span: for_span });

// `mut iter => { ... }`
let iter_arm = self.arm(iter_pat, loop_expr);

let into_iter_span = self.mark_span_with_reason(
DesugaringKind::ForLoop(ForLoopLoc::IntoIter),
head.span,
None,
);

// `match ::std::iter::IntoIterator::into_iter(<head>) { ... }`
let into_iter_expr = {
self.expr_call_lang_item_fn(
into_iter_span,
head_span,
hir::LangItem::IntoIterIntoIter,
arena_vec![self; head],
)
};

// #82462: to correctly diagnose borrow errors, the block that contains
// the iter expr needs to have a span that covers the loop body.
let desugared_full_span =
self.mark_span_with_reason(DesugaringKind::ForLoop(ForLoopLoc::Head), e_span, None);

let match_expr = self.arena.alloc(self.expr_match(
desugared_full_span,
for_span,
into_iter_expr,
arena_vec![self; iter_arm],
hir::MatchSource::ForLoopDesugar,
Expand All @@ -1472,7 +1411,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
// surrounding scope of the `match` since the `match` is not a terminating scope.
//
// Also, add the attributes to the outer returned expr node.
self.expr_drop_temps_mut(desugared_full_span, match_expr, attrs.into())
self.expr_drop_temps_mut(for_span, match_expr, attrs.into())
}

/// Desugar `ExprKind::Try` from: `<expr>?` into:
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_middle::mir::{
use rustc_middle::ty::adjustment::PointerCast;
use rustc_middle::ty::{self, RegionVid, TyCtxt};
use rustc_span::symbol::Symbol;
use rustc_span::Span;
use rustc_span::{sym, DesugaringKind, Span};

use crate::region_infer::BlameConstraint;
use crate::{
Expand Down Expand Up @@ -135,7 +135,16 @@ impl BorrowExplanation {
should_note_order,
} => {
let local_decl = &body.local_decls[dropped_local];
let (dtor_desc, type_desc) = match local_decl.ty.kind() {
let mut ty = local_decl.ty;
if local_decl.source_info.span.desugaring_kind() == Some(DesugaringKind::ForLoop) {
if let ty::Adt(adt, substs) = local_decl.ty.kind() {
if tcx.is_diagnostic_item(sym::Option, adt.did) {
// in for loop desugaring, only look at the `Some(..)` inner type
ty = substs.type_at(0);
}
}
}
let (dtor_desc, type_desc) = match ty.kind() {
// If type is an ADT that implements Drop, then
// simplify output by reporting just the ADT name.
ty::Adt(adt, _substs) if adt.has_dtor(tcx) && !adt.is_box() => {
Expand Down
12 changes: 3 additions & 9 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ use rustc_middle::mir::{
use rustc_middle::ty::print::Print;
use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt};
use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult};
use rustc_span::{
hygiene::{DesugaringKind, ForLoopLoc},
symbol::sym,
Span,
};
use rustc_span::{hygiene::DesugaringKind, symbol::sym, Span};
use rustc_target::abi::VariantIdx;

use super::borrow_set::BorrowData;
Expand Down Expand Up @@ -955,10 +951,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let kind = kind.unwrap_or_else(|| {
// This isn't a 'special' use of `self`
debug!("move_spans: method_did={:?}, fn_call_span={:?}", method_did, fn_call_span);
let implicit_into_iter = matches!(
fn_call_span.desugaring_kind(),
Some(DesugaringKind::ForLoop(ForLoopLoc::IntoIter))
);
let implicit_into_iter = Some(method_did) == tcx.lang_items().into_iter_fn()
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::ForLoop);
let parent_self_ty = parent
.filter(|did| tcx.def_kind(*did) == rustc_hir::def::DefKind::Impl)
.and_then(|did| match tcx.type_of(did).kind() {
Expand Down
24 changes: 16 additions & 8 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,15 +445,23 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
},
))) => {
// check if the RHS is from desugaring
let locations = self.body.find_assignments(local);
let opt_assignment_rhs_span = locations
.first()
.map(|&location| self.body.source_info(location).span);
let opt_desugaring_kind =
opt_assignment_rhs_span.and_then(|span| span.desugaring_kind());
match opt_desugaring_kind {
let opt_assignment_rhs_span =
self.body.find_assignments(local).first().map(|&location| {
let stmt = &self.body[location.block].statements
[location.statement_index];
match stmt.kind {
mir::StatementKind::Assign(box (
_,
mir::Rvalue::Use(mir::Operand::Copy(place)),
)) => {
self.body.local_decls[place.local].source_info.span
}
_ => self.body.source_info(location).span,
}
});
match opt_assignment_rhs_span.and_then(|s| s.desugaring_kind()) {
// on for loops, RHS points to the iterator part
Some(DesugaringKind::ForLoop(_)) => {
Some(DesugaringKind::ForLoop) => {
self.suggest_similar_mut_method_for_for_loop(&mut err);
Some((
false,
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_hir/src/hir.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1821,8 +1821,6 @@ impl<'hir> QPath<'hir> {
pub enum LocalSource {
/// A `match _ { .. }`.
Normal,
/// A desugared `for _ in _ { .. }` loop.
ForLoopDesugar,
/// When lowering async functions, we create locals within the `async move` so that
/// all parameters are dropped after the future is polled.
///
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_hir/src/pat_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::def::{CtorOf, DefKind, Res};
use crate::def_id::DefId;
use crate::hir::{self, HirId, PatKind};
use rustc_data_structures::stable_set::FxHashSet;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::symbol::Ident;
use rustc_span::Span;

Expand Down Expand Up @@ -143,4 +144,14 @@ impl hir::Pat<'_> {
});
result
}

/// If the pattern is `Some(<pat>)` from a desugared for loop, returns the inner pattern
pub fn for_loop_some(&self) -> Option<&Self> {
if self.span.desugaring_kind() == Some(DesugaringKind::ForLoop) {
if let hir::PatKind::Struct(_, [pat_field], _) = self.kind {
return Some(pat_field.pat);
}
}
None
}
}
24 changes: 18 additions & 6 deletions compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ use rustc_hir as hir;
use rustc_hir::def::{DefKind, Namespace};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{Body, Expr, ExprKind, FnRetTy, HirId, Local, Pat};
use rustc_hir::{Body, Expr, ExprKind, FnRetTy, HirId, Local, MatchSource, Pat};
use rustc_middle::hir::map::Map;
use rustc_middle::infer::unify_key::ConstVariableOriginKind;
use rustc_middle::ty::print::Print;
use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
use rustc_middle::ty::{self, DefIdTree, InferConst, Ty, TyCtxt};
use rustc_span::source_map::DesugaringKind;
use rustc_span::symbol::kw;
use rustc_span::Span;
use std::borrow::Cow;
Expand All @@ -26,6 +25,7 @@ struct FindHirNodeVisitor<'a, 'tcx> {
found_closure: Option<&'tcx Expr<'tcx>>,
found_method_call: Option<&'tcx Expr<'tcx>>,
found_exact_method_call: Option<&'tcx Expr<'tcx>>,
found_for_loop_iter: Option<&'tcx Expr<'tcx>>,
found_use_diagnostic: Option<UseDiagnostic<'tcx>>,
}

Expand All @@ -41,6 +41,7 @@ impl<'a, 'tcx> FindHirNodeVisitor<'a, 'tcx> {
found_closure: None,
found_method_call: None,
found_exact_method_call: None,
found_for_loop_iter: None,
found_use_diagnostic: None,
}
}
Expand Down Expand Up @@ -111,6 +112,15 @@ impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> {
}

fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if let ExprKind::Match(scrutinee, [_, arm], MatchSource::ForLoopDesugar) = expr.kind {
if let Some(pat) = arm.pat.for_loop_some() {
if let Some(ty) = self.node_ty_contains_target(pat.hir_id) {
self.found_for_loop_iter = Some(scrutinee);
self.found_node_ty = Some(ty);
return;
}
}
}
if let ExprKind::MethodCall(_, call_span, exprs, _) = expr.kind {
if call_span == self.target_span
&& Some(self.target)
Expand Down Expand Up @@ -643,10 +653,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let msg = if let Some(simple_ident) = pattern.simple_ident() {
match pattern.span.desugaring_kind() {
None => format!("consider giving `{}` {}", simple_ident, suffix),
Some(DesugaringKind::ForLoop(_)) => {
"the element type for this iterator is not specified".to_string()
}
_ => format!("this needs {}", suffix),
Some(_) => format!("this needs {}", suffix),
}
} else {
format!("consider giving this pattern {}", suffix)
Expand Down Expand Up @@ -719,6 +726,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// = note: type must be known at this point
self.annotate_method_call(segment, e, &mut err);
}
} else if let Some(scrutinee) = local_visitor.found_for_loop_iter {
err.span_label(
scrutinee.span,
"the element type for this iterator is not specified".to_string(),
);
}
// Instead of the following:
// error[E0282]: type annotations needed
Expand Down
Loading

0 comments on commit cebd2dd

Please sign in to comment.