Skip to content

Commit

Permalink
Auto merge of rust-lang#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: rust-lang#87335

Fixes rust-lang#89688, fixes rust-lang#89807, edit: fixes  rust-lang#89960 as well

As explained in rust-lang#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 ece0946 + 17c1ff9 commit af1eea3
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 31 deletions.
16 changes: 8 additions & 8 deletions clippy_lints/src/equatable_if_let.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops/never_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/manual_assert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/pattern_type_mismatch.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
17 changes: 12 additions & 5 deletions clippy_lints/src/utils/author.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions clippy_lints/src/utils/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 12 additions & 2 deletions clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 9 additions & 4 deletions clippy_utils/src/hir_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 => {},
Expand Down
4 changes: 2 additions & 2 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/author/if.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit af1eea3

Please sign in to comment.