Skip to content

Commit

Permalink
Auto merge of #89841 - cormacrelf:let-else-typed, r=nagisa
Browse files Browse the repository at this point in the history
Implement let-else type annotations natively

Tracking issue: #87335

Fixes #89688, fixes #89807, edit: fixes  #89960 as well

As explained in #89688 (comment), the previous desugaring moved the let-else scrutinee into a dummy variable, which meant if you wanted to refer to it again in the else block, it had moved.

This introduces a new hir type, ~~`hir::LetExpr`~~ `hir::Let`, which takes over all the fields of `hir::ExprKind::Let(...)` and adds an optional type annotation. The `hir::Let` is then treated like a `hir::Local` when type checking a function body, specifically:

* `GatherLocalsVisitor` overrides a new `Visitor::visit_let_expr` and does pretty much exactly what it does for `visit_local`, assigning a local type to the `hir::Let` ~~(they could be deduplicated but they are right next to each other, so at least we know they're the same)~~
* It reuses the code in `check_decl_local` to typecheck the `hir::Let`, simply returning 'bool' for the expression type after doing that.

* ~~`FnCtxt::check_expr_let` passes this local type in to `demand_scrutinee_type`, and then imitates check_decl_local's pattern checking~~
* ~~`demand_scrutinee_type` (the blindest change for me, please give this extra scrutiny) uses this local type instead of of creating a new one~~
    * ~~Just realised the `check_expr_with_needs` was passing NoExpectation further down, need to pass the type there too. And apparently this Expectation API already exists.~~

Some other misc notes:

* ~~Is the clippy code supposed to be autoformatted? I tried not to give huge diffs but maybe some rustfmt changes simply haven't hit it yet.~~
* in `rustc_ast_lowering/src/block.rs`, I noticed some existing `self.alias_attrs()` calls in `LoweringContext::lower_stmts` seem to be copying attributes from the lowered locals/etc to the statements. Is that right? I'm new at this, I don't know.
  • Loading branch information
bors committed Dec 17, 2021
2 parents 7abab1e + fec8a50 commit dde825d
Show file tree
Hide file tree
Showing 46 changed files with 900 additions and 142 deletions.
32 changes: 10 additions & 22 deletions compiler/rustc_ast_lowering/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::{ImplTraitContext, ImplTraitPosition, LoweringContext};
use rustc_ast::{AttrVec, Block, BlockCheckMode, Expr, Local, LocalKind, Stmt, StmtKind};
use rustc_hir as hir;
use rustc_session::parse::feature_err;
use rustc_span::symbol::Ident;
use rustc_span::{sym, DesugaringKind};

use smallvec::SmallVec;
Expand Down Expand Up @@ -39,8 +38,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let hir_id = self.lower_node_id(s.id);
match &local.kind {
LocalKind::InitElse(init, els) => {
let (s, e) = self.lower_let_else(hir_id, local, init, els, tail);
stmts.push(s);
let e = self.lower_let_else(hir_id, local, init, els, tail);
expr = Some(e);
// remaining statements are in let-else expression
break;
Expand Down Expand Up @@ -125,36 +123,25 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
init: &Expr,
els: &Block,
tail: &[Stmt],
) -> (hir::Stmt<'hir>, &'hir hir::Expr<'hir>) {
) -> &'hir hir::Expr<'hir> {
let ty = local
.ty
.as_ref()
.map(|t| self.lower_ty(t, ImplTraitContext::Disallowed(ImplTraitPosition::Binding)));
let span = self.lower_span(local.span);
let span = self.mark_span_with_reason(DesugaringKind::LetElse, span, None);
let init = Some(self.lower_expr(init));
let val = Ident::with_dummy_span(sym::val);
let (pat, val_id) =
self.pat_ident_binding_mode(span, val, hir::BindingAnnotation::Unannotated);
let init = self.lower_expr(init);
let local_hir_id = self.lower_node_id(local.id);
self.lower_attrs(local_hir_id, &local.attrs);
// first statement which basically exists for the type annotation
let stmt = {
let local = self.arena.alloc(hir::Local {
let let_expr = {
let lex = self.arena.alloc(hir::Let {
hir_id: local_hir_id,
pat: self.lower_pat(&local.pat),
ty,
pat,
init,
span,
source: hir::LocalSource::Normal,
});
let kind = hir::StmtKind::Local(local);
hir::Stmt { hir_id: stmt_hir_id, kind, span }
};
let let_expr = {
let scrutinee = self.expr_ident(span, val, val_id);
let let_kind = hir::ExprKind::Let(self.lower_pat(&local.pat), scrutinee, span);
self.arena.alloc(self.expr(span, let_kind, AttrVec::new()))
self.arena.alloc(self.expr(span, hir::ExprKind::Let(lex), AttrVec::new()))
};
let then_expr = {
let (stmts, expr) = self.lower_stmts(tail);
Expand All @@ -165,9 +152,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let block = self.lower_block(els, false);
self.arena.alloc(self.expr_block(block, AttrVec::new()))
};
self.alias_attrs(let_expr.hir_id, local_hir_id);
self.alias_attrs(else_expr.hir_id, local_hir_id);
let if_expr = self.arena.alloc(hir::Expr {
hir_id: self.next_id(),
hir_id: stmt_hir_id,
span,
kind: hir::ExprKind::If(let_expr, then_expr, Some(else_expr)),
});
Expand All @@ -180,6 +168,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
)
.emit();
}
(stmt, if_expr)
if_expr
}
}
14 changes: 9 additions & 5 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
let ohs = self.lower_expr(ohs);
hir::ExprKind::AddrOf(k, m, ohs)
}
ExprKind::Let(ref pat, ref scrutinee, span) => hir::ExprKind::Let(
self.lower_pat(pat),
self.lower_expr(scrutinee),
self.lower_span(span),
),
ExprKind::Let(ref pat, ref scrutinee, span) => {
hir::ExprKind::Let(self.arena.alloc(hir::Let {
hir_id: self.next_id(),
span: self.lower_span(span),
pat: self.lower_pat(pat),
ty: None,
init: self.lower_expr(scrutinee),
}))
}
ExprKind::If(ref cond, ref then, ref else_opt) => {
self.lower_expr_if(cond, then, else_opt.as_deref())
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ macro_rules! arena_types {
[] generic_bound: rustc_hir::GenericBound<'tcx>,
[] generic_param: rustc_hir::GenericParam<'tcx>,
[] expr: rustc_hir::Expr<'tcx>,
[] let_expr: rustc_hir::Let<'tcx>,
[] expr_field: rustc_hir::ExprField<'tcx>,
[] pat_field: rustc_hir::PatField<'tcx>,
[] fn_decl: rustc_hir::FnDecl<'tcx>,
Expand Down
18 changes: 16 additions & 2 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1160,10 +1160,24 @@ pub struct Arm<'hir> {
pub body: &'hir Expr<'hir>,
}

/// Represents a `let <pat>[: <ty>] = <expr>` expression (not a Local), occurring in an `if-let` or
/// `let-else`, evaluating to a boolean. Typically the pattern is refutable.
///
/// In an if-let, imagine it as `if (let <pat> = <expr>) { ... }`; in a let-else, it is part of the
/// desugaring to if-let. Only let-else supports the type annotation at present.
#[derive(Debug, HashStable_Generic)]
pub struct Let<'hir> {
pub hir_id: HirId,
pub span: Span,
pub pat: &'hir Pat<'hir>,
pub ty: Option<&'hir Ty<'hir>>,
pub init: &'hir Expr<'hir>,
}

#[derive(Debug, HashStable_Generic)]
pub enum Guard<'hir> {
If(&'hir Expr<'hir>),
// FIXME use ExprKind::Let for this.
// FIXME use hir::Let for this.
IfLet(&'hir Pat<'hir>, &'hir Expr<'hir>),
}

