From a38e987427c8c39ca066e2cf4ecdbcd609cc2e76 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Fri, 27 Aug 2021 12:01:21 -0700 Subject: [PATCH] Revert "Auto merge of #80357 - c410-f3r:new-hir-let, r=matthewjasper" This reverts commit 2a6fb9a4c0e5ca7a81999065943b211c226fe9d8, reversing changes made to 2bd17c1d43bba43412cc2f051323a279d6751e43. --- compiler/rustc_ast/src/ast.rs | 4 +- compiler/rustc_ast/src/mut_visit.rs | 2 +- compiler/rustc_ast/src/visit.rs | 4 +- compiler/rustc_ast_lowering/src/expr.rs | 260 ++++++--- compiler/rustc_ast_lowering/src/lib.rs | 10 + .../rustc_ast_passes/src/ast_validation.rs | 69 +-- compiler/rustc_ast_pretty/src/pprust/state.rs | 27 +- compiler/rustc_expand/src/base.rs | 1 - compiler/rustc_hir/src/hir.rs | 26 +- compiler/rustc_hir/src/intravisit.rs | 4 - compiler/rustc_hir_pretty/src/lib.rs | 124 ++--- .../src/infer/error_reporting/mod.rs | 18 + compiler/rustc_lint/src/unused.rs | 12 +- compiler/rustc_middle/src/thir.rs | 4 - .../src/build/expr/as_place.rs | 1 - .../src/build/expr/as_rvalue.rs | 1 - .../src/build/expr/category.rs | 1 - .../rustc_mir_build/src/build/expr/into.rs | 66 +-- .../rustc_mir_build/src/build/matches/mod.rs | 126 ++--- .../rustc_mir_build/src/check_unsafety.rs | 9 - compiler/rustc_mir_build/src/thir/cx/expr.rs | 3 - .../src/thir/pattern/check_match.rs | 192 +++---- compiler/rustc_mir_build/src/thir/visit.rs | 3 - compiler/rustc_parse/src/parser/expr.rs | 2 +- compiler/rustc_passes/src/check_const.rs | 9 +- compiler/rustc_passes/src/liveness.rs | 17 +- compiler/rustc_passes/src/naked_functions.rs | 1 - compiler/rustc_passes/src/region.rs | 24 +- compiler/rustc_resolve/src/late.rs | 2 +- compiler/rustc_typeck/src/check/_match.rs | 310 +++++++---- compiler/rustc_typeck/src/check/expr.rs | 44 +- compiler/rustc_typeck/src/expr_use_visitor.rs | 28 +- .../rustc_typeck/src/mem_categorization.rs | 1 - src/test/incremental/hashes/if_expressions.rs | 2 +- .../incremental/hashes/while_let_loops.rs | 6 +- src/test/incremental/hashes/while_loops.rs | 4 +- .../discriminant.main.ConstProp.64bit.diff | 14 +- ...float_to_exponential_common.ConstProp.diff | 54 +- .../issue_41888.main.ElaborateDrops.after.mir | 74 ++- ...e_75439.foo.MatchBranchSimplification.diff | 28 +- ..._locals_fixedpoint.foo.SimplifyLocals.diff | 24 +- ...reachable.main.UnreachablePropagation.diff | 17 +- ...hable_asm.main.UnreachablePropagation.diff | 17 +- ...ble_asm_2.main.UnreachablePropagation.diff | 21 +- ...diverging.main.UnreachablePropagation.diff | 31 +- ...oops.change_loop_body.ConstProp.32bit.diff | 6 +- ...oops.change_loop_body.ConstProp.64bit.diff | 16 +- ...le_storage.while_loop.PreCodegen.after.mir | 18 +- src/test/ui-fulldeps/pprust-expr-roundtrip.rs | 2 +- .../migrations/issue-78720.stderr | 9 +- .../const_eval_limit_reached.stderr | 24 +- src/test/ui/expr/if/if-let.rs | 2 +- src/test/ui/expr/if/if-let.stderr | 46 +- src/test/ui/expr/if/if-ret.rs | 2 +- src/test/ui/expr/if/if-ret.stderr | 4 +- src/test/ui/issues/issue-14091.stderr | 2 + src/test/ui/match/issue-82392.stdout | 11 +- src/test/ui/pattern/issue-82290.rs | 4 +- src/test/ui/pattern/issue-82290.stderr | 22 +- .../deny-irrefutable-let-patterns.stderr | 12 +- src/test/ui/reachable/expr_if.rs | 2 +- src/test/ui/reachable/expr_if.stderr | 4 +- src/test/ui/reachable/expr_while.rs | 4 +- src/test/ui/reachable/expr_while.stderr | 8 +- .../ui/rfc-2294-if-let-guard/feature-gate.rs | 16 + .../rfc-2294-if-let-guard/feature-gate.stderr | 164 +++++- .../disallowed-positions.rs | 1 + .../disallowed-positions.stderr | 189 +++---- .../ui/rfc-2497-if-let-chains/feature-gate.rs | 32 ++ .../feature-gate.stderr | 334 ++++++++++-- .../protect-precedences.rs | 3 +- .../protect-precedences.stderr | 4 +- src/test/ui/union/union-unsafe.mir.stderr | 4 +- src/test/ui/union/union-unsafe.thir.stderr | 4 +- src/test/ui/while-let.stderr | 17 +- .../src/assertions_on_constants.rs | 5 +- .../src/blocks_in_if_conditions.rs | 3 +- .../clippy_lints/src/collapsible_match.rs | 56 ++ src/tools/clippy/clippy_lints/src/copies.rs | 17 +- .../clippy/clippy_lints/src/dereference.rs | 1 - src/tools/clippy/clippy_lints/src/entry.rs | 8 +- .../clippy/clippy_lints/src/eta_reduction.rs | 2 +- .../src/floating_point_arithmetic.rs | 7 +- .../clippy/clippy_lints/src/if_let_mutex.rs | 27 +- .../clippy_lints/src/if_let_some_result.rs | 15 +- .../src/if_then_some_else_none.rs | 4 +- .../src/implicit_saturating_sub.rs | 3 +- .../clippy_lints/src/indexing_slicing.rs | 2 +- .../clippy/clippy_lints/src/infinite_iter.rs | 2 +- .../clippy/clippy_lints/src/let_if_seq.rs | 4 +- .../clippy_lints/src/loops/manual_flatten.rs | 13 +- .../clippy_lints/src/loops/manual_memcpy.rs | 2 +- .../clippy/clippy_lints/src/loops/mod.rs | 6 +- .../clippy_lints/src/loops/mut_range_bound.rs | 2 +- .../src/loops/needless_range_loop.rs | 2 +- .../clippy_lints/src/loops/never_loop.rs | 9 +- .../clippy_lints/src/loops/while_let_loop.rs | 98 ++-- .../src/loops/while_let_on_iterator.rs | 25 +- .../clippy/clippy_lints/src/manual_map.rs | 301 +++++------ .../clippy/clippy_lints/src/manual_strip.rs | 4 +- src/tools/clippy/clippy_lints/src/matches.rs | 265 ++++------ .../src/methods/iter_next_slice.rs | 4 +- src/tools/clippy/clippy_lints/src/mut_mut.rs | 2 +- .../clippy/clippy_lints/src/needless_bool.rs | 14 +- .../clippy_lints/src/non_expressive_names.rs | 3 +- .../clippy_lints/src/option_if_let_else.rs | 43 +- .../clippy_lints/src/pattern_type_mismatch.rs | 33 +- .../clippy/clippy_lints/src/question_mark.rs | 27 +- src/tools/clippy/clippy_lints/src/ranges.rs | 14 +- .../clippy_lints/src/redundant_clone.rs | 2 +- src/tools/clippy/clippy_lints/src/returns.rs | 8 + .../src/suspicious_operation_groupings.rs | 2 +- .../clippy_lints/src/unnested_or_patterns.rs | 2 +- src/tools/clippy/clippy_lints/src/unwrap.rs | 7 +- .../clippy/clippy_lints/src/utils/author.rs | 9 - .../clippy_lints/src/utils/inspector.rs | 33 +- src/tools/clippy/clippy_lints/src/vec.rs | 8 +- .../clippy/clippy_utils/src/ast_utils.rs | 2 +- .../clippy/clippy_utils/src/eager_or_lazy.rs | 1 - src/tools/clippy/clippy_utils/src/higher.rs | 495 ++++++------------ .../clippy/clippy_utils/src/hir_utils.rs | 7 - src/tools/clippy/clippy_utils/src/lib.rs | 19 +- src/tools/clippy/clippy_utils/src/sugg.rs | 3 +- src/tools/clippy/tests/ui/author/if.stdout | 3 +- src/tools/clippy/tests/ui/crashes/ice-7410.rs | 1 - .../tests/ui/crashes/issues_loop_mut_cond.rs | 1 - src/tools/clippy/tests/ui/infinite_loop.rs | 2 - .../clippy/tests/ui/infinite_loop.stderr | 22 +- .../clippy/tests/ui/match_overlapping_arm.rs | 1 - .../tests/ui/match_overlapping_arm.stderr | 20 +- src/tools/rustfmt/src/expr.rs | 2 +- 131 files changed, 2250 insertions(+), 2086 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 2c2d30d872e20..f3bac07328ac9 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1302,9 +1302,7 @@ pub enum ExprKind { Type(P, P), /// A `let pat = expr` expression that is only semantically allowed in the condition /// of `if` / `while` expressions. (e.g., `if let 0 = x { .. }`). - /// - /// `Span` represents the whole `let pat = expr` statement. - Let(P, P, Span), + Let(P, P), /// An `if` block, with an optional `else` block. /// /// `if expr { block } else { expr }` diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index fb4db6005aca5..c62a23cc7050b 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1234,7 +1234,7 @@ pub fn noop_visit_expr( vis.visit_ty(ty); } ExprKind::AddrOf(_, _, ohs) => vis.visit_expr(ohs), - ExprKind::Let(pat, scrutinee, _) => { + ExprKind::Let(pat, scrutinee) => { vis.visit_pat(pat); vis.visit_expr(scrutinee); } diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index dd8927496e019..112312ea87551 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -776,9 +776,9 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) { visitor.visit_expr(subexpression); visitor.visit_ty(typ) } - ExprKind::Let(ref pat, ref expr, _) => { + ExprKind::Let(ref pat, ref scrutinee) => { visitor.visit_pat(pat); - visitor.visit_expr(expr); + visitor.visit_expr(scrutinee); } ExprKind::If(ref head_expression, ref if_block, ref optional_else) => { visitor.visit_expr(head_expression); diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index bf7589e84adc4..bacf5662bc005 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -86,12 +86,32 @@ 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), span) - } - ExprKind::If(ref cond, ref then, ref else_opt) => { - self.lower_expr_if(cond, then, else_opt.as_deref()) + ExprKind::Let(ref pat, ref scrutinee) => { + self.lower_expr_let(e.span, pat, scrutinee) } + ExprKind::If(ref cond, ref then, ref else_opt) => match cond.kind { + ExprKind::Let(ref pat, ref scrutinee) => { + self.lower_expr_if_let(e.span, pat, scrutinee, then, else_opt.as_deref()) + } + ExprKind::Paren(ref paren) => match paren.peel_parens().kind { + ExprKind::Let(ref pat, ref scrutinee) => { + // A user has written `if (let Some(x) = foo) {`, we want to avoid + // confusing them with mentions of nightly features. + // If this logic is changed, you will also likely need to touch + // `unused::UnusedParens::check_expr`. + self.if_let_expr_with_parens(cond, &paren.peel_parens()); + self.lower_expr_if_let( + e.span, + pat, + scrutinee, + then, + else_opt.as_deref(), + ) + } + _ => self.lower_expr_if(cond, then, else_opt.as_deref()), + }, + _ => self.lower_expr_if(cond, then, else_opt.as_deref()), + }, ExprKind::While(ref cond, ref body, opt_label) => self .with_loop_scope(e.id, |this| { this.lower_expr_while_in_loop_scope(e.span, cond, body, opt_label) @@ -348,51 +368,115 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Call(f, self.lower_exprs(&real_args)) } + fn if_let_expr_with_parens(&mut self, cond: &Expr, paren: &Expr) { + let start = cond.span.until(paren.span); + let end = paren.span.shrink_to_hi().until(cond.span.shrink_to_hi()); + self.sess + .struct_span_err( + vec![start, end], + "invalid parentheses around `let` expression in `if let`", + ) + .multipart_suggestion( + "`if let` needs to be written without parentheses", + vec![(start, String::new()), (end, String::new())], + rustc_errors::Applicability::MachineApplicable, + ) + .emit(); + // Ideally, we'd remove the feature gating of a `let` expression since we are already + // complaining about it here, but `feature_gate::check_crate` has already run by now: + // self.sess.parse_sess.gated_spans.ungate_last(sym::let_chains, paren.span); + } + + /// Emit an error and lower `ast::ExprKind::Let(pat, scrutinee)` into: + /// ```rust + /// match scrutinee { pats => true, _ => false } + /// ``` + fn lower_expr_let(&mut self, span: Span, pat: &Pat, scrutinee: &Expr) -> hir::ExprKind<'hir> { + // If we got here, the `let` expression is not allowed. + + if self.sess.opts.unstable_features.is_nightly_build() { + self.sess + .struct_span_err(span, "`let` expressions are not supported here") + .note( + "only supported directly without parentheses in conditions of `if`- and \ + `while`-expressions, as well as in `let` chains within parentheses", + ) + .emit(); + } else { + self.sess + .struct_span_err(span, "expected expression, found statement (`let`)") + .note("variable declaration using `let` is a statement") + .emit(); + } + + // For better recovery, we emit: + // ``` + // match scrutinee { pat => true, _ => false } + // ``` + // While this doesn't fully match the user's intent, it has key advantages: + // 1. We can avoid using `abort_if_errors`. + // 2. We can typeck both `pat` and `scrutinee`. + // 3. `pat` is allowed to be refutable. + // 4. The return type of the block is `bool` which seems like what the user wanted. + let scrutinee = self.lower_expr(scrutinee); + let then_arm = { + let pat = self.lower_pat(pat); + let expr = self.expr_bool(span, true); + self.arm(pat, expr) + }; + let else_arm = { + let pat = self.pat_wild(span); + let expr = self.expr_bool(span, false); + self.arm(pat, expr) + }; + hir::ExprKind::Match( + scrutinee, + arena_vec![self; then_arm, else_arm], + hir::MatchSource::Normal, + ) + } + fn lower_expr_if( &mut self, cond: &Expr, then: &Block, else_opt: Option<&Expr>, ) -> hir::ExprKind<'hir> { - let lowered_cond = self.lower_expr(cond); - let new_cond = self.manage_let_cond(lowered_cond); - let then_expr = self.lower_block_expr(then); - if let Some(rslt) = else_opt { - hir::ExprKind::If(new_cond, self.arena.alloc(then_expr), Some(self.lower_expr(rslt))) - } else { - hir::ExprKind::If(new_cond, self.arena.alloc(then_expr), None) - } + let cond = self.lower_expr(cond); + let then = self.arena.alloc(self.lower_block_expr(then)); + let els = else_opt.map(|els| self.lower_expr(els)); + hir::ExprKind::If(cond, then, els) } - // If `cond` kind is `let`, returns `let`. Otherwise, wraps and returns `cond` - // in a temporary block. - fn manage_let_cond(&mut self, cond: &'hir hir::Expr<'hir>) -> &'hir hir::Expr<'hir> { - match cond.kind { - hir::ExprKind::Let(..) => cond, - _ => { - let span_block = - self.mark_span_with_reason(DesugaringKind::CondTemporary, cond.span, None); - self.expr_drop_temps(span_block, cond, AttrVec::new()) - } - } + fn lower_expr_if_let( + &mut self, + span: Span, + pat: &Pat, + scrutinee: &Expr, + then: &Block, + else_opt: Option<&Expr>, + ) -> hir::ExprKind<'hir> { + // FIXME(#53667): handle lowering of && and parens. + + // `_ => else_block` where `else_block` is `{}` if there's `None`: + let else_pat = self.pat_wild(span); + let (else_expr, contains_else_clause) = match else_opt { + None => (self.expr_block_empty(span.shrink_to_hi()), false), + Some(els) => (self.lower_expr(els), true), + }; + let else_arm = self.arm(else_pat, else_expr); + + // Handle then + scrutinee: + let scrutinee = self.lower_expr(scrutinee); + let then_pat = self.lower_pat(pat); + + let then_expr = self.lower_block_expr(then); + let then_arm = self.arm(then_pat, self.arena.alloc(then_expr)); + + let desugar = hir::MatchSource::IfLetDesugar { contains_else_clause }; + hir::ExprKind::Match(scrutinee, arena_vec![self; then_arm, else_arm], desugar) } - // We desugar: `'label: while $cond $body` into: - // - // ``` - // 'label: loop { - // if { let _t = $cond; _t } { - // $body - // } - // else { - // break; - // } - // } - // ``` - // - // Wrap in a construct equivalent to `{ let _t = $cond; _t }` - // to preserve drop semantics since `while $cond { ... }` does not - // let temporaries live outside of `cond`. fn lower_expr_while_in_loop_scope( &mut self, span: Span, @@ -400,17 +484,72 @@ impl<'hir> LoweringContext<'_, 'hir> { body: &Block, opt_label: Option