Skip to content

Commit cebd2dd

Browse files
borsbors
committed
Auto merge of rust-lang#90352 - camsteffen:for-loop-desugar, r=oli-obk
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 } } } ```
2 parents 65f3f8b + 66da8fa commit cebd2dd

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+321
-569
lines changed

‎compiler/rustc_ast_lowering/src/expr.rs

Lines changed: 38 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_session::parse::feature_err;
1313
use rustc_span::hygiene::ExpnId;
1414
use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned};
1515
use rustc_span::symbol::{sym, Ident, Symbol};
16-
use rustc_span::{hygiene::ForLoopLoc, DUMMY_SP};
16+
use rustc_span::DUMMY_SP;
1717

1818
impl<'hir> LoweringContext<'_, 'hir> {
1919
fn lower_exprs(&mut self, exprs: &[AstP<Expr>]) -> &'hir [hir::Expr<'hir>] {
@@ -1308,16 +1308,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
13081308
/// Desugar `ExprForLoop` from: `[opt_ident]: for <pat> in <head> <body>` into:
13091309
/// ```rust
13101310
/// {
1311-
/// let result = match ::std::iter::IntoIterator::into_iter(<head>) {
1311+
/// let result = match IntoIterator::into_iter(<head>) {
13121312
/// mut iter => {
13131313
/// [opt_ident]: loop {
1314-
/// let mut __next;
1315-
/// match ::std::iter::Iterator::next(&mut iter) {
1316-
/// ::std::option::Option::Some(val) => __next = val,
1317-
/// ::std::option::Option::None => break
1314+
/// match Iterator::next(&mut iter) {
1315+
/// None => break,
1316+
/// Some(<pat>) => <body>,
13181317
/// };
1319-
/// let <pat> = __next;
1320-
/// StmtKind::Expr(<body>);
13211318
/// }
13221319
/// }
13231320
/// };
@@ -1332,133 +1329,75 @@ impl<'hir> LoweringContext<'_, 'hir> {
13321329
body: &Block,
13331330
opt_label: Option<Label>,
13341331
) -> hir::Expr<'hir> {
1335-
// expand <head>
13361332
let head = self.lower_expr_mut(head);
1337-
let desugared_span =
1338-
self.mark_span_with_reason(DesugaringKind::ForLoop(ForLoopLoc::Head), head.span, None);
1339-
let e_span = self.lower_span(e.span);
1340-
1341-
let iter = Ident::with_dummy_span(sym::iter);
1342-
1343-
let next_ident = Ident::with_dummy_span(sym::__next);
1344-
let (next_pat, next_pat_hid) = self.pat_ident_binding_mode(
1345-
desugared_span,
1346-
next_ident,
1347-
hir::BindingAnnotation::Mutable,
1348-
);
1349-
1350-
// `::std::option::Option::Some(val) => __next = val`
1351-
let pat_arm = {
1352-
let val_ident = Ident::with_dummy_span(sym::val);
1353-
let pat_span = self.lower_span(pat.span);
1354-
let (val_pat, val_pat_hid) = self.pat_ident(pat_span, val_ident);
1355-
let val_expr = self.expr_ident(pat_span, val_ident, val_pat_hid);
1356-
let next_expr = self.expr_ident(pat_span, next_ident, next_pat_hid);
1357-
let assign = self.arena.alloc(self.expr(
1358-
pat_span,
1359-
hir::ExprKind::Assign(next_expr, val_expr, self.lower_span(pat_span)),
1360-
ThinVec::new(),
1361-
));
1362-
let some_pat = self.pat_some(pat_span, val_pat);
1363-
self.arm(some_pat, assign)
1364-
};
1333+
let pat = self.lower_pat(pat);
1334+
let for_span =
1335+
self.mark_span_with_reason(DesugaringKind::ForLoop, self.lower_span(e.span), None);
1336+
let head_span = self.mark_span_with_reason(DesugaringKind::ForLoop, head.span, None);
1337+
let pat_span = self.mark_span_with_reason(DesugaringKind::ForLoop, pat.span, None);
13651338

1366-
// `::std::option::Option::None => break`
1367-
let break_arm = {
1339+
// `None => break`
1340+
let none_arm = {
13681341
let break_expr =
1369-
self.with_loop_scope(e.id, |this| this.expr_break_alloc(e_span, ThinVec::new()));
1370-
let pat = self.pat_none(e_span);
1342+
self.with_loop_scope(e.id, |this| this.expr_break_alloc(for_span, ThinVec::new()));
1343+
let pat = self.pat_none(for_span);
13711344
self.arm(pat, break_expr)
13721345
};
13731346

1347+
// Some(<pat>) => <body>,
1348+
let some_arm = {
1349+
let some_pat = self.pat_some(pat_span, pat);
1350+
let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false));
1351+
let body_expr = self.arena.alloc(self.expr_block(body_block, ThinVec::new()));
1352+
self.arm(some_pat, body_expr)
1353+
};
1354+
13741355
// `mut iter`
1356+
let iter = Ident::with_dummy_span(sym::iter);
13751357
let (iter_pat, iter_pat_nid) =
1376-
self.pat_ident_binding_mode(desugared_span, iter, hir::BindingAnnotation::Mutable);
1358+
self.pat_ident_binding_mode(head_span, iter, hir::BindingAnnotation::Mutable);
13771359

