diff options
| author | bors <bors@rust-lang.org> | 2021-12-17 22:12:34 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-12-17 22:12:34 +0000 |
| commit | dde825db464b08d6f572766579dfb629b837368c (patch) | |
| tree | d10ae64adc03c0baadf998ef99b4a18c4c850ae7 | |
| parent | 7abab1efb21617ba6845fa86328dffa16cfcf1dc (diff) | |
| parent | fec8a507a27de1b08a0b95592dc8ec93bf0a321a (diff) | |
| download | rust-dde825db464b08d6f572766579dfb629b837368c.tar.gz rust-dde825db464b08d6f572766579dfb629b837368c.zip | |
Auto merge of #89841 - cormacrelf:let-else-typed, r=nagisa
Implement let-else type annotations natively
Tracking issue: #87335
Fixes #89688, fixes #89807, edit: fixes #89960 as well
As explained in https://github.com/rust-lang/rust/issues/89688#issuecomment-940405082, 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.
46 files changed, 900 insertions, 142 deletions
diff --git a/compiler/rustc_ast_lowering/src/block.rs b/compiler/rustc_ast_lowering/src/block.rs index 14a894d61f4..082c5bb7833 100644 --- a/compiler/rustc_ast_lowering/src/block.rs +++ b/compiler/rustc_ast_lowering/src/block.rs @@ -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; @@ -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; @@ -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); @@ -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)), }); @@ -180,6 +168,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ) .emit(); } - (stmt, if_expr) + if_expr } } diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index be67fe72127..6ee1dbe4ae3 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -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()) } diff --git a/compiler/rustc_hir/src/arena.rs b/compiler/rustc_hir/src/arena.rs index f19ca497d8b..edad00ed6a2 100644 --- a/compiler/rustc_hir/src/arena.rs +++ b/compiler/rustc_hir/src/arena.rs @@ -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>, diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index e2358aac1e7..64bd32b8ddc 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -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>), } @@ -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> }`. diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index a2f1db3579a..0fab7cbfeea 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -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) } @@ -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 { @@ -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); diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index 389e3845c56..2f5f158856f 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -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? @@ -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)); diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index b4005ccd1cc..092fe131174 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -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, diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index d74c53fae53..7a4fd6ffc4a 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -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) + } _ => {} } } @@ -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); } diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index 3d7a215754a..4ae1e5cee92 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -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); } @@ -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 @@ -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 diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 6a63feb09a1..4c385828ea9 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -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) } @@ -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 } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index b2641726075..38a8c1bb9f5 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -1,5 +1,6 @@ use crate::astconv::AstConv; use crate::check::coercion::CoerceMany; +use crate::check::gather_locals::Declaration; use crate::check::method::MethodCallee; use crate::check::Expectation::*; use crate::check::TupleArgumentsFlag::*; @@ -538,16 +539,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn check_decl_initializer( &self, - local: &'tcx hir::Local<'tcx>, + hir_id: hir::HirId, + pat: &'tcx hir::Pat<'tcx>, init: &'tcx hir::Expr<'tcx>, ) -> Ty<'tcx> { // FIXME(tschottdorf): `contains_explicit_ref_binding()` must be removed // for #42640 (default match binding modes). // // See #44848. - let ref_bindings = local.pat.contains_explicit_ref_binding(); + let ref_bindings = pat.contains_explicit_ref_binding(); - let local_ty = self.local_ty(init.span, local.hir_id).revealed_ty; + let local_ty = self.local_ty(init.span, hir_id).revealed_ty; if let Some(m) = ref_bindings { // Somewhat subtle: if we have a `ref` binding in the pattern, // we want to avoid introducing coercions for the RHS. This is @@ -565,29 +567,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - /// Type check a `let` statement. - pub fn check_decl_local(&self, local: &'tcx hir::Local<'tcx>) { + pub(in super::super) fn check_decl(&self, decl: Declaration<'tcx>) { // Determine and write the type which we'll check the pattern against. - let ty = self.local_ty(local.span, local.hir_id).decl_ty; - self.write_ty(local.hir_id, ty); + let decl_ty = self.local_ty(decl.span, decl.hir_id).decl_ty; + self.write_ty(decl.hir_id, decl_ty); // Type check the initializer. - if let Some(ref init) = local.init { - let init_ty = self.check_decl_initializer(local, &init); - self.overwrite_local_ty_if_err(local, ty, init_ty); + if let Some(ref init) = decl.init { + let init_ty = self.check_decl_initializer(decl.hir_id, decl.pat, &init); + self.overwrite_local_ty_if_err(decl.hir_id, decl.pat, decl_ty, init_ty); } // Does the expected pattern type originate from an expression and what is the span? - let (origin_expr, ty_span) = match (local.ty, local.init) { + let (origin_expr, ty_span) = match (decl.ty, decl.init) { (Some(ty), _) => (false, Some(ty.span)), // Bias towards the explicit user type. (_, Some(init)) => (true, Some(init.span)), // No explicit type; so use the scrutinee. _ => (false, None), // We have `let $pat;`, so the expected type is unconstrained. }; // Type check the pattern. Override if necessary to avoid knock-on errors. - self.check_pat_top(&local.pat, ty, ty_span, origin_expr); - let pat_ty = self.node_ty(local.pat.hir_id); - self.overwrite_local_ty_if_err(local, ty, pat_ty); + self.check_pat_top(&decl.pat, decl_ty, ty_span, origin_expr); + let pat_ty = self.node_ty(decl.pat.hir_id); + self.overwrite_local_ty_if_err(decl.hir_id, decl.pat, decl_ty, pat_ty); + } + + /// Type check a `let` statement. + pub fn check_decl_local(&self, local: &'tcx hir::Local<'tcx>) { + self.check_decl(local.into()); } pub fn check_stmt(&self, stmt: &'tcx hir::Stmt<'tcx>, is_last: bool) { @@ -891,17 +897,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn overwrite_local_ty_if_err( &self, - local: &'tcx hir::Local<'tcx>, + hir_id: hir::HirId, + pat: &'tcx hir::Pat<'tcx>, decl_ty: Ty<'tcx>, ty: Ty<'tcx>, ) { if ty.references_error() { // Override the types everywhere with `err()` to avoid knock on errors. - self.write_ty(local.hir_id, ty); - self.write_ty(local.pat.hir_id, ty); + self.write_ty(hir_id, ty); + self.write_ty(pat.hir_id, ty); let local_ty = LocalTy { decl_ty, revealed_ty: ty }; - self.locals.borrow_mut().insert(local.hir_id, local_ty); - self.locals.borrow_mut().insert(local.pat.hir_id, local_ty); + self.locals.borrow_mut().insert(hir_id, local_ty); + self.locals.borrow_mut().insert(pat.hir_id, local_ty); } } diff --git a/compiler/rustc_typeck/src/check/gather_locals.rs b/compiler/rustc_typeck/src/check/gather_locals.rs index 4ebfd7fd212..839bd56b396 100644 --- a/compiler/rustc_typeck/src/check/gather_locals.rs +++ b/compiler/rustc_typeck/src/check/gather_locals.rs @@ -7,6 +7,31 @@ use rustc_middle::ty::Ty; use rustc_span::Span; use rustc_trait_selection::traits; +/// A declaration is an abstraction of [hir::Local] and [hir::Let]. +/// +/// It must have a hir_id, as this is how we connect gather_locals to the check functions. +pub(super) struct Declaration<'a> { + pub hir_id: hir::HirId, + pub pat: &'a hir::Pat<'a>, + pub ty: Option<&'a hir::Ty<'a>>, + pub span: Span, + pub init: Option<&'a hir::Expr<'a>>, +} + +impl<'a> From<&'a hir::Local<'a>> for Declaration<'a> { + fn from(local: &'a hir::Local<'a>) -> Self { + let hir::Local { hir_id, pat, ty, span, init, .. } = *local; + Declaration { hir_id, pat, ty, span, init } + } +} + +impl<'a> From<&'a hir::Let<'a>> for Declaration<'a> { + fn from(let_expr: &'a hir::Let<'a>) -> Self { + let hir::Let { hir_id, pat, ty, span, init } = *let_expr; + Declaration { hir_id, pat, ty, span, init: Some(init) } + } +} + pub(super) struct GatherLocalsVisitor<'a, 'tcx> { fcx: &'a FnCtxt<'a, 'tcx>, // parameters are special cases of patterns, but we want to handle them as @@ -41,18 +66,12 @@ impl<'a, 'tcx> GatherLocalsVisitor<'a, 'tcx> { } } } -} - -impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> { - type Map = intravisit::ErasedMap<'tcx>; - - fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { - NestedVisitorMap::None - } - // Add explicitly-declared locals. - fn visit_local(&mut self, local: &'tcx hir::Local<'tcx>) { - let local_ty = match local.ty { + /// Allocates a [LocalTy] for a declaration, which may have a type annotation. If it does have + /// a type annotation, then the LocalTy stored will be the resolved type. This may be found + /// again during type checking by querying [FnCtxt::local_ty] for the same hir_id. + fn declare(&mut self, decl: Declaration<'tcx>) { + let local_ty = match decl.ty { Some(ref ty) => { let o_ty = self.fcx.to_ty(&ty); @@ -68,16 +87,34 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> { } None => None, }; - self.assign(local.span, local.hir_id, local_ty); + self.assign(decl.span, decl.hir_id, local_ty); debug!( "local variable {:?} is assigned type {}", - local.pat, - self.fcx.ty_to_string(&*self.fcx.locals.borrow().get(&local.hir_id).unwrap().decl_ty) + decl.pat, + self.fcx.ty_to_string(&*self.fcx.locals.borrow().get(&decl.hir_id).unwrap().decl_ty) ); + } +} + +impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> { + type Map = intravisit::ErasedMap<'tcx>; + + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::None + } + + // Add explicitly-declared locals. + fn visit_local(&mut self, local: &'tcx hir::Local<'tcx>) { + self.declare(local.into()); intravisit::walk_local(self, local); } + fn visit_let_expr(&mut self, let_expr: &'tcx hir::Let<'tcx>) { + self.declare(let_expr.into()); + intravisit::walk_let_expr(self, let_expr); + } + fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) { let old_outermost_fn_param_pat = self.outermost_fn_param_pat.replace(param.ty_span); intravisit::walk_param(self, param); diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 0896daf48b7..32b4018f626 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -229,8 +229,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } } - hir::ExprKind::Let(pat, ref expr, _) => { - self.walk_local(expr, pat, |t| t.borrow_expr(expr, ty::ImmBorrow)); + hir::ExprKind::Let(hir::Let { pat, init, .. }) => { + self.walk_local(init, pat, |t| t.borrow_expr(init, ty::ImmBorrow)); } hir::ExprKind::Match(ref discr, arms, _) => { diff --git a/src/test/ui/let-else/issue-89960.rs b/src/test/ui/let-else/issue-89960.rs deleted file mode 100644 index 8fd55adbfd4..00000000000 --- a/src/test/ui/let-else/issue-89960.rs +++ /dev/null @@ -1,7 +0,0 @@ -#![feature(let_else)] - -fn main() { - // FIXME: more precise diagnostics - let Some(ref mut meow) = Some(()) else { return }; - //~^ ERROR: cannot borrow value as mutable, as `val` is not declared as mutable -} diff --git a/src/test/ui/let-else/issue-89960.stderr b/src/test/ui/let-else/issue-89960.stderr deleted file mode 100644 index 697f04d6d27..00000000000 --- a/src/test/ui/let-else/issue-89960.stderr +++ /dev/null @@ -1,12 +0,0 @@ -error[E0596]: cannot borrow value as mutable, as `val` is not declared as mutable - --> $DIR/issue-89960.rs:5:14 - | -LL | let Some(ref mut meow) = Some(()) else { return }; - | ---------^^^^^^^^^^^^----------------------------- - | | | - | | cannot borrow as mutable - | help: consider changing this to be mutable: `mut val` - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/let-else/let-else-allow-unused.rs b/src/test/ui/let-else/let-else-allow-unused.rs new file mode 100644 index 00000000000..bcd8c987628 --- /dev/null +++ b/src/test/ui/let-else/let-else-allow-unused.rs @@ -0,0 +1,14 @@ +// check-pass +// issue #89807 + +#![feature(let_else)] + +#[deny(unused_variables)] + +fn main() { + let value = Some(String::new()); + #[allow(unused)] + let banana = 1; + #[allow(unused)] + let Some(chaenomeles) = value else { return }; // OK +} diff --git a/src/test/ui/let-else/let-else-binding-explicit-mut-annotated.rs b/src/test/ui/let-else/let-else-binding-explicit-mut-annotated.rs new file mode 100644 index 00000000000..b65fa13c1de --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-explicit-mut-annotated.rs @@ -0,0 +1,16 @@ +// from rfc2005 test suite + +#![feature(let_else)] + +// Verify the binding mode shifts - only when no `&` are auto-dereferenced is the +// final default binding mode mutable. + +fn main() { + let Some(n): &mut Option<i32> = &&Some(5i32) else { return }; //~ ERROR mismatched types + *n += 1; + let _ = n; + + let Some(n): &mut Option<i32> = &&mut Some(5i32) else { return }; //~ ERROR mismatched types + *n += 1; + let _ = n; +} diff --git a/src/test/ui/let-else/let-else-binding-explicit-mut-annotated.stderr b/src/test/ui/let-else/let-else-binding-explicit-mut-annotated.stderr new file mode 100644 index 00000000000..fdec7e7f6a7 --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-explicit-mut-annotated.stderr @@ -0,0 +1,21 @@ +error[E0308]: mismatched types + --> $DIR/let-else-binding-explicit-mut-annotated.rs:9:37 + | +LL | let Some(n): &mut Option<i32> = &&Some(5i32) else { return }; + | ^^^^^^^^^^^^ types differ in mutability + | + = note: expected mutable reference `&mut Option<i32>` + found reference `&&Option<i32>` + +error[E0308]: mismatched types + --> $DIR/let-else-binding-explicit-mut-annotated.rs:13:37 + | +LL | let Some(n): &mut Option<i32> = &&mut Some(5i32) else { return }; + | ^^^^^^^^^^^^^^^^ types differ in mutability + | + = note: expected mutable reference `&mut Option<i32>` + found reference `&&mut Option<i32>` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/let-else/let-else-binding-explicit-mut-borrow.rs b/src/test/ui/let-else/let-else-binding-explicit-mut-borrow.rs new file mode 100644 index 00000000000..63b35df76aa --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-explicit-mut-borrow.rs @@ -0,0 +1,13 @@ +#![feature(let_else)] + +// Slightly different from explicit-mut-annotated -- this won't show an error until borrowck. +// Should it show a type error instead? + +fn main() { + let Some(n): &mut Option<i32> = &mut &Some(5i32) else { + //~^ ERROR cannot borrow data in a `&` reference as mutable + return + }; + *n += 1; + let _ = n; +} diff --git a/src/test/ui/let-else/let-else-binding-explicit-mut-borrow.stderr b/src/test/ui/let-else/let-else-binding-explicit-mut-borrow.stderr new file mode 100644 index 00000000000..023fab8fe4a --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-explicit-mut-borrow.stderr @@ -0,0 +1,9 @@ +error[E0596]: cannot borrow data in a `&` reference as mutable + --> $DIR/let-else-binding-explicit-mut-borrow.rs:7:37 + | +LL | let Some(n): &mut Option<i32> = &mut &Some(5i32) else { + | ^^^^^^^^^^^^^^^^ cannot borrow as mutable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/let-else/let-else-binding-explicit-mut-pass.rs b/src/test/ui/let-else/let-else-binding-explicit-mut-pass.rs new file mode 100644 index 00000000000..305be922192 --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-explicit-mut-pass.rs @@ -0,0 +1,13 @@ +// check-pass + +#![feature(let_else)] + +fn main() { + let Some(n) = &mut &mut Some(5i32) else { return; }; + *n += 1; // OK + let _ = n; + + let Some(n): &mut Option<i32> = &mut &mut Some(5i32) else { return; }; + *n += 1; // OK + let _ = n; +} diff --git a/src/test/ui/let-else/let-else-binding-explicit-mut.rs b/src/test/ui/let-else/let-else-binding-explicit-mut.rs new file mode 100644 index 00000000000..dbe4715b1a9 --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-explicit-mut.rs @@ -0,0 +1,20 @@ +// from rfc2005 test suite + +#![feature(let_else)] + +// Verify the binding mode shifts - only when no `&` are auto-dereferenced is the +// final default binding mode mutable. + +fn main() { + let Some(n) = &&Some(5i32) else { return }; + *n += 1; //~ ERROR cannot assign to `*n`, which is behind a `&` reference + let _ = n; + + let Some(n) = &mut &Some(5i32) else { return }; + *n += 1; //~ ERROR cannot assign to `*n`, which is behind a `&` reference + let _ = n; + + let Some(n) = &&mut Some(5i32) else { return }; + *n += 1; //~ ERROR cannot assign to `*n`, which is behind a `&` reference + let _ = n; +} diff --git a/src/test/ui/let-else/let-else-binding-explicit-mut.stderr b/src/test/ui/let-else/let-else-binding-explicit-mut.stderr new file mode 100644 index 00000000000..45f2b6b3bce --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-explicit-mut.stderr @@ -0,0 +1,21 @@ +error[E0594]: cannot assign to `*n`, which is behind a `&` reference + --> $DIR/let-else-binding-explicit-mut.rs:10:5 + | +LL | *n += 1; + | ^^^^^^^ `n` is a `&` reference, so the data it refers to cannot be written + +error[E0594]: cannot assign to `*n`, which is behind a `&` reference + --> $DIR/let-else-binding-explicit-mut.rs:14:5 + | +LL | *n += 1; + | ^^^^^^^ `n` is a `&` reference, so the data it refers to cannot be written + +error[E0594]: cannot assign to `*n`, which is behind a `&` reference + --> $DIR/let-else-binding-explicit-mut.rs:18:5 + | +LL | *n += 1; + | ^^^^^^^ `n` is a `&` reference, so the data it refers to cannot be written + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0594`. diff --git a/src/test/ui/let-else/let-else-binding-immutable.rs b/src/test/ui/let-else/let-else-binding-immutable.rs new file mode 100644 index 00000000000..96de0ffe26e --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-immutable.rs @@ -0,0 +1,10 @@ +// from rfc2005 test suite + +#![feature(let_else)] + +pub fn main() { + let Some(x) = &Some(3) else { + panic!(); + }; + *x += 1; //~ ERROR: cannot assign to `*x`, which is behind a `&` reference +} diff --git a/src/test/ui/let-else/let-else-binding-immutable.stderr b/src/test/ui/let-else/let-else-binding-immutable.stderr new file mode 100644 index 00000000000..dd1365a9ef0 --- /dev/null +++ b/src/test/ui/let-else/let-else-binding-immutable.stderr @@ -0,0 +1,9 @@ +error[E0594]: cannot assign to `*x`, which is behind a `&` reference + --> $DIR/let-else-binding-immutable.rs:9:5 + | +LL | *x += 1; + | ^^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0594`. diff --git a/src/test/ui/let-else/let-else-bindings.rs b/src/test/ui/let-else/let-else-bindings.rs new file mode 100644 index 00000000000..d5121e744da --- /dev/null +++ b/src/test/ui/let-else/let-else-bindings.rs @@ -0,0 +1,75 @@ +// run-pass +// adapted from src/test/ui/binding/if-let.rs +#![feature(let_else)] +#![allow(dead_code)] + +fn none() -> bool { + let None = Some("test") else { + return true; + }; + false +} + +fn ok() -> bool { + let Ok(()) = Err::<(),&'static str>("test") else { + return true; + }; + false +} + +pub fn main() { + let x = Some(3); + let Some(y) = x else { + panic!("let-else panicked"); + }; + assert_eq!(y, 3); + let Some(_) = x else { + panic!("bad match"); + }; + assert!(none()); + assert!(ok()); + + assert!((|| { + let 1 = 2 else { + return true; + }; + false + })()); + + enum Foo { + One, + Two(usize), + Three(String, isize), + } + + let foo = Foo::Three("three".to_string(), 42); + let one = || { + let Foo::One = foo else { + return true; + }; + false + }; + assert!(one()); + let two = || { + let Foo::Two(_x) = foo else { + return true; + }; + false + }; + assert!(two()); + let three = || { + let Foo::Three(s, _x) = foo else { + return false; + }; + s == "three" + }; + assert!(three()); + + let a@Foo::Two(_) = Foo::Two(42_usize) else { + panic!("bad match") + }; + let Foo::Two(b) = a else { + panic!("panic in nested `if let`"); + }; + assert_eq!(b, 42_usize); +} diff --git a/src/test/ui/let-else/let-else-deref-coercion-annotated.rs b/src/test/ui/let-else/let-else-deref-coercion-annotated.rs new file mode 100644 index 00000000000..65d88a6d828 --- /dev/null +++ b/src/test/ui/let-else/let-else-deref-coercion-annotated.rs @@ -0,0 +1,77 @@ +// check-pass +// +// Taken from https://github.com/rust-lang/rust/blob/6cc0a764e082d9c0abcf37a768d5889247ba13e2/compiler/rustc_typeck/src/check/_match.rs#L445-L462 +// +// We attempt to `let Bar::Present(_): &mut Bar = foo else { ... }` where foo is meant to +// Deref/DerefMut to Bar. You can do this with an irrefutable binding, so it should work with +// let-else too. + +#![feature(let_else)] +use std::ops::{Deref, DerefMut}; + +struct Foo(Bar); + +enum Bar { + Present(u32), + Absent, +} +impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Bar { + &self.0 + } +} +impl DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Bar { + &mut self.0 + } +} +impl Bar { + fn bar(&self) -> Option<u32> { + let Bar::Present(z): &Bar = self else { + return None; + }; + return Some(*z); + } +} +impl Foo { + fn set_bar_annotated(&mut self, value: u32) { + let Bar::Present(z): &mut Bar = self else { // OK + return; + }; + *z = value; + } +} + +fn main() { + let mut foo = Foo(Bar::Present(1)); + foo.set_bar_annotated(42); + assert_eq!(foo.bar(), Some(42)); + irrefutable::inner(); +} + +// The original, to show it works for irrefutable let decls +mod irrefutable { + use std::ops::{Deref, DerefMut}; + struct Foo(Bar); + struct Bar(u32); + impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Bar { + &self.0 + } + } + impl DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Bar { + &mut self.0 + } + } + fn foo(x: &mut Foo) { + let Bar(z): &mut Bar = x; // OK + *z = 42; + assert_eq!((x.0).0, 42); + } + pub fn inner() { + foo(&mut Foo(Bar(1))); + } +} diff --git a/src/test/ui/let-else/let-else-deref-coercion.rs b/src/test/ui/let-else/let-else-deref-coercion.rs new file mode 100644 index 00000000000..87489d84bbf --- /dev/null +++ b/src/test/ui/let-else/let-else-deref-coercion.rs @@ -0,0 +1,75 @@ +// Taken from https://github.com/rust-lang/rust/blob/6cc0a764e082d9c0abcf37a768d5889247ba13e2/compiler/rustc_typeck/src/check/_match.rs#L445-L462 +// +// We attempt to `let Bar::Present(_) = foo else { ... }` where foo is meant to Deref/DerefMut to +// Bar. This fails, you must add a type annotation like `let _: &mut Bar = _ else { ... }` + +#![feature(let_else)] +use std::ops::{Deref, DerefMut}; + +struct Foo(Bar); + +enum Bar { + Present(u32), + Absent, +} +impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Bar { + &self.0 + } +} +impl DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Bar { + &mut self.0 + } +} +impl Bar { + fn bar(&self) -> Option<u32> { + let Bar::Present(z): &Bar = self else { + return None; + }; + return Some(*z); + } +} +impl Foo { + // Try without the type annotation + fn set_bar_unannotated(&mut self, value: u32) { + let Bar::Present(z) = self else { //~ ERROR mismatched types + return; + }; + *z = value; + } +} + +fn main() { + let mut foo = Foo(Bar::Present(1)); + foo.set_bar_unannotated(54); + assert_eq!(foo.bar(), Some(54)); + irrefutable::inner(); +} + +// The original, to show it fails for irrefutable let decls +mod irrefutable { + use std::ops::{Deref, DerefMut}; + struct Foo(Bar); + struct Bar(u32); + impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Bar { + &self.0 + } + } + impl DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Bar { + &mut self.0 + } + } + fn foo(x: &mut Foo) { + let Bar(z) = x; //~ ERROR mismatched types + *z = 54; + assert_eq!((x.0).0, 54); + } + pub fn inner() { + foo(&mut Foo(Bar(1))); + } +} diff --git a/src/test/ui/let-else/let-else-deref-coercion.stderr b/src/test/ui/let-else/let-else-deref-coercion.stderr new file mode 100644 index 00000000000..addcd798f4f --- /dev/null +++ b/src/test/ui/let-else/let-else-deref-coercion.stderr @@ -0,0 +1,19 @@ +error[E0308]: mismatched types + --> $DIR/let-else-deref-coercion.rs:37:13 + | +LL | let Bar::Present(z) = self else { + | ^^^^^^^^^^^^^^^ ---- this expression has type `&mut Foo` + | | + | expected struct `Foo`, found enum `Bar` + +error[E0308]: mismatched types + --> $DIR/let-else-deref-coercion.rs:68:13 + | +LL | let Bar(z) = x; + | ^^^^^^ - this expression has type `&mut irrefutable::Foo` + | | + | expected struct `irrefutable::Foo`, found struct `irrefutable::Bar` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/let-else/let-else-no-double-error.rs b/src/test/ui/let-else/let-else-no-double-error.rs new file mode 100644 index 00000000000..35dcdd3f6be --- /dev/null +++ b/src/test/ui/let-else/let-else-no-double-error.rs @@ -0,0 +1,12 @@ +// from rfc2005 test suite + +#![feature(let_else)] + +// Without caching type lookups in FnCtxt.resolve_ty_and_def_ufcs +// the error below would be reported twice (once when checking +// for a non-ref pattern, once when processing the pattern). + +fn main() { + let foo = 22; + let u32::XXX = foo else { return }; //~ ERROR: no associated item named `XXX` found for type `u32` in the current scope [E0599] +} diff --git a/src/test/ui/let-else/let-else-no-double-error.stderr b/src/test/ui/let-else/let-else-no-double-error.stderr new file mode 100644 index 00000000000..941e588b176 --- /dev/null +++ b/src/test/ui/let-else/let-else-no-double-error.stderr @@ -0,0 +1,9 @@ +error[E0599]: no associated item named `XXX` found for type `u32` in the current scope + --> $DIR/let-else-no-double-error.rs:11:14 + | +LL | let u32::XXX = foo else { return }; + | ^^^ associated item not found in `u32` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. diff --git a/src/test/ui/let-else/let-else-non-copy.rs b/src/test/ui/let-else/let-else-non-copy.rs new file mode 100644 index 00000000000..79ed82dd124 --- /dev/null +++ b/src/test/ui/let-else/let-else-non-copy.rs @@ -0,0 +1,45 @@ +// run-pass +// +// This is derived from a change to compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs, in +// preparation for adopting let-else within the compiler (thanks @est31): +// +// ``` +// - let place = if let mir::VarDebugInfoContents::Place(p) = var.value { p } else { continue }; +// + let mir::VarDebugInfoContents::Place(place) = var.value else { continue }; +// ``` +// +// The move was due to mir::Place being Copy, but mir::VarDebugInfoContents not being Copy. + +#![feature(let_else)] + +#[derive(Copy, Clone)] +struct Copyable; + +enum NonCopy { + Thing(Copyable), + #[allow(unused)] + Other, +} + +struct Wrapper { + field: NonCopy, +} + +fn let_else() { + let vec = vec![Wrapper { field: NonCopy::Thing(Copyable) }]; + for item in &vec { + let NonCopy::Thing(_copyable) = item.field else { continue }; + } +} + +fn if_let() { + let vec = vec![Wrapper { field: NonCopy::Thing(Copyable) }]; + for item in &vec { + let _copyable = if let NonCopy::Thing(copyable) = item.field { copyable } else { continue }; + } +} + +fn main() { + let_else(); + if_let(); +} diff --git a/src/test/ui/let-else/let-else-ref-bindings-pass.rs b/src/test/ui/let-else/let-else-ref-bindings-pass.rs new file mode 100644 index 00000000000..f4abd6cc2df --- /dev/null +++ b/src/test/ui/let-else/let-else-ref-bindings-pass.rs @@ -0,0 +1,71 @@ +// check-pass + +#![feature(let_else)] +#![allow(unused_variables)] + +fn ref_() { + let bytes: Vec<u8> = b"Hello"[..].to_vec(); + let some = Some(bytes); + + let Some(ref a) = Some(()) else { return }; + + // | ref | type annotation | & | + // | --- | --------------- | - | + // | x | x | | error + // | x | x | x | error + // | | x | | error + // | | x | x | error + // | x | | | + let Some(ref a) = some else { return }; // OK + let b: &[u8] = a; + + // | x | | x | + let Some(ref a) = &some else { return }; // OK + let b: &[u8] = a; + + + // | | | x | + let Some(a) = &some else { return }; // OK + let b: &[u8] = a; + + let Some(a): Option<&[u8]> = some.as_deref() else { return }; // OK + let b: &[u8] = a; + let Some(ref a): Option<&[u8]> = some.as_deref() else { return }; // OK + let b: &[u8] = a; +} + +fn ref_mut() { + // This `ref mut` case had an ICE, see issue #89960 + let Some(ref mut a) = Some(()) else { return }; + + let bytes: Vec<u8> = b"Hello"[..].to_vec(); + let mut some = Some(bytes); + + // | ref mut | type annotation | &mut | + // | ------- | --------------- | ---- | + // | x | x | | error + // | x | x | x | error + // | | x | | error + // | | x | x | error + // | x | | | + let Some(ref mut a) = some else { return }; // OK + let b: &mut [u8] = a; + + // | x | | x | + let Some(ref mut a) = &mut some else { return }; // OK + let b: &mut [u8] = a; + + // | | | x | + let Some(a) = &mut some else { return }; // OK + let b: &mut [u8] = a; + + let Some(a): Option<&mut [u8]> = some.as_deref_mut() else { return }; // OK + let b: &mut [u8] = a; + let Some(ref mut a): Option<&mut [u8]> = some.as_deref_mut() else { return }; // OK + let b: &mut [u8] = a; +} + +fn main() { + ref_(); + ref_mut(); +} diff --git a/src/test/ui/let-else/let-else-ref-bindings.rs b/src/test/ui/let-else/let-else-ref-bindings.rs new file mode 100644 index 00000000000..a4cd8e8c47d --- /dev/null +++ b/src/test/ui/let-else/let-else-ref-bindings.rs @@ -0,0 +1,62 @@ +#![feature(let_else)] +#![allow(unused_variables)] + +fn ref_() { + let bytes: Vec<u8> = b"Hello"[..].to_vec(); + let some = Some(bytes); + + let Some(ref a) = Some(()) else { return }; + + // | ref | type annotation | & | + // | --- | --------------- | - | + // | x | | | OK + // | x | | x | OK + // | | | x | OK + // | x | x | | + let Some(ref a): Option<&[u8]> = some else { return }; //~ ERROR mismatched types + let b: & [u8] = a; + + // | x | x | x | + let Some(ref a): Option<&[u8]> = &some else { return }; //~ ERROR mismatched types + let b: & [u8] = a; + + // | | x | | + let Some(a): Option<&[u8]> = some else { return }; //~ ERROR mismatched types + let b: &[u8] = a; + // | | x | x | + let Some(a): Option<&[u8]> = &some else { return }; //~ ERROR mismatched types + let b: &[u8] = a; +} + +fn ref_mut() { + // This `ref mut` case had an ICE, see issue #89960 + let Some(ref mut a) = Some(()) else { return }; + + let bytes: Vec<u8> = b"Hello"[..].to_vec(); + let mut some = Some(bytes); + + // | ref mut | type annotation | &mut | + // | ------- | --------------- | ---- | + // | x | | | OK + // | x | | x | OK + // | | | x | OK + // | x | x | | + let Some(ref mut a): Option<&mut [u8]> = some else { return }; //~ ERROR mismatched types + let b: &mut [u8] = a; + + // | x | x | x | (nope) + let Some(ref mut a): Option<&mut [u8]> = &mut some else { return }; //~ ERROR mismatched types + let b: &mut [u8] = a; + + // | | x | | + let Some(a): Option<&mut [u8]> = some else { return }; //~ ERROR mismatched types + let b: &mut [u8] = a; + // | | x | x | + let Some(a): Option<&mut [u8]> = &mut some else { return }; //~ ERROR mismatched types + let b: &mut [u8] = a; +} + +fn main() { + ref_(); + ref_mut(); +} diff --git a/src/test/ui/let-else/let-else-ref-bindings.stderr b/src/test/ui/let-else/let-else-ref-bindings.stderr new file mode 100644 index 00000000000..650f4ec5e77 --- /dev/null +++ b/src/test/ui/let-else/let-else-ref-bindings.stderr @@ -0,0 +1,75 @@ +error[E0308]: mismatched types + --> $DIR/let-else-ref-bindings.rs:16:38 + | +LL | let Some(ref a): Option<&[u8]> = some else { return }; + | ^^^^ expected `&[u8]`, found struct `Vec` + | + = note: expected enum `Option<&[u8]>` + found enum `Option<Vec<u8>>` + +error[E0308]: mismatched types + --> $DIR/let-else-ref-bindings.rs:20:38 + | +LL | let Some(ref a): Option<&[u8]> = &some else { return }; + | ^^^^^ expected enum `Option`, found `&Option<Vec<u8>>` + | + = note: expected enum `Option<&[u8]>` + found reference `&Option<Vec<u8>>` + +error[E0308]: mismatched types + --> $DIR/let-else-ref-bindings.rs:24:34 + | +LL | let Some(a): Option<&[u8]> = some else { return }; + | ^^^^ expected `&[u8]`, found struct `Vec` + | + = note: expected enum `Option<&[u8]>` + found enum `Option<Vec<u8>>` + +error[E0308]: mismatched types + --> $DIR/let-else-ref-bindings.rs:27:34 + | +LL | let Some(a): Option<&[u8]> = &some else { return }; + | ^^^^^ expected enum `Option`, found `&Option<Vec<u8>>` + | + = note: expected enum `Option<&[u8]>` + found reference `&Option<Vec<u8>>` + +error[E0308]: mismatched types + --> $DIR/let-else-ref-bindings.rs:44:46 + | +LL | let Some(ref mut a): Option<&mut [u8]> = some else { return }; + | ^^^^ expected `&mut [u8]`, found struct `Vec` + | + = note: expected enum `Option<&mut [u8]>` + found enum `Option<Vec<u8>>` + +error[E0308]: mismatched types + --> $DIR/let-else-ref-bindings.rs:48:46 + | +LL | let Some(ref mut a): Option<&mut [u8]> = &mut some else { return }; + | ^^^^^^^^^ expected enum `Option`, found mutable reference + | + = note: expected enum `Option<&mut [u8]>` + found mutable reference `&mut Option<Vec<u8>>` + +error[E0308]: mismatched types + --> $DIR/let-else-ref-bindings.rs:52:38 + | +LL | let Some(a): Option<&mut [u8]> = some else { return }; + | ^^^^ expected `&mut [u8]`, found struct `Vec` + | + = note: expected enum `Option<&mut [u8]>` + found enum `Option<Vec<u8>>` + +error[E0308]: mismatched types + --> $DIR/let-else-ref-bindings.rs:55:38 + | +LL | let Some(a): Option<&mut [u8]> = &mut some else { return }; + | ^^^^^^^^^ expected enum `Option`, found mutable reference + | + = note: expected enum `Option<&mut [u8]>` + found mutable reference `&mut Option<Vec<u8>>` + +error: aborting due to 8 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/let-else/let-else-source-expr-nomove-pass.rs b/src/test/ui/let-else/let-else-source-expr-nomove-pass.rs new file mode 100644 index 00000000000..2aa17ae8ceb --- /dev/null +++ b/src/test/ui/let-else/let-else-source-expr-nomove-pass.rs @@ -0,0 +1,17 @@ +// run-pass +// issue #89688 + +#![feature(let_else)] + +fn example_let_else(value: Option<String>) { + let Some(inner) = value else { + println!("other: {:?}", value); // OK + return; + }; + println!("inner: {}", inner); +} + +fn main() { + example_let_else(Some("foo".into())); + example_let_else(None); +} diff --git a/src/tools/clippy/clippy_lints/src/equatable_if_let.rs b/src/tools/clippy/clippy_lints/src/equatable_if_let.rs index 8905cc0de45..06d128f5527 100644 --- a/src/tools/clippy/clippy_lints/src/equatable_if_let.rs +++ b/src/tools/clippy/clippy_lints/src/equatable_if_let.rs @@ -67,20 +67,20 @@ fn is_structural_partial_eq(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx impl<'tcx> LateLintPass<'tcx> for PatternEquality { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if_chain! { - if let ExprKind::Let(pat, exp, _) = expr.kind; - if unary_pattern(pat); - let exp_ty = cx.typeck_results().expr_ty(exp); - let pat_ty = cx.typeck_results().pat_ty(pat); + if let ExprKind::Let(let_expr) = expr.kind; + if unary_pattern(let_expr.pat); + let exp_ty = cx.typeck_results().expr_ty(let_expr.init); + let pat_ty = cx.typeck_results().pat_ty(let_expr.pat); if is_structural_partial_eq(cx, exp_ty, pat_ty); then { let mut applicability = Applicability::MachineApplicable; - let pat_str = match pat.kind { + let pat_str = match let_expr.pat.kind { PatKind::Struct(..) => format!( "({})", - snippet_with_context(cx, pat.span, expr.span.ctxt(), "..", &mut applicability).0, + snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0, ), - _ => snippet_with_context(cx, pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(), + _ => snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(), }; span_lint_and_sugg( cx, @@ -90,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for PatternEquality { "try", format!( "{} == {}", - snippet_with_context(cx, exp.span, expr.span.ctxt(), "..", &mut applicability).0, + snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0, pat_str, ), applicability, diff --git a/src/tools/clippy/clippy_lints/src/loops/never_loop.rs b/src/tools/clippy/clippy_lints/src/loops/never_loop.rs index 68ffcd1abfb..a3aa6be6afd 100644 --- a/src/tools/clippy/clippy_lints/src/loops/never_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/never_loop.rs @@ -115,12 +115,12 @@ fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult { | ExprKind::Unary(_, e) | ExprKind::Cast(e, _) | ExprKind::Type(e, _) - | ExprKind::Let(_, e, _) | ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) | ExprKind::Struct(_, _, Some(e)) | ExprKind::Repeat(e, _) | ExprKind::DropTemps(e) => never_loop_expr(e, main_loop_id), + ExprKind::Let(let_expr) => never_loop_expr(let_expr.init, main_loop_id), ExprKind::Array(es) | ExprKind::MethodCall(_, _, es, _) | ExprKind::Tup(es) => { never_loop_expr_all(&mut es.iter(), main_loop_id) }, diff --git a/src/tools/clippy/clippy_lints/src/manual_assert.rs b/src/tools/clippy/clippy_lints/src/manual_assert.rs index ed3166086f7..5a2a965716c 100644 --- a/src/tools/clippy/clippy_lints/src/manual_assert.rs +++ b/src/tools/clippy/clippy_lints/src/manual_assert.rs @@ -50,7 +50,7 @@ impl LateLintPass<'_> for ManualAssert { .. } = &expr; if is_expn_of(stmt.span, "panic").is_some(); - if !matches!(cond.kind, ExprKind::Let(_, _, _)); + if !matches!(cond.kind, ExprKind::Let(_)); if let StmtKind::Semi(semi) = stmt.kind; if !cx.tcx.sess.source_map().is_multiline(cond.span); diff --git a/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs b/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs index c7d77d30927..be319ee110d 100644 --- a/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs +++ b/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_hir::{ - intravisit, Body, Expr, ExprKind, FnDecl, HirId, LocalSource, Mutability, Pat, PatKind, Stmt, StmtKind, + intravisit, Body, Expr, ExprKind, FnDecl, HirId, Let, LocalSource, Mutability, Pat, PatKind, Stmt, StmtKind, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; @@ -104,8 +104,8 @@ impl<'tcx> LateLintPass<'tcx> for PatternTypeMismatch { } } } - if let ExprKind::Let(let_pat, ..) = expr.kind { - apply_lint(cx, let_pat, DerefPossible::Possible); + if let ExprKind::Let(Let { pat, .. }) = expr.kind { + apply_lint(cx, pat, DerefPossible::Possible); } } diff --git a/src/tools/clippy/clippy_lints/src/utils/author.rs b/src/tools/clippy/clippy_lints/src/utils/author.rs index f186e1f05a0..c1b811c2174 100644 --- a/src/tools/clippy/clippy_lints/src/utils/author.rs +++ b/src/tools/clippy/clippy_lints/src/utils/author.rs @@ -373,11 +373,18 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> { } match expr.value.kind { - ExprKind::Let(pat, expr, _) => { - bind!(self, pat, expr); - kind!("Let({pat}, {expr}, _)"); - self.pat(pat); - self.expr(expr); + ExprKind::Let(let_expr) => { + bind!(self, let_expr); + kind!("Let({let_expr})"); + self.pat(field!(let_expr.pat)); + // Does what ExprKind::Cast does, only adds a clause for the type + // if it's a path + if let Some(TyKind::Path(ref qpath)) = let_expr.value.ty.as_ref().map(|ty| &ty.kind) { + bind!(self, qpath); + out!("if let TyKind::Path(ref {qpath}) = {let_expr}.ty.kind;"); + self.qpath(qpath); + } + self.expr(field!(let_expr.init)); }, ExprKind::Box(inner) => { bind!(self, inner); diff --git a/src/tools/clippy/clippy_lints/src/utils/inspector.rs b/src/tools/clippy/clippy_lints/src/utils/inspector.rs index 43590cc7862..abf4826a069 100644 --- a/src/tools/clippy/clippy_lints/src/utils/inspector.rs +++ b/src/tools/clippy/clippy_lints/src/utils/inspector.rs @@ -142,9 +142,12 @@ fn print_expr(cx: &LateContext<'_>, expr: &hir::Expr<'_>, indent: usize) { print_expr(cx, arg, indent + 1); } }, - hir::ExprKind::Let(pat, expr, _) => { + hir::ExprKind::Let(hir::Let { pat, init, ty, .. }) => { print_pat(cx, pat, indent + 1); - print_expr(cx, expr, indent + 1); + if let Some(ty) = ty { + println!("{} type annotation: {:?}", ind, ty); + } + print_expr(cx, init, indent + 1); }, hir::ExprKind::MethodCall(path, _, args, _) => { println!("{}MethodCall", ind); diff --git a/src/tools/clippy/clippy_utils/src/higher.rs b/src/tools/clippy/clippy_utils/src/higher.rs index fc32e49420e..c764c35d444 100644 --- a/src/tools/clippy/clippy_utils/src/higher.rs +++ b/src/tools/clippy/clippy_utils/src/higher.rs @@ -101,7 +101,12 @@ impl<'hir> IfLet<'hir> { pub fn hir(cx: &LateContext<'_>, expr: &Expr<'hir>) -> Option<Self> { if let ExprKind::If( Expr { - kind: ExprKind::Let(let_pat, let_expr, _), + kind: + ExprKind::Let(hir::Let { + pat: let_pat, + init: let_expr, + .. + }), .. }, if_then, @@ -368,7 +373,12 @@ impl<'hir> WhileLet<'hir> { kind: ExprKind::If( Expr { - kind: ExprKind::Let(let_pat, let_expr, _), + kind: + ExprKind::Let(hir::Let { + pat: let_pat, + init: let_expr, + .. + }), .. }, if_then, diff --git a/src/tools/clippy/clippy_utils/src/hir_utils.rs b/src/tools/clippy/clippy_utils/src/hir_utils.rs index b8e53e47ed9..ad50759effa 100644 --- a/src/tools/clippy/clippy_utils/src/hir_utils.rs +++ b/src/tools/clippy/clippy_utils/src/hir_utils.rs @@ -7,7 +7,7 @@ use rustc_hir::def::Res; use rustc_hir::HirIdMap; use rustc_hir::{ BinOpKind, Block, BodyId, Expr, ExprField, ExprKind, FnRetTy, GenericArg, GenericArgs, Guard, HirId, - InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatField, PatKind, Path, PathSegment, QPath, Stmt, + InlineAsmOperand, Let, Lifetime, LifetimeName, ParamName, Pat, PatField, PatKind, Path, PathSegment, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding, }; use rustc_lexer::{tokenize, TokenKind}; @@ -234,7 +234,9 @@ impl HirEqInterExpr<'_, '_, '_> { (&ExprKind::If(lc, lt, ref le), &ExprKind::If(rc, rt, ref re)) => { self.eq_expr(lc, rc) && self.eq_expr(&**lt, &**rt) && both(le, re, |l, r| self.eq_expr(l, r)) }, - (&ExprKind::Let(lp, le, _), &ExprKind::Let(rp, re, _)) => self.eq_pat(lp, rp) && self.eq_expr(le, re), + (&ExprKind::Let(l), &ExprKind::Let(r)) => { + self.eq_pat(l.pat, r.pat) && both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) && self.eq_expr(l.init, r.init) + }, (&ExprKind::Lit(ref l), &ExprKind::Lit(ref r)) => l.node == r.node, (&ExprKind::Loop(lb, ref ll, ref lls, _), &ExprKind::Loop(rb, ref rl, ref rls, _)) => { lls == rls && self.eq_block(lb, rb) && both(ll, rl, |l, r| l.ident.name == r.ident.name) @@ -668,8 +670,11 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } } }, - ExprKind::Let(pat, expr, _) => { - self.hash_expr(expr); + ExprKind::Let(Let { pat, init, ty, .. }) => { + self.hash_expr(init); + if let Some(ty) = ty { + self.hash_ty(ty); + } self.hash_pat(pat); }, ExprKind::LlvmInlineAsm(..) | ExprKind::Err => {}, diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index d384c5a069e..7e054a54c3c 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -870,8 +870,8 @@ pub fn capture_local_usage(cx: &LateContext<'tcx>, e: &Expr<'_>) -> CaptureKind capture_expr_ty = e; } }, - ExprKind::Let(pat, ..) => { - let mutability = match pat_capture_kind(cx, pat) { + ExprKind::Let(let_expr) => { + let mutability = match pat_capture_kind(cx, let_expr.pat) { CaptureKind::Value => Mutability::Not, CaptureKind::Ref(m) => m, }; diff --git a/src/tools/clippy/tests/ui/author/if.stdout b/src/tools/clippy/tests/ui/author/if.stdout index 75ff3faf29a..8d92849b366 100644 --- a/src/tools/clippy/tests/ui/author/if.stdout +++ b/src/tools/clippy/tests/ui/author/if.stdout @@ -32,11 +32,11 @@ if_chain! { } if_chain! { if let ExprKind::If(cond, then, Some(else_expr)) = expr.kind; - if let ExprKind::Let(pat, expr1, _) = cond.kind; - if let PatKind::Lit(lit_expr) = pat.kind; + if let ExprKind::Let(let_expr) = cond.kind; + if let PatKind::Lit(lit_expr) = let_expr.pat.kind; if let ExprKind::Lit(ref lit) = lit_expr.kind; if let LitKind::Bool(true) = lit.node; - if let ExprKind::Path(ref qpath) = expr1.kind; + if let ExprKind::Path(ref qpath) = let_expr.init.kind; if match_qpath(qpath, &["a"]); if let ExprKind::Block(block, None) = then.kind; if block.stmts.is_empty(); |