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.

Sign up for GitHub

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

Simplify for loop desugar #90352

Merged
merged 4 commits into from
Nov 22, 2021
Merged

Simplify for loop desugar #90352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c83f8c
Simplify for loop desugar
camsteffen Oct 26, 2021
9d5ea8f
Fix Clippy with changed for loop desugar
camsteffen Oct 27, 2021
7508a9b
clippy: Fix pattern_type_mismatch for loop
camsteffen Nov 19, 2021
66da8fa
Bless coverage tests
camsteffen Nov 19, 2021
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
137 changes: 38 additions & 99 deletions compiler/rustc_ast_lowering/src/expr.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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