1378-
// `match ::std::iter::Iterator::next(&mut iter) { ... }`
1360+
// `match Iterator::next(&mut iter) { ... }`
13791361
let match_expr = {
1380-
let iter = self.expr_ident(desugared_span, iter, iter_pat_nid);
1381-
let ref_mut_iter = self.expr_mut_addr_of(desugared_span, iter);
1362+
let iter = self.expr_ident(head_span, iter, iter_pat_nid);
1363+
let ref_mut_iter = self.expr_mut_addr_of(head_span, iter);
13821364
let next_expr = self.expr_call_lang_item_fn(
1383-
desugared_span,
1365+
head_span,
13841366
hir::LangItem::IteratorNext,
13851367
arena_vec![self; ref_mut_iter],
13861368
);
1387-
let arms = arena_vec![self; pat_arm, break_arm];
1369+
let arms = arena_vec![self; none_arm, some_arm];
13881370

1389-
self.expr_match(desugared_span, next_expr, arms, hir::MatchSource::ForLoopDesugar)
1371+
self.expr_match(head_span, next_expr, arms, hir::MatchSource::ForLoopDesugar)
13901372
};
1391-
let match_stmt = self.stmt_expr(desugared_span, match_expr);
1392-
1393-
let next_expr = self.expr_ident(desugared_span, next_ident, next_pat_hid);
1394-
1395-
// `let mut __next`
1396-
let next_let = self.stmt_let_pat(
1397-
None,
1398-
desugared_span,
1399-
None,
1400-
next_pat,
1401-
hir::LocalSource::ForLoopDesugar,
1402-
);
1373+
let match_stmt = self.stmt_expr(for_span, match_expr);
14031374

1404-
// `let <pat> = __next`
1405-
let pat = self.lower_pat(pat);
1406-
let pat_let = self.stmt_let_pat(
1407-
None,
1408-
desugared_span,
1409-
Some(next_expr),
1410-
pat,
1411-
hir::LocalSource::ForLoopDesugar,
1412-
);
1413-
1414-
let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false));
1415-
let body_expr = self.expr_block(body_block, ThinVec::new());
1416-
let body_stmt = self.stmt_expr(body_block.span, body_expr);
1417-
1418-
let loop_block = self.block_all(
1419-
e_span,
1420-
arena_vec![self; next_let, match_stmt, pat_let, body_stmt],
1421-
None,
1422-
);
1375+
let loop_block = self.block_all(for_span, arena_vec![self; match_stmt], None);
14231376

14241377
// `[opt_ident]: loop { ... }`
14251378
let kind = hir::ExprKind::Loop(
14261379
loop_block,
14271380
self.lower_label(opt_label),
14281381
hir::LoopSource::ForLoop,
1429-
self.lower_span(e_span.with_hi(head.span.hi())),
1382+
self.lower_span(for_span.with_hi(head.span.hi())),
14301383
);
1431-
let loop_expr = self.arena.alloc(hir::Expr {
1432-
hir_id: self.lower_node_id(e.id),
1433-
kind,
1434-
span: self.lower_span(e.span),
1435-
});
1384+
let loop_expr =
1385+
self.arena.alloc(hir::Expr { hir_id: self.lower_node_id(e.id), kind, span: for_span });
14361386

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

1440-
let into_iter_span = self.mark_span_with_reason(
1441-
DesugaringKind::ForLoop(ForLoopLoc::IntoIter),
1442-
head.span,
1443-
None,
1444-
);
1445-
14461390
// `match ::std::iter::IntoIterator::into_iter(<head>) { ... }`
14471391
let into_iter_expr = {
14481392
self.expr_call_lang_item_fn(
1449-
into_iter_span,
1393+
head_span,
14501394
hir::LangItem::IntoIterIntoIter,
14511395
arena_vec![self; head],
14521396
)
14531397
};
14541398

