From cab607edcfe556343aa4fcb3de6229a790fc5e72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 31 Aug 2019 18:34:50 -0700 Subject: [PATCH 1/3] Emit a single error on if expr with expectation and no else clause --- src/librustc_typeck/check/_match.rs | 41 ++++++--- src/test/ui/if/if-without-else-as-fn-expr.rs | 6 -- .../ui/if/if-without-else-as-fn-expr.stderr | 83 ++----------------- src/test/ui/issues/issue-50577.rs | 1 - src/test/ui/issues/issue-50577.stderr | 15 +--- 5 files changed, 38 insertions(+), 108 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index efc37cc04b212..3983a3b820391 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -112,23 +112,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } self.diverges.set(pats_diverge); - let arm_ty = self.check_expr_with_expectation(&arm.body, expected); - all_arms_diverge &= self.diverges.get(); - - let span = expr.span; - - if source_if { + let arm_ty = if source_if { let then_expr = &arms[0].body; match (i, if_no_else) { - (0, _) => coercion.coerce(self, &self.misc(span), &arm.body, arm_ty), - (_, true) => self.if_fallback_coercion(span, then_expr, &mut coercion), + (0, _) => { + let arm_ty = self.check_expr_with_expectation(&arm.body, expected); + all_arms_diverge &= self.diverges.get(); + coercion.coerce(self, &self.misc(expr.span), &arm.body, arm_ty); + arm_ty + } + (_, true) => { + if self.if_fallback_coercion(expr.span, then_expr, &mut coercion) { + tcx.types.err + } else { + let arm_ty = self.check_expr_with_expectation(&arm.body, expected); + all_arms_diverge &= self.diverges.get(); + arm_ty + } + } (_, _) => { + let arm_ty = self.check_expr_with_expectation(&arm.body, expected); + all_arms_diverge &= self.diverges.get(); let then_ty = prior_arm_ty.unwrap(); - let cause = self.if_cause(span, then_expr, &arm.body, then_ty, arm_ty); + let cause = self.if_cause(expr.span, then_expr, &arm.body, then_ty, arm_ty); coercion.coerce(self, &cause, &arm.body, arm_ty); + arm_ty } } } else { + let arm_ty = self.check_expr_with_expectation(&arm.body, expected); + all_arms_diverge &= self.diverges.get(); let arm_span = if let hir::ExprKind::Block(blk, _) = &arm.body.node { // Point at the block expr instead of the entire block blk.expr.as_ref().map(|e| e.span).unwrap_or(arm.body.span) @@ -139,7 +152,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // The reason for the first arm to fail is not that the match arms diverge, // but rather that there's a prior obligation that doesn't hold. 0 => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)), - _ => (span, ObligationCauseCode::MatchExpressionArm { + _ => (expr.span, ObligationCauseCode::MatchExpressionArm { arm_span, source: match_src, prior_arms: other_arms.clone(), @@ -153,7 +166,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if other_arms.len() > 5 { other_arms.remove(0); } - } + arm_ty + }; prior_arm_ty = Some(arm_ty); } @@ -185,11 +199,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { span: Span, then_expr: &'tcx hir::Expr, coercion: &mut CoerceMany<'tcx, '_, rustc::hir::Arm>, - ) { + ) -> bool { // If this `if` expr is the parent's function return expr, // the cause of the type coercion is the return type, point at it. (#25228) let ret_reason = self.maybe_get_coercion_reason(then_expr.hir_id, span); let cause = self.cause(span, ObligationCauseCode::IfExpressionWithNoElse); + let mut error = false; coercion.coerce_forced_unit(self, &cause, &mut |err| { if let Some((span, msg)) = &ret_reason { err.span_label(*span, msg.as_str()); @@ -200,7 +215,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } err.note("`if` expressions without `else` evaluate to `()`"); err.help("consider adding an `else` block that evaluates to the expected type"); + error = true; }, ret_reason.is_none()); + error } fn maybe_get_coercion_reason(&self, hir_id: hir::HirId, span: Span) -> Option<(Span, String)> { diff --git a/src/test/ui/if/if-without-else-as-fn-expr.rs b/src/test/ui/if/if-without-else-as-fn-expr.rs index 15892de83854c..826371be35f45 100644 --- a/src/test/ui/if/if-without-else-as-fn-expr.rs +++ b/src/test/ui/if/if-without-else-as-fn-expr.rs @@ -3,7 +3,6 @@ fn foo(bar: usize) -> usize { return 3; } //~^^^ ERROR if may be missing an else clause - //~| ERROR mismatched types [E0308] } fn foo2(bar: usize) -> usize { @@ -11,7 +10,6 @@ fn foo2(bar: usize) -> usize { return 3; }; //~^^^ ERROR if may be missing an else clause - //~| ERROR mismatched types [E0308] x } @@ -20,7 +18,6 @@ fn foo3(bar: usize) -> usize { 3 } //~^^^ ERROR if may be missing an else clause - //~| ERROR mismatched types [E0308] } fn foo_let(bar: usize) -> usize { @@ -28,7 +25,6 @@ fn foo_let(bar: usize) -> usize { return 3; } //~^^^ ERROR if may be missing an else clause - //~| ERROR mismatched types [E0308] } fn foo2_let(bar: usize) -> usize { @@ -36,7 +32,6 @@ fn foo2_let(bar: usize) -> usize { return 3; }; //~^^^ ERROR if may be missing an else clause - //~| ERROR mismatched types [E0308] x } @@ -45,7 +40,6 @@ fn foo3_let(bar: usize) -> usize { 3 } //~^^^ ERROR if may be missing an else clause - //~| ERROR mismatched types [E0308] } // FIXME(60254): deduplicate first error in favor of second. diff --git a/src/test/ui/if/if-without-else-as-fn-expr.stderr b/src/test/ui/if/if-without-else-as-fn-expr.stderr index 06600b1cb9aea..b49c2aa6319df 100644 --- a/src/test/ui/if/if-without-else-as-fn-expr.stderr +++ b/src/test/ui/if/if-without-else-as-fn-expr.stderr @@ -1,14 +1,3 @@ -error[E0308]: mismatched types - --> $DIR/if-without-else-as-fn-expr.rs:2:5 - | -LL | / if bar % 5 == 0 { -LL | | return 3; -LL | | } - | |_____^ expected usize, found () - | - = note: expected type `usize` - found type `()` - error[E0317]: if may be missing an else clause --> $DIR/if-without-else-as-fn-expr.rs:2:5 | @@ -24,20 +13,8 @@ LL | | } = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type -error[E0308]: mismatched types - --> $DIR/if-without-else-as-fn-expr.rs:10:20 - | -LL | let x: usize = if bar % 5 == 0 { - | ____________________^ -LL | | return 3; -LL | | }; - | |_____^ expected usize, found () - | - = note: expected type `usize` - found type `()` - error[E0317]: if may be missing an else clause - --> $DIR/if-without-else-as-fn-expr.rs:10:20 + --> $DIR/if-without-else-as-fn-expr.rs:9:20 | LL | let x: usize = if bar % 5 == 0 { | _________-__________^ @@ -52,19 +29,8 @@ LL | | }; = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type -error[E0308]: mismatched types - --> $DIR/if-without-else-as-fn-expr.rs:19:5 - | -LL | / if bar % 5 == 0 { -LL | | 3 -LL | | } - | |_____^ expected usize, found () - | - = note: expected type `usize` - found type `()` - error[E0317]: if may be missing an else clause - --> $DIR/if-without-else-as-fn-expr.rs:19:5 + --> $DIR/if-without-else-as-fn-expr.rs:17:5 | LL | fn foo3(bar: usize) -> usize { | ----- expected `usize` because of this return type @@ -78,19 +44,8 @@ LL | | } = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type -error[E0308]: mismatched types - --> $DIR/if-without-else-as-fn-expr.rs:27:5 - | -LL | / if let 0 = 1 { -LL | | return 3; -LL | | } - | |_____^ expected usize, found () - | - = note: expected type `usize` - found type `()` - error[E0317]: if may be missing an else clause - --> $DIR/if-without-else-as-fn-expr.rs:27:5 + --> $DIR/if-without-else-as-fn-expr.rs:24:5 | LL | fn foo_let(bar: usize) -> usize { | ----- expected `usize` because of this return type @@ -104,20 +59,8 @@ LL | | } = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type -error[E0308]: mismatched types - --> $DIR/if-without-else-as-fn-expr.rs:35:20 - | -LL | let x: usize = if let 0 = 1 { - | ____________________^ -LL | | return 3; -LL | | }; - | |_____^ expected usize, found () - | - = note: expected type `usize` - found type `()` - error[E0317]: if may be missing an else clause - --> $DIR/if-without-else-as-fn-expr.rs:35:20 + --> $DIR/if-without-else-as-fn-expr.rs:31:20 | LL | let x: usize = if let 0 = 1 { | _________-__________^ @@ -132,19 +75,8 @@ LL | | }; = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type -error[E0308]: mismatched types - --> $DIR/if-without-else-as-fn-expr.rs:44:5 - | -LL | / if let 0 = 1 { -LL | | 3 -LL | | } - | |_____^ expected usize, found () - | - = note: expected type `usize` - found type `()` - error[E0317]: if may be missing an else clause - --> $DIR/if-without-else-as-fn-expr.rs:44:5 + --> $DIR/if-without-else-as-fn-expr.rs:39:5 | LL | fn foo3_let(bar: usize) -> usize { | ----- expected `usize` because of this return type @@ -158,7 +90,6 @@ LL | | } = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type -error: aborting due to 12 previous errors +error: aborting due to 6 previous errors -Some errors have detailed explanations: E0308, E0317. -For more information about an error, try `rustc --explain E0308`. +For more information about this error, try `rustc --explain E0317`. diff --git a/src/test/ui/issues/issue-50577.rs b/src/test/ui/issues/issue-50577.rs index bf892a8daa27f..f0f1dc6c28667 100644 --- a/src/test/ui/issues/issue-50577.rs +++ b/src/test/ui/issues/issue-50577.rs @@ -2,6 +2,5 @@ fn main() { enum Foo { Drop = assert_eq!(1, 1) //~^ ERROR if may be missing an else clause - //~| ERROR mismatched types [E0308] } } diff --git a/src/test/ui/issues/issue-50577.stderr b/src/test/ui/issues/issue-50577.stderr index 413c8c5c80b52..0c3ba2ea4f94d 100644 --- a/src/test/ui/issues/issue-50577.stderr +++ b/src/test/ui/issues/issue-50577.stderr @@ -1,13 +1,3 @@ -error[E0308]: mismatched types - --> $DIR/issue-50577.rs:3:16 - | -LL | Drop = assert_eq!(1, 1) - | ^^^^^^^^^^^^^^^^ expected isize, found () - | - = note: expected type `isize` - found type `()` - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - error[E0317]: if may be missing an else clause --> $DIR/issue-50577.rs:3:16 | @@ -23,7 +13,6 @@ LL | Drop = assert_eq!(1, 1) = help: consider adding an `else` block that evaluates to the expected type = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: aborting due to 2 previous errors +error: aborting due to previous error -Some errors have detailed explanations: E0308, E0317. -For more information about an error, try `rustc --explain E0308`. +For more information about this error, try `rustc --explain E0317`. From aae2b245e4c3958ca37d6eaa58b89d955d0b66e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 31 Aug 2019 18:42:13 -0700 Subject: [PATCH 2/3] deduplicate code --- src/librustc_typeck/check/_match.rs | 40 ++++++++++++----------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 3983a3b820391..63d059bdf2a80 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -112,36 +112,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } self.diverges.set(pats_diverge); - let arm_ty = if source_if { + let arm_ty = if source_if && if_no_else && i != 0 && self.if_fallback_coercion( + expr.span, + &arms[0].body, + &mut coercion, + ) { + tcx.types.err + } else { + // Only call this if this is not an `if` expr with an expected type and no `else` + // clause to avoid duplicated type errors. (#60254) + let arm_ty = self.check_expr_with_expectation(&arm.body, expected); + all_arms_diverge &= self.diverges.get(); + arm_ty + }; + if source_if { let then_expr = &arms[0].body; match (i, if_no_else) { - (0, _) => { - let arm_ty = self.check_expr_with_expectation(&arm.body, expected); - all_arms_diverge &= self.diverges.get(); - coercion.coerce(self, &self.misc(expr.span), &arm.body, arm_ty); - arm_ty - } - (_, true) => { - if self.if_fallback_coercion(expr.span, then_expr, &mut coercion) { - tcx.types.err - } else { - let arm_ty = self.check_expr_with_expectation(&arm.body, expected); - all_arms_diverge &= self.diverges.get(); - arm_ty - } - } + (0, _) => coercion.coerce(self, &self.misc(expr.span), &arm.body, arm_ty), + (_, true) => {} // Handled above to avoid duplicated type errors (#60254). (_, _) => { - let arm_ty = self.check_expr_with_expectation(&arm.body, expected); - all_arms_diverge &= self.diverges.get(); let then_ty = prior_arm_ty.unwrap(); let cause = self.if_cause(expr.span, then_expr, &arm.body, then_ty, arm_ty); coercion.coerce(self, &cause, &arm.body, arm_ty); - arm_ty } } } else { - let arm_ty = self.check_expr_with_expectation(&arm.body, expected); - all_arms_diverge &= self.diverges.get(); let arm_span = if let hir::ExprKind::Block(blk, _) = &arm.body.node { // Point at the block expr instead of the entire block blk.expr.as_ref().map(|e| e.span).unwrap_or(arm.body.span) @@ -166,8 +161,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if other_arms.len() > 5 { other_arms.remove(0); } - arm_ty - }; + } prior_arm_ty = Some(arm_ty); } From f53c2179ba2ae35bd241a28027fb703aec89139b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 1 Sep 2019 10:49:07 -0700 Subject: [PATCH 3/3] review comments --- src/librustc_typeck/check/_match.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 63d059bdf2a80..7427ae9ce8de3 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -117,14 +117,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &arms[0].body, &mut coercion, ) { - tcx.types.err + tcx.types.err } else { // Only call this if this is not an `if` expr with an expected type and no `else` // clause to avoid duplicated type errors. (#60254) - let arm_ty = self.check_expr_with_expectation(&arm.body, expected); - all_arms_diverge &= self.diverges.get(); - arm_ty + self.check_expr_with_expectation(&arm.body, expected) }; + all_arms_diverge &= self.diverges.get(); if source_if { let then_expr = &arms[0].body; match (i, if_no_else) { @@ -188,6 +187,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } /// Handle the fallback arm of a desugared if(-let) like a missing else. + /// + /// Returns `true` if there was an error forcing the coercion to the `()` type. fn if_fallback_coercion( &self, span: Span,