Auto merge of rust-lang#79328 - c410-f3r:hir-if, r=matthewjasper

Reintroduce hir::ExprKind::If

Basically copied and paste rust-lang#59288/rust-lang/rust-clippy#4080 with some modifications.

The vast majority of tests were fixed and now there are only a few remaining. Since I am still unable to figure out the missing pieces, any help with the following list is welcome.

- [ ] **Unnecessary `typeck` exception**: [Cheated on this one to make CI green.](https://github.com/rust-lang/rust/pull/79328/files#diff-3faee9ba23fc54a12b7c43364ba81f8c5660045c7e1d7989a02a0cee1c5b2051)
- [x] **Incorrect span**: [Span should reference `then` and `else` separately.](https://github.com/rust-lang/rust/pull/79328/files#diff-cf2c46e82222ee4b1037a68fff8a1af3c4f1de7a6b3fd798aacbf3c0475abe3d)
- [x] **New note regarding `assert!`**: [Modified but not "wrong". Maybe can be a good thing?](https://github.com/rust-lang/rust/pull/79328/files#diff-9e0d7c89ed0224e2b62060c957177c27db43c30dfe3c2974cb6b5091cda9cfb5)
- [x] **Inverted report location**: [Modified but not "wrong". Locations were inverted.](https://github.com/rust-lang/rust/pull/79328/files#diff-f637ce7c1f68d523a165aa9651765df05e36c4d7d279194b1a6b28b48a323691)
- [x] **`src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs` has weird errors**: [Not sure why this is happening.](https://github.com/rust-lang/rust/pull/79328/files#diff-c823c09660f5b112f95e97e8ff71f1797b6c7f37dbb3d16f8e98bbaea8072e95)
- [x] **Missing diagnostic**: [???](https://github.com/rust-lang/rust/pull/79328/files#diff-6b8ab09360d725ba4513933827f9796b42ff9522b0690f80b76de067143af2fc)

Author: bors

compiler/rustc_ast_lowering/src/expr.rs

@@ -87,9 +87,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 ExprKind::Let(ref pat, ref scrutinee) => {
                     self.lower_expr_let(e.span, pat, scrutinee)
                 }
-                ExprKind::If(ref cond, ref then, ref else_opt) => {
-                    self.lower_expr_if(e.span, cond, then, else_opt.as_deref())
-                }
+                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())
+                    }
+                    _ => 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)
@@ -337,10 +340,30 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
     fn lower_expr_if(
         &mut self,
-        span: Span,
         cond: &Expr,
         then: &Block,
         else_opt: Option<&Expr>,
+    ) -> hir::ExprKind<'hir> {
+        macro_rules! make_if {
+            ($opt:expr) => {{
+                let then_expr = self.lower_block_expr(then);
+                hir::ExprKind::If(self.lower_expr(cond), self.arena.alloc(then_expr), $opt)
+            }};
+        }
+        if let Some(rslt) = else_opt {
+            make_if!(Some(self.lower_expr(rslt)))
+        } else {
+            make_if!(None)
+        }
+    }
+
+    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.
 
@@ -353,30 +376,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let else_arm = self.arm(else_pat, else_expr);
 
         // Handle then + scrutinee:
-        let (then_pat, scrutinee, desugar) = match cond.kind {
-            // `<pat> => <then>`:
-            ExprKind::Let(ref pat, ref scrutinee) => {
-                let scrutinee = self.lower_expr(scrutinee);
-                let pat = self.lower_pat(pat);
-                (pat, scrutinee, hir::MatchSource::IfLetDesugar { contains_else_clause })
-            }
-            // `true => <then>`:
-            _ => {
-                // Lower condition:
-                let cond = self.lower_expr(cond);
-                let span_block =
-                    self.mark_span_with_reason(DesugaringKind::CondTemporary, cond.span, None);
-                // Wrap in a construct equivalent to `{ let _t = $cond; _t }`
-                // to preserve drop semantics since `if cond { ... }` does not
-                // let temporaries live outside of `cond`.
-                let cond = self.expr_drop_temps(span_block, cond, ThinVec::new());
-                let pat = self.pat_bool(span, true);
-                (pat, cond, hir::MatchSource::IfDesugar { contains_else_clause })
-            }
-        };
+        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)
     }
 

compiler/rustc_hir/src/hir.rs

@@ -1431,6 +1431,7 @@ impl Expr<'_> {
             ExprKind::Lit(_) => ExprPrecedence::Lit,
             ExprKind::Type(..) | ExprKind::Cast(..) => ExprPrecedence::Cast,
             ExprKind::DropTemps(ref expr, ..) => expr.precedence(),
+            ExprKind::If(..) => ExprPrecedence::If,
             ExprKind::Loop(..) => ExprPrecedence::Loop,
             ExprKind::Match(..) => ExprPrecedence::Match,
             ExprKind::Closure(..) => ExprPrecedence::Closure,
@@ -1492,6 +1493,7 @@ impl Expr<'_> {
             | ExprKind::MethodCall(..)
             | ExprKind::Struct(..)
             | ExprKind::Tup(..)