1455-
// #82462: to correctly diagnose borrow errors, the block that contains
1456-
// the iter expr needs to have a span that covers the loop body.
1457-
let desugared_full_span =
1458-
self.mark_span_with_reason(DesugaringKind::ForLoop(ForLoopLoc::Head), e_span, None);
1459-
14601399
let match_expr = self.arena.alloc(self.expr_match(
1461-
desugared_full_span,
1400+
for_span,
14621401
into_iter_expr,
14631402
arena_vec![self; iter_arm],
14641403
hir::MatchSource::ForLoopDesugar,
@@ -1472,7 +1411,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
14721411
// surrounding scope of the `match` since the `match` is not a terminating scope.
14731412
//
14741413
// Also, add the attributes to the outer returned expr node.
1475-
self.expr_drop_temps_mut(desugared_full_span, match_expr, attrs.into())
1414+
self.expr_drop_temps_mut(for_span, match_expr, attrs.into())
14761415
}
14771416

14781417
/// Desugar `ExprKind::Try` from: `<expr>?` into:

‎compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_middle::mir::{
1313
use rustc_middle::ty::adjustment::PointerCast;
1414
use rustc_middle::ty::{self, RegionVid, TyCtxt};
1515
use rustc_span::symbol::Symbol;
16-
use rustc_span::Span;
16+
use rustc_span::{sym, DesugaringKind, Span};
1717

1818
use crate::region_infer::BlameConstraint;
1919
use crate::{
@@ -135,7 +135,16 @@ impl BorrowExplanation {
135135
should_note_order,
136136
} => {
137137
let local_decl = &body.local_decls[dropped_local];
138-
let (dtor_desc, type_desc) = match local_decl.ty.kind() {
138+
let mut ty = local_decl.ty;
139+
if local_decl.source_info.span.desugaring_kind() == Some(DesugaringKind::ForLoop) {
140+
if let ty::Adt(adt, substs) = local_decl.ty.kind() {
141+
if tcx.is_diagnostic_item(sym::Option, adt.did) {
142+
// in for loop desugaring, only look at the `Some(..)` inner type
143+
ty = substs.type_at(0);
144+
}
145+
}
146+
}
147+
let (dtor_desc, type_desc) = match ty.kind() {
139148
// If type is an ADT that implements Drop, then
140149
// simplify output by reporting just the ADT name.
141150
ty::Adt(adt, _substs) if adt.has_dtor(tcx) && !adt.is_box() => {

‎compiler/rustc_borrowck/src/diagnostics/mod.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,7 @@ use rustc_middle::mir::{
1313
use rustc_middle::ty::print::Print;
1414
use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt};
1515
use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult};
16-
use rustc_span::{
17-
hygiene::{DesugaringKind, ForLoopLoc},
18-
symbol::sym,
19-
Span,
20-
};
16+
use rustc_span::{hygiene::DesugaringKind, symbol::sym, Span};
2117
use rustc_target::abi::VariantIdx;
2218

2319
use super::borrow_set::BorrowData;
@@ -955,10 +951,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
955951
let kind = kind.unwrap_or_else(|| {
956952
// This isn't a 'special' use of `self`
957953
debug!("move_spans: method_did={:?}, fn_call_span={:?}", method_did, fn_call_span);
958-
let implicit_into_iter = matches!(
959-
fn_call_span.desugaring_kind(),
960-
Some(DesugaringKind::ForLoop(ForLoopLoc::IntoIter))
961-
);
954+
let implicit_into_iter = Some(method_did) == tcx.lang_items().into_iter_fn()
955+
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::ForLoop);
962956
let parent_self_ty = parent
963957
.filter(|did| tcx.def_kind(*did) == rustc_hir::def::DefKind::Impl)
964958
.and_then(|did| match tcx.type_of(did).kind() {

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -445,15 +445,23 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
445445
},
446446
))) => {
447447
// check if the RHS is from desugaring
448-
let locations = self.body.find_assignments(local);
449-
let opt_assignment_rhs_span = locations
450-
.first()
451-
.map(|&location| self.body.source_info(location).span);
452-
let opt_desugaring_kind =
453-
opt_assignment_rhs_span.and_then(|span| span.desugaring_kind());
454-
match opt_desugaring_kind {
448+
let opt_assignment_rhs_span =
449+
self.body.find_assignments(local).first().map(|&location| {
450+
let stmt = &self.body[location.block].statements
451+
[location.statement_index];
452+
match stmt.kind {
453+
mir::StatementKind::Assign(box (
454+
_,
455+
mir::Rvalue::Use(mir::Operand::Copy(place)),
456+
)) => {
457+
self.body.local_decls[place.local].source_info.span
458+
}
459+
_ => self.body.source_info(location).span,
460+
}
461+
});
462+
match opt_assignment_rhs_span.and_then(|s| s.desugaring_kind()) {
455463
// on for loops, RHS points to the iterator part
456-
Some(DesugaringKind::ForLoop(_)) => {
464+
Some(DesugaringKind::ForLoop) => {
457465
self.suggest_similar_mut_method_for_for_loop(&mut err);
458466
Some((
459467
false,

‎compiler/rustc_hir/src/hir.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1821,8 +1821,6 @@ impl<'hir> QPath<'hir> {
18211821
pub enum LocalSource {
18221822
/// A `match _ { .. }`.
18231823
Normal,
1824-
/// A desugared `for _ in _ { .. }` loop.
1825-
ForLoopDesugar,
18261824
/// When lowering async functions, we create locals within the `async move` so that
18271825
/// all parameters are dropped after the future is polled.
18281826
///

‎compiler/rustc_hir/src/pat_util.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::def::{CtorOf, DefKind, Res};
22
use crate::def_id::DefId;
33
use crate::hir::{self, HirId, PatKind};
44
use rustc_data_structures::stable_set::FxHashSet;
5+
use rustc_span::hygiene::DesugaringKind;
56
use rustc_span::symbol::Ident;
67
use rustc_span::Span;
78

@@ -143,4 +144,14 @@ impl hir::Pat<'_> {
143144
});
144145
result
145146
}
147+
148+
/// If the pattern is `Some(<pat>)` from a desugared for loop, returns the inner pattern
149+
pub fn for_loop_some(&self) -> Option<&Self> {
150+
if self.span.desugaring_kind() == Some(DesugaringKind::ForLoop) {
151+
if let hir::PatKind::Struct(_, [pat_field], _) = self.kind {
152+
return Some(pat_field.pat);
153+
}
154+
}
155+
None
156+
}
146157
}

‎compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,12 @@ use rustc_hir as hir;
55
use rustc_hir::def::{DefKind, Namespace};
66
use rustc_hir::def_id::DefId;
77
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
8-
use rustc_hir::{Body, Expr, ExprKind, FnRetTy, HirId, Local, Pat};
8+
use rustc_hir::{Body, Expr, ExprKind, FnRetTy, HirId, Local, MatchSource, Pat};
99
use rustc_middle::hir::map::Map;
1010
use rustc_middle::infer::unify_key::ConstVariableOriginKind;
1111
use rustc_middle::ty::print::Print;
1212
use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
1313
use rustc_middle::ty::{self, DefIdTree, InferConst, Ty, TyCtxt};
14-
use rustc_span::source_map::DesugaringKind;
1514
use rustc_span::symbol::kw;
1615
use rustc_span::Span;
1716
use std::borrow::Cow;
@@ -26,6 +25,7 @@ struct FindHirNodeVisitor<'a, 'tcx> {
2625
found_closure: Option<&'tcx Expr<'tcx>>,
2726
found_method_call: Option<&'tcx Expr<'tcx>>,
2827
found_exact_method_call: Option<&'tcx Expr<'tcx>>,
28+
found_for_loop_iter: Option<&'tcx Expr<'tcx>>,
2929
found_use_diagnostic: Option<UseDiagnostic<'tcx>>,
3030
}
3131

@@ -41,6 +41,7 @@ impl<'a, 'tcx> FindHirNodeVisitor<'a, 'tcx> {
4141
found_closure: None,
4242
found_method_call: None,
4343
found_exact_method_call: None,
44+
found_for_loop_iter: None,
4445
found_use_diagnostic: None,
4546
}
4647
}
@@ -111,6 +112,15 @@ impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> {
111112
}
112113

113114
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
115+
if let ExprKind::Match(scrutinee, [_, arm], MatchSource::ForLoopDesugar) = expr.kind {
116+
if let Some(pat) = arm.pat.for_loop_some() {
117+
if let Some(ty) = self.node_ty_contains_target(pat.hir_id) {
118+
self.found_for_loop_iter = Some(scrutinee);
119+
self.found_node_ty = Some(ty);
120+
return;
121+
}
122+
}
123+
}
114124
if let ExprKind::MethodCall(_, call_span, exprs, _) = expr.kind {
115125
if call_span == self.target_span
116126
&& Some(self.target)
@@ -643,10 +653,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
643653
let msg = if let Some(simple_ident) = pattern.simple_ident() {
644654
match pattern.span.desugaring_kind() {
645655
None => format!("consider giving `{}` {}", simple_ident, suffix),
646-
Some(DesugaringKind::ForLoop(_)) => {
647-
"the element type for this iterator is not specified".to_string()
648-
}
649-
_ => format!("this needs {}", suffix),
656+
Some(_) => format!("this needs {}", suffix),
650657
}
651658
} else {
652659
format!("consider giving this pattern {}", suffix)
@@ -719,6 +726,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
719726
// = note: type must be known at this point
720727
self.annotate_method_call(segment, e, &mut err);
721728
}
729+
} else if let Some(scrutinee) = local_visitor.found_for_loop_iter {
730+
err.span_label(
731+
scrutinee.span,
732+
"the element type for this iterator is not specified".to_string(),
733+
);
722734
}
723735
// Instead of the following:
724736
// error[E0282]: type annotations needed

0 commit comments

Comments
 (0)