HIR typeck: use correct expectations for negation operators

compiler/rustc_hir_typeck/src/expr.rs

@@ -411,17 +411,64 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr: &'tcx hir::Expr<'tcx>,
     ) -> Ty<'tcx> {
         let tcx = self.tcx;
-        let expected_inner = match unop {
-            hir::UnOp::Not | hir::UnOp::Neg => expected,
-            hir::UnOp::Deref => NoExpectation,
+        // Help inference out with negative numeric literals by propagating our type expectation.
+        // They only possible `Not` and `Neg` impls for them have the same input and output types,
+        // and inferring numeric variables early gives us better diagnostics in `check_user_unop`.
+        let expected_inner = if unop == hir::UnOp::Neg
+            && let hir::ExprKind::Lit(lit) = oprnd.kind
+            && matches!(lit.node, ast::LitKind::Int(..) | ast::LitKind::Float(..))
+        {
+            expected
+        } else {
+            NoExpectation
         };
+
+        // TODO: clean this up. the hack needs to see inference state *before* typecking `oprnd` to
+        // avoid accidentally allowing new `match`es :(
+        let opt_hack_expected_ty = match unop {
+            hir::UnOp::Deref => None,
+            hir::UnOp::Not | hir::UnOp::Neg => {
+                match oprnd.kind {
+                    // Blocks will always force coercions if the expectation is `ExpectHasType(..)`.
+                    ExprKind::Block(..) => expected.only_has_type(self),
+                    // `match`es won't force coercions to `()` or to uninferred type variables.
+                    ExprKind::Match(..) => expected
+                        .try_structurally_resolve_and_adjust_for_branches(self)
+                        .only_has_type(self)
+                        .filter(|&ety| ety != tcx.types.unit),
+                    // Some other expression kinds would also force a coercion to `expected`, but
+                    // we're limiting the hack to the most common cases of breakage.
+                    _ => None,
+                }
+            }
+        };
+
         let oprnd_t = self.check_expr_with_expectation(oprnd, expected_inner);
 
         if let Err(guar) = oprnd_t.error_reported() {
             return Ty::new_error(tcx, guar);
         }
 
-        let oprnd_t = self.structurally_resolve_type(expr.span, oprnd_t);
+        // HACK(TODO: lint name): Previously, we always propagated `expected` to the operand for
+        // negation operators. This was wrong, but it helped inference in some cases. To reduce
+        // breakage from not doing that anymore, try using the expected result type for the operand
+        // type if we can't infer the operand type and the operand is a block or `match` that would
+        // previously have used the expected result type as a coercion target.
+        // TODO: clean this up and probably also pull it out into a helper function
+        let oprnd_t = self.resolve_vars_with_obligations(oprnd_t);
+        let oprnd_t = if !oprnd_t.is_ty_var() {
+            oprnd_t
+        } else if let Some(expected_ty) = opt_hack_expected_ty {
+            // TODO: commit_if_ok the sup if we don't want to preserve occurs check errors
+            self.demand_suptype(expr.span, expected_ty, oprnd_t);
+            // TODO: fcw if this resolves
+            self.structurally_resolve_type(expr.span, oprnd_t)
+        } else {
+            // FIXME(#26830): We don't need to error here.
+            // TODO: explain: erroring here keeps the hack from from affecting otherwise-fine code
+            self.type_must_be_known_at_this_point(expr.span, oprnd_t)
+        };
+
         match unop {
             hir::UnOp::Deref => self.lookup_derefing(expr, oprnd, oprnd_t).unwrap_or_else(|| {
                 let mut err =
@@ -433,13 +480,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 Ty::new_error(tcx, err.emit())
             }),
             hir::UnOp::Not => {
-                let result = self.check_user_unop(expr, oprnd_t, unop, expected_inner);
-                // If it's builtin, we can reuse the type, this helps inference.
+                let result = self.check_user_unop(expr, oprnd_t, unop, expected);
+                // If it's builtin, we can reuse the operand type, this helps inference.
                 if oprnd_t.is_integral() || *oprnd_t.kind() == ty::Bool { oprnd_t } else { result }
             }
             hir::UnOp::Neg => {
-                let result = self.check_user_unop(expr, oprnd_t, unop, expected_inner);
-                // If it's builtin, we can reuse the type, this helps inference.
+                let result = self.check_user_unop(expr, oprnd_t, unop, expected);
+                // If it's builtin, we can reuse the operand type, this helps inference.
                 if oprnd_t.is_numeric() { oprnd_t } else { result }
             }
         }

tests/ui/inference/need_type_info/not-diverging-block.rs

@@ -0,0 +1,17 @@
+fn match_not_diverging_block() {
+    // Due to never-to-any coercion, the type of `{ return }` is a fresh type variable. Since we
+    // can't resolve its type immediately, we can't negate it. `!` has a `Not` impl, so if the
+    // pre-fallback type of `{ return }` was `!` instead, this would compile.
+    match !{ return } {}
+    //~^ ERROR type annotations needed
+}
+
+fn if_not_diverging_block() {
+    // Here, `if` uses an expected type of `bool` for its condition, which we previously propagated
+    // to the `!` operator's operand; see <https://github.com/rust-lang/rust/issues/151202>. To
+    // prevent breakage in fixing that bug, we currently still accept this.
+    // TODO: fcw
+    if !{ return } {}
+}
+
+fn main() {}

tests/ui/inference/need_type_info/not-diverging-block.stderr

@@ -0,0 +1,9 @@
+error[E0282]: type annotations needed
+  --> $DIR/not-diverging-block.rs:5:11
+   |
+LL |     match !{ return } {}
+   |           ^^^^^^^^^^^ cannot infer type
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0282`.

tests/ui/typeck/inference-hack-for-unop-operand-fail.rs

@@ -0,0 +1,104 @@
+//! Tests for the inference hack in <https://github.com/rust-lang/rust/pull/151539>. As of that PR,
+//! we no longer propagate type expectations from `!` and `-` operators to their operands. To
+//! minimize breakage, if we can't infer the type of the operand to `!` or `-`, the operand is a
+//! block or `match` expression, and it previously would have used the expected type of the whole
+//! operation as a coercion target, we use that expected type as the type of the operand.
+//! These tests are for cases where type-checking fails, either because the hack didn't apply or it
+//! didn't help. See `inference-hack-for-unop-operand-pass.rs` for succesful tests.
+
+fn infer_me<T>() -> T { panic!() }
+
+// These used to propagate the `i8` expectation to the inner block, through both operators. Since
+// we don't propagate it anymore, the inner negations now have no expectations on them.
+fn not_not_block_inference_failure() {
+    let _: i8 = !!{ infer_me() }; //~ ERROR type annotations needed
+}
+fn neg_neg_block_inference_failure() {
+    let _: i8 = -(-{ infer_me() }); //~ ERROR type annotations needed
+}
+fn not_match_not_block_inference_failure() {
+    let _: i8 = !match infer_me() { x => !{ x } }; //~ ERROR type annotations needed
+}
+fn neg_match_neg_block_inference_failure() {
+    let _: i8 = -match infer_me() { x => -{ x } }; //~ ERROR type annotations needed
+}
+
+// We're not applying the hack for negated blocks and `match`es to negated `if`s or `loop`s, since
+// they seemed much less common in practice.
+fn not_if_inference_failure() {
+    let _: i8 = !if true { infer_me() } else { infer_me() }; //~ ERROR type annotations needed
+}
+fn neg_if_inference_failure() {
+    let _: i8 = -if true { infer_me() } else { infer_me() }; //~ ERROR type annotations needed
+}
+fn not_loop_inference_failure() {
+    let _: i8 = !'l: loop { break 'l infer_me() }; //~ ERROR type annotations needed
+}
+fn neg_loop_inference_failure() {
+    let _: i8 = -'l: loop { break 'l infer_me() }; //~ ERROR type annotations needed
+}
+
+// The following tests are for code that failed to type-check before #151539, to make sure they still
+// fail in the same way now.
+
+// `match` doesn't coerce its arms to its expected type if that type is `()`. `()` doesn't have a
+// `Neg` or `Not` impl anyway, but the error is about needing type annotations.
+fn unit_not_match_inference_failure() {
+    let _: () = !match infer_me() { x => x }; //~ ERROR type annotations needed
+}
+fn unit_neg_match_inference_failure() {
+    let _: () = -match infer_me() { x => x }; //~ ERROR type annotations needed
+}
+
+// Blocks don't special-case `()`, so the error here is about negating `()`.
+fn unit_negated_blocks_inference_success_but_no_impl_exists() {
+    let _: () = !{ infer_me() }; //~ ERROR cannot apply unary operator `!` to type `()`
+    let _: () = -{ infer_me() }; //~ ERROR cannot apply unary operator `-` to type `()`
+}
+
+// `match` also doesn't coerce its arms to its expected type if that type is uninferred at the time
+// we start checking the `match` expression.
+fn initially_uninferred_not_match_inference_failure() {
+    // This `let`'s initializer will be checked with a fresh type variable as its expected type.
+    let _ = 'a: {
+        // That type variable is used as the expected type for the `!` operator expression. Since
+        // it's not inferred at this point, we'll use a fresh type variable for the `match`.
+        !match () {
+            _ => {
+                // Unify the expected type of the `!` operator expression with `i8`.
+                if false { break 'a 0i8 }
+                // The `match`'s type is still a fresh type variable, so we can't negate it.
+                infer_me() //~ ERROR type annotations needed
+            }
+        }
+    };
+}
+fn initially_uninferred_neg_match_inference_failure() {
+    let _ = 'a: {
+        -match () {
+            _ => {
+                if false { break 'a 0i8 }
+                infer_me() //~ ERROR type annotations needed
+            }
+        }
+    };
+}
+
+// TODO: funny test: this was previously an occurs check failure because `!{ y }` was checked with
+// `x`'s type as its expected type, but now we don't apply the hack because we know `y` is a `Box`
+fn we_got_rid_of_this_occurs_check_failure_because_y_is_known_to_be_a_box() {
+    let x;
+    let y = Box::new(x);
+    x = !{ y }; //~ ERROR cannot apply unary operator `!` to type `Box<_>`
+}
+
+// TODO: funny test: we keep this occurs check failure around, but report it differently. we don't
+// know `{ x }`'s type when checking the negation, so the hack fires. do we want this...? do we
+// roll back and prevent the hack from firing here? if we let it through, do we keep the fcw on it?
+fn we_keep_this_occurs_check_failure_around() {
+    let x;
+    let mut y = Box::new(x);
+    y = !{ x }; //~ ERROR overflow assigning `_` to `Box<_>`
+}
+
+fn main() {}

tests/ui/typeck/inference-hack-for-unop-operand-fail.stderr

@@ -0,0 +1,170 @@
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:14:21
+   |
+LL |     let _: i8 = !!{ infer_me() };
+   |                     ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: i8 = !!{ infer_me::</* Type */>() };
+   |                             ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:17:22
+   |
+LL |     let _: i8 = -(-{ infer_me() });
+   |                      ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: i8 = -(-{ infer_me::</* Type */>() });
+   |                              ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:20:24
+   |
+LL |     let _: i8 = !match infer_me() { x => !{ x } };
+   |                        ^^^^^^^^          ------ type must be known at this point
+   |                        |
+   |                        cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: i8 = !match infer_me::</* Type */>() { x => !{ x } };
+   |                                ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:23:24
+   |
+LL |     let _: i8 = -match infer_me() { x => -{ x } };
+   |                        ^^^^^^^^          ------ type must be known at this point
+   |                        |
+   |                        cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: i8 = -match infer_me::</* Type */>() { x => -{ x } };
+   |                                ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:29:28
+   |
+LL |     let _: i8 = !if true { infer_me() } else { infer_me() };
+   |                            ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: i8 = !if true { infer_me::</* Type */>() } else { infer_me() };
+   |                                    ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:32:28
+   |
+LL |     let _: i8 = -if true { infer_me() } else { infer_me() };
+   |                            ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: i8 = -if true { infer_me::</* Type */>() } else { infer_me() };
+   |                                    ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:35:38
+   |
+LL |     let _: i8 = !'l: loop { break 'l infer_me() };
+   |                                      ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: i8 = !'l: loop { break 'l infer_me::</* Type */>() };
+   |                                              ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:38:38
+   |
+LL |     let _: i8 = -'l: loop { break 'l infer_me() };
+   |                                      ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: i8 = -'l: loop { break 'l infer_me::</* Type */>() };
+   |                                              ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:47:24
+   |
+LL |     let _: () = !match infer_me() { x => x };
+   |                        ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: () = !match infer_me::</* Type */>() { x => x };
+   |                                ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:50:24
+   |
+LL |     let _: () = -match infer_me() { x => x };
+   |                        ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |     let _: () = -match infer_me::</* Type */>() { x => x };
+   |                                ++++++++++++++
+
+error[E0600]: cannot apply unary operator `!` to type `()`
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:55:17
+   |
+LL |     let _: () = !{ infer_me() };
+   |                 ^^^^^^^^^^^^^^^ cannot apply unary operator `!`
+
+error[E0600]: cannot apply unary operator `-` to type `()`
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:56:17
+   |
+LL |     let _: () = -{ infer_me() };
+   |                 ^^^^^^^^^^^^^^^ cannot apply unary operator `-`
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:71:17
+   |
+LL |                 infer_me()
+   |                 ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |                 infer_me::</* Type */>()
+   |                         ++++++++++++++
+
+error[E0282]: type annotations needed
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:81:17
+   |
+LL |                 infer_me()
+   |                 ^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `infer_me`
+   |
+help: consider specifying a concrete type for the type parameter `T`
+   |
+LL |                 infer_me::</* Type */>()
+   |                         ++++++++++++++
+
+error[E0600]: cannot apply unary operator `!` to type `Box<_>`
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:92:9
+   |
+LL |     x = !{ y };
+   |         ^^^^^^ cannot apply unary operator `!`
+   |
+note: `Box<_>` does not implement `Not`
+  --> $SRC_DIR/alloc/src/boxed.rs:LL:COL
+  ::: $SRC_DIR/alloc/src/boxed.rs:LL:COL
+   |
+   = note: `Box<_>` is defined in another crate
+
+error[E0275]: overflow assigning `_` to `Box<_>`
+  --> $DIR/inference-hack-for-unop-operand-fail.rs:101:12
+   |
+LL |     y = !{ x };
+   |            ^
+
+error: aborting due to 16 previous errors
+
+Some errors have detailed explanations: E0275, E0282, E0600.
+For more information about an error, try `rustc --explain E0275`.