+            | ExprKind::If(..)
             | ExprKind::Match(..)
             | ExprKind::Closure(..)
             | ExprKind::Block(..)
@@ -1608,6 +1610,10 @@ pub enum ExprKind<'hir> {
     /// This construct only exists to tweak the drop order in HIR lowering.
     /// An example of that is the desugaring of `for` loops.
     DropTemps(&'hir Expr<'hir>),
+    /// An `if` block, with an optional else block.
+    ///
+    /// I.e., `if <expr> { <expr> } else { <expr> }`.
+    If(&'hir Expr<'hir>, &'hir Expr<'hir>, Option<&'hir Expr<'hir>>),
     /// A conditionless loop (can be exited with `break`, `continue`, or `return`).
     ///
     /// I.e., `'label: loop { <block> }`.
@@ -1761,8 +1767,6 @@ pub enum LocalSource {
 pub enum MatchSource {
     /// A `match _ { .. }`.
     Normal,
-    /// An `if _ { .. }` (optionally with `else { .. }`).
-    IfDesugar { contains_else_clause: bool },
     /// An `if let _ = _ { .. }` (optionally with `else { .. }`).
     IfLetDesugar { contains_else_clause: bool },
     /// An `if let _ = _ => { .. }` match guard.
@@ -1785,7 +1789,7 @@ impl MatchSource {
         use MatchSource::*;
         match self {
             Normal => "match",
-            IfDesugar { .. } | IfLetDesugar { .. } | IfLetGuardDesugar => "if",
+            IfLetDesugar { .. } | IfLetGuardDesugar => "if",
             WhileDesugar | WhileLetDesugar => "while",
             ForLoopDesugar => "for",
             TryDesugar => "?",

compiler/rustc_hir/src/intravisit.rs

@@ -1146,6 +1146,11 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
         ExprKind::DropTemps(ref subexpression) => {
             visitor.visit_expr(subexpression);
         }
+        ExprKind::If(ref cond, ref then, ref else_opt) => {
+            visitor.visit_expr(cond);
+            visitor.visit_expr(then);
+            walk_list!(visitor, visit_expr, else_opt);
+        }
         ExprKind::Loop(ref block, ref opt_label, _) => {
             walk_list!(visitor, visit_label, opt_label);
             visitor.visit_block(block);

compiler/rustc_hir_pretty/src/lib.rs

@@ -1080,6 +1080,50 @@ impl<'a> State<'a> {
         self.ann.post(self, AnnNode::Block(blk))
     }
 
+    fn print_else(&mut self, els: Option<&hir::Expr<'_>>) {
+        match els {
+            Some(_else) => {
+                match _else.kind {
+                    // "another else-if"
+                    hir::ExprKind::If(ref i, ref then, ref e) => {
+                        self.cbox(INDENT_UNIT - 1);
+                        self.ibox(0);
+                        self.s.word(" else if ");
+                        self.print_expr_as_cond(&i);
+                        self.s.space();
+                        self.print_expr(&then);
+                        self.print_else(e.as_ref().map(|e| &**e))
+                    }
+                    // "final else"
+                    hir::ExprKind::Block(ref b, _) => {
+                        self.cbox(INDENT_UNIT - 1);
+                        self.ibox(0);
+                        self.s.word(" else ");
+                        self.print_block(&b)
+                    }
+                    // BLEAH, constraints would be great here
+                    _ => {
+                        panic!("print_if saw if with weird alternative");
+                    }
+                }
+            }
+            _ => {}
+        }
+    }
+
+    pub fn print_if(
+        &mut self,
+        test: &hir::Expr<'_>,
+        blk: &hir::Expr<'_>,
+        elseopt: Option<&hir::Expr<'_>>,
+    ) {
+        self.head("if");
+        self.print_expr_as_cond(test);
+        self.s.space();
+        self.print_expr(blk);
+        self.print_else(elseopt)
+    }
+
     pub fn print_anon_const(&mut self, constant: &hir::AnonConst) {
         self.ann.nested(self, Nested::Body(constant.body))
     }
@@ -1349,6 +1393,9 @@ impl<'a> State<'a> {
                 // Print `}`:
                 self.bclose_maybe_open(expr.span, true);
             }
+            hir::ExprKind::If(ref test, ref blk, ref elseopt) => {
+                self.print_if(&test, &blk, elseopt.as_ref().map(|e| &**e));
+            }
             hir::ExprKind::Loop(ref blk, opt_label, _) => {
                 if let Some(label) = opt_label {
                     self.print_ident(label.ident);
@@ -2429,7 +2476,13 @@ impl<'a> State<'a> {
 //
 // Duplicated from `parse::classify`, but adapted for the HIR.
 fn expr_requires_semi_to_be_stmt(e: &hir::Expr<'_>) -> bool {
-    !matches!(e.kind, hir::ExprKind::Match(..) | hir::ExprKind::Block(..) | hir::ExprKind::Loop(..))
+    !matches!(
+        e.kind,
+        hir::ExprKind::If(..)
+            | hir::ExprKind::Match(..)
+            | hir::ExprKind::Block(..)
+            | hir::ExprKind::Loop(..)
+    )
 }
 
 /// This statement requires a semicolon after it.

compiler/rustc_middle/src/hir/map/mod.rs

@@ -658,20 +658,22 @@ impl<'hir> Map<'hir> {
         CRATE_HIR_ID
     }
 
-    /// When on a match arm tail expression or on a match arm, give back the enclosing `match`
-    /// expression.
+    /// When on an if expression, a match arm tail expression or a match arm, give back
+    /// the enclosing `if` or `match` expression.
     ///
-    /// Used by error reporting when there's a type error in a match arm caused by the `match`
+    /// Used by error reporting when there's a type error in an if or match arm caused by the
     /// expression needing to be unit.
-    pub fn get_match_if_cause(&self, hir_id: HirId) -> Option<&'hir Expr<'hir>> {
+    pub fn get_if_cause(&self, hir_id: HirId) -> Option<&'hir Expr<'hir>> {
         for (_, node) in self.parent_iter(hir_id) {
             match node {
                 Node::Item(_)
                 | Node::ForeignItem(_)
                 | Node::TraitItem(_)
                 | Node::ImplItem(_)
                 | Node::Stmt(Stmt { kind: StmtKind::Local(_), .. }) => break,
-                Node::Expr(expr @ Expr { kind: ExprKind::Match(..), .. }) => return Some(expr),
+                Node::Expr(expr @ Expr { kind: ExprKind::If(..) | ExprKind::Match(..), .. }) => {
+                    return Some(expr);
+                }
                 _ => {}
             }
         }

compiler/rustc_mir_build/src/build/expr/as_place.rs

@@ -531,6 +531,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             | ExprKind::Borrow { .. }
             | ExprKind::AddressOf { .. }
             | ExprKind::Match { .. }
+            | ExprKind::If { .. }
             | ExprKind::Loop { .. }
             | ExprKind::Block { .. }
             | ExprKind::Assign { .. }

compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

@@ -251,6 +251,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             | ExprKind::StaticRef { .. }
             | ExprKind::Block { .. }
             | ExprKind::Match { .. }
+            | ExprKind::If { .. }
             | ExprKind::NeverToAny { .. }
             | ExprKind::Use { .. }
             | ExprKind::Borrow { .. }

compiler/rustc_mir_build/src/build/expr/category.rs

@@ -45,6 +45,7 @@ impl Category {
 
             ExprKind::LogicalOp { .. }
             | ExprKind::Match { .. }
+            | ExprKind::If { .. }
             | ExprKind::NeverToAny { .. }
             | ExprKind::Use { .. }
             | ExprKind::Adt { .. }

compiler/rustc_mir_build/src/build/expr/into.rs

@@ -69,6 +69,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             ExprKind::Match { scrutinee, arms } => {
                 this.match_expr(destination, scope, expr_span, block, scrutinee, arms)
             }
+            ExprKind::If { cond, then, else_opt } => {
+                let place = unpack!(block = this.as_temp(block, Some(this.local_scope()), cond, Mutability::Mut));
+                let operand = Operand::Move(Place::from(place));
+
+                let mut then_block = this.cfg.start_new_block();
+                let mut else_block = this.cfg.start_new_block();
+                let term = TerminatorKind::if_(this.hir.tcx(), operand, then_block, else_block);
+                this.cfg.terminate(block, source_info, term);
+
+                unpack!(then_block = this.into(destination, scope, then_block, then));
+                else_block = if let Some(else_opt) = else_opt {
+                    unpack!(this.into(destination, None, else_block, else_opt))
+                } else {
+                    // Body of the `if` expression without an `else` clause must return `()`, thus
+                    // we implicitly generate a `else {}` if it is not specified.
+                    let correct_si = this.source_info(expr_span.shrink_to_hi());
+                    this.cfg.push_assign_unit(else_block, correct_si, destination, this.hir.tcx());
+                    else_block
+                };
+
+                let join_block = this.cfg.start_new_block();
+                this.cfg.terminate(
+                    then_block,
+                    source_info,
+                    TerminatorKind::Goto { target: join_block },
+                );
+                this.cfg.terminate(
+                    else_block,
+                    source_info,
+                    TerminatorKind::Goto { target: join_block },
+                );
+
+                join_block.unit()
+            },
             ExprKind::NeverToAny { source } => {
                 let source = this.hir.mirror(source);
                 let is_call = matches!(source.kind, ExprKind::Call { .. } | ExprKind::InlineAsm { .. });

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -537,6 +537,11 @@ fn make_mirror_unadjusted<'a, 'tcx>(
             },
             Err(err) => bug!("invalid loop id for continue: {}", err),
         },
+        hir::ExprKind::If(cond, then, else_opt) => ExprKind::If {
+            cond: cond.to_ref(),
+            then: then.to_ref(),
+            else_opt: else_opt.map(|el| el.to_ref()),
+        },
         hir::ExprKind::Match(ref discr, ref arms, _) => ExprKind::Match {
             scrutinee: discr.to_ref(),
             arms: arms.iter().map(|a| convert_arm(cx, a)).collect(),