Expand Down Expand Up @@ -1680,7 +1694,7 @@ pub enum ExprKind<'hir> {
///
/// These are not `Local` and only occur as expressions.
/// The `let Some(x) = foo()` in `if let Some(x) = foo()` is an example of `Let(..)`.
Let(&'hir Pat<'hir>, &'hir Expr<'hir>, Span),
Let(&'hir Let<'hir>),
/// An `if` block, with an optional else block.
///
/// I.e., `if <expr> { <expr> } else { <expr> }`.
Expand Down
16 changes: 12 additions & 4 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,9 @@ pub trait Visitor<'v>: Sized {
fn visit_expr(&mut self, ex: &'v Expr<'v>) {
walk_expr(self, ex)
}
fn visit_let_expr(&mut self, lex: &'v Let<'v>) {
walk_let_expr(self, lex)
}
fn visit_ty(&mut self, t: &'v Ty<'v>) {
walk_ty(self, t)
}
Expand Down Expand Up @@ -1126,6 +1129,14 @@ pub fn walk_anon_const<'v, V: Visitor<'v>>(visitor: &mut V, constant: &'v AnonCo
visitor.visit_nested_body(constant.body);
}

pub fn walk_let_expr<'v, V: Visitor<'v>>(visitor: &mut V, let_expr: &'v Let<'v>) {
// match the visit order in walk_local
visitor.visit_expr(let_expr.init);
visitor.visit_id(let_expr.hir_id);
visitor.visit_pat(let_expr.pat);
walk_list!(visitor, visit_ty, let_expr.ty);
}

pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>) {
visitor.visit_id(expression.hir_id);
match expression.kind {
Expand Down Expand Up @@ -1172,10 +1183,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
ExprKind::DropTemps(ref subexpression) => {
visitor.visit_expr(subexpression);
}
ExprKind::Let(ref pat, ref expr, _) => {
visitor.visit_expr(expr);
visitor.visit_pat(pat);
}
ExprKind::Let(ref let_expr) => visitor.visit_let_expr(let_expr),
ExprKind::If(ref cond, ref then, ref else_opt) => {
visitor.visit_expr(cond);
visitor.visit_expr(then);
Expand Down
16 changes: 10 additions & 6 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,13 +1101,17 @@ impl<'a> State<'a> {
}

/// Print a `let pat = expr` expression.
fn print_let(&mut self, pat: &hir::Pat<'_>, expr: &hir::Expr<'_>) {
self.word("let ");
fn print_let(&mut self, pat: &hir::Pat<'_>, ty: Option<&hir::Ty<'_>>, init: &hir::Expr<'_>) {
self.word_space("let");
self.print_pat(pat);
if let Some(ty) = ty {
self.word_space(":");
self.print_type(ty);
}
self.space();
self.word_space("=");
let npals = || parser::needs_par_as_let_scrutinee(expr.precedence().order());
self.print_expr_cond_paren(expr, Self::cond_needs_par(expr) || npals())
let npals = || parser::needs_par_as_let_scrutinee(init.precedence().order());
self.print_expr_cond_paren(init, Self::cond_needs_par(init) || npals())
}

// Does `expr` need parentheses when printed in a condition position?
Expand Down Expand Up @@ -1462,8 +1466,8 @@ impl<'a> State<'a> {
// Print `}`:
self.bclose_maybe_open(expr.span, true);
}
hir::ExprKind::Let(ref pat, ref scrutinee, _) => {
self.print_let(pat, scrutinee);
hir::ExprKind::Let(hir::Let { pat, ty, init, .. }) => {
self.print_let(pat, *ty, init);
}
hir::ExprKind::If(ref test, ref blk, ref elseopt) => {
self.print_if(&test, &blk, elseopt.as_ref().map(|e| &**e));
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,9 +605,10 @@ impl<'tcx> Cx<'tcx> {
},
Err(err) => bug!("invalid loop id for continue: {}", err),
},
hir::ExprKind::Let(ref pat, ref expr, _) => {
ExprKind::Let { expr: self.mirror_expr(expr), pat: self.pattern_from_hir(pat) }
}
hir::ExprKind::Let(let_expr) => ExprKind::Let {
expr: self.mirror_expr(let_expr.init),
pat: self.pattern_from_hir(let_expr.pat),
},
hir::ExprKind::If(cond, then, else_opt) => ExprKind::If {
if_then_scope: region::Scope {
id: then.hir_id.local_id,
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, '_, 'tcx> {
intravisit::walk_expr(self, ex);
match &ex.kind {
hir::ExprKind::Match(scrut, arms, source) => self.check_match(scrut, arms, *source),
hir::ExprKind::Let(pat, scrut, span) => self.check_let(pat, scrut, *span),
hir::ExprKind::Let(hir::Let { pat, init, span, .. }) => {
self.check_let(pat, init, *span)
}
_ => {}
}
}
Expand Down Expand Up @@ -148,9 +150,9 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
}
}

fn check_let(&mut self, pat: &'tcx hir::Pat<'tcx>, expr: &hir::Expr<'_>, span: Span) {
fn check_let(&mut self, pat: &'tcx hir::Pat<'tcx>, scrutinee: &hir::Expr<'_>, span: Span) {
self.check_patterns(pat, Refutable);
let mut cx = self.new_cx(expr.hir_id);
let mut cx = self.new_cx(scrutinee.hir_id);
let tpat = self.lower_pattern(&mut cx, pat, &mut false);
check_let_reachability(&mut cx, pat.hir_id, tpat, span);
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_passes/src/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,8 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
intravisit::walk_expr(self, expr);
}

hir::ExprKind::Let(ref pat, ..) => {
self.add_from_pat(pat);
hir::ExprKind::Let(let_expr) => {
self.add_from_pat(let_expr.pat);
intravisit::walk_expr(self, expr);
}

Expand Down Expand Up @@ -856,9 +856,9 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
})
}

hir::ExprKind::Let(ref pat, ref scrutinee, _) => {
let succ = self.propagate_through_expr(scrutinee, succ);
self.define_bindings_in_pat(pat, succ)
hir::ExprKind::Let(let_expr) => {
let succ = self.propagate_through_expr(let_expr.init, succ);
self.define_bindings_in_pat(let_expr.pat, succ)
}

// Note that labels have been resolved, so we don't need to look
Expand Down Expand Up @@ -1401,8 +1401,8 @@ fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr<'tcx>) {
}
}

hir::ExprKind::Let(ref pat, ..) => {
this.check_unused_vars_in_pat(pat, None, |_, _, _, _| {});
hir::ExprKind::Let(let_expr) => {
this.check_unused_vars_in_pat(let_expr.pat, None, |_, _, _, _| {});
}

// no correctness conditions related to liveness
Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
ExprKind::Ret(ref expr_opt) => self.check_expr_return(expr_opt.as_deref(), expr),
ExprKind::Let(pat, let_expr, _) => self.check_expr_let(let_expr, pat),
ExprKind::Let(let_expr) => self.check_expr_let(let_expr),
ExprKind::Loop(body, _, source, _) => {
self.check_expr_loop(body, source, expected, expr)
}
Expand Down Expand Up @@ -1044,10 +1044,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

fn check_expr_let(&self, expr: &'tcx hir::Expr<'tcx>, pat: &'tcx hir::Pat<'tcx>) -> Ty<'tcx> {
self.warn_if_unreachable(expr.hir_id, expr.span, "block in `let` expression");
let expr_ty = self.demand_scrutinee_type(expr, pat.contains_explicit_ref_binding(), false);
self.check_pat_top(pat, expr_ty, Some(expr.span), true);
fn check_expr_let(&self, let_expr: &'tcx hir::Let<'tcx>) -> Ty<'tcx> {
// for let statements, this is done in check_stmt
let init = let_expr.init;
self.warn_if_unreachable(init.hir_id, init.span, "block in `let` expression");
// otherwise check exactly as a let statement
self.check_decl(let_expr.into());
// but return a bool, for this is a boolean expression
self.tcx.types.bool
}

Expand Down
Loading

0 comments on commit dde825d

Please sign in to comment.