Skip to content

Commit

Permalink
Rollup merge of #121153 - chenyukang:yukang-fix-105431-type-mismatch,…
Browse files Browse the repository at this point in the history
… r=estebank

Suggest removing superfluous semicolon when statements used as expression

Fixes #105431

- it's not a pure recursive visitor, so I guess there may be some more complex scenarios not covered.
- moved `consider_removing_semicolon` to `compiler/rustc_infer` for reusing this helper function.
  • Loading branch information
matthiaskrgr committed Mar 1, 2024
2 parents 3c89280 + e2ce5d7 commit b8cdcfa
Show file tree
Hide file tree
Showing 6 changed files with 308 additions and 41 deletions.
6 changes: 5 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Expand Up @@ -1744,7 +1744,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Ty::new_unit(self.tcx),
);
}
if !self.consider_removing_semicolon(blk, expected_ty, err) {
if !self.err_ctxt().consider_removing_semicolon(
blk,
expected_ty,
err,
) {
self.err_ctxt().consider_returning_binding(
blk,
expected_ty,
Expand Down
41 changes: 1 addition & 40 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Expand Up @@ -22,7 +22,7 @@ use rustc_hir::{
Path, QPath, Stmt, StmtKind, TyKind, WherePredicate,
};
use rustc_hir_analysis::astconv::AstConv;
use rustc_infer::traits::{self, StatementAsExpression};
use rustc_infer::traits::{self};
use rustc_middle::lint::in_external_macro;
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::ty::print::with_no_trimmed_paths;
Expand Down Expand Up @@ -1791,45 +1791,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

/// A common error is to add an extra semicolon:
///
/// ```compile_fail,E0308
/// fn foo() -> usize {
/// 22;
/// }
/// ```
///
/// This routine checks if the final statement in a block is an
/// expression with an explicit semicolon whose type is compatible
/// with `expected_ty`. If so, it suggests removing the semicolon.
pub(crate) fn consider_removing_semicolon(
&self,
blk: &'tcx hir::Block<'tcx>,
expected_ty: Ty<'tcx>,
err: &mut Diag<'_>,
) -> bool {
if let Some((span_semi, boxed)) = self.err_ctxt().could_remove_semicolon(blk, expected_ty) {
if let StatementAsExpression::NeedsBoxing = boxed {
err.span_suggestion_verbose(
span_semi,
"consider removing this semicolon and boxing the expression",
"",
Applicability::HasPlaceholders,
);
} else {
err.span_suggestion_short(
span_semi,
"remove this semicolon to return this value",
"",
Applicability::MachineApplicable,
);
}
true
} else {
false
}
}

pub(crate) fn is_field_suggestable(
&self,
field: &ty::FieldDef,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Expand Up @@ -1989,6 +1989,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
self.suggest_accessing_field_where_appropriate(cause, &exp_found, diag);
self.suggest_await_on_expect_found(cause, span, &exp_found, diag);
self.suggest_function_pointers(cause, span, &exp_found, diag);
self.suggest_turning_stmt_into_expr(cause, &exp_found, diag);
}
}

Expand Down
96 changes: 96 additions & 0 deletions compiler/rustc_infer/src/infer/error_reporting/suggest.rs
@@ -1,8 +1,13 @@
use crate::infer::error_reporting::hir::Path;
use hir::def::CtorKind;
use hir::intravisit::{walk_expr, walk_stmt, Visitor};
use hir::{Local, QPath};
use rustc_data_structures::fx::FxIndexSet;
use rustc_errors::{Applicability, Diag};
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::MatchSource;
use rustc_hir::Node;
use rustc_middle::traits::{
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
StatementAsExpression,
Expand Down Expand Up @@ -293,6 +298,97 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
}
}

pub(super) fn suggest_turning_stmt_into_expr(
&self,
cause: &ObligationCause<'tcx>,
exp_found: &ty::error::ExpectedFound<Ty<'tcx>>,
diag: &mut Diag<'_>,
) {
let ty::error::ExpectedFound { expected, found } = exp_found;
if !found.peel_refs().is_unit() {
return;
}

let ObligationCauseCode::BlockTailExpression(hir_id, MatchSource::Normal) = cause.code()
else {
return;
};

let node = self.tcx.hir_node(*hir_id);
let mut blocks = vec![];
if let hir::Node::Block(block) = node
&& let Some(expr) = block.expr
&& let hir::ExprKind::Path(QPath::Resolved(_, Path { res, .. })) = expr.kind
&& let Res::Local(local) = res
&& let Node::Local(Local { init: Some(init), .. }) = self.tcx.parent_hir_node(*local)
{
fn collect_blocks<'hir>(expr: &hir::Expr<'hir>, blocks: &mut Vec<&hir::Block<'hir>>) {
match expr.kind {
// `blk1` and `blk2` must be have the same types, it will be reported before reaching here
hir::ExprKind::If(_, blk1, Some(blk2)) => {
collect_blocks(blk1, blocks);
collect_blocks(blk2, blocks);
}
hir::ExprKind::Match(_, arms, _) => {
// all arms must have same types
for arm in arms.iter() {
collect_blocks(arm.body, blocks);
}
}
hir::ExprKind::Block(blk, _) => {
blocks.push(blk);
}
_ => {}
}
}
collect_blocks(init, &mut blocks);
}

let expected_inner: Ty<'_> = expected.peel_refs();
for block in blocks.iter() {
self.consider_removing_semicolon(block, expected_inner, diag);
}
}

/// A common error is to add an extra semicolon:
///
/// ```compile_fail,E0308
/// fn foo() -> usize {
/// 22;
/// }
/// ```
///
/// This routine checks if the final statement in a block is an
/// expression with an explicit semicolon whose type is compatible
/// with `expected_ty`. If so, it suggests removing the semicolon.
pub fn consider_removing_semicolon(
&self,
blk: &'tcx hir::Block<'tcx>,
expected_ty: Ty<'tcx>,
diag: &mut Diag<'_>,
) -> bool {
if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) {
if let StatementAsExpression::NeedsBoxing = boxed {
diag.span_suggestion_verbose(
span_semi,
"consider removing this semicolon and boxing the expression",
"",
Applicability::HasPlaceholders,
);
} else {
diag.span_suggestion_short(
span_semi,
"remove this semicolon to return this value",
"",
Applicability::MachineApplicable,
);
}
true
} else {
false
}
}

pub(super) fn suggest_function_pointers(
&self,
cause: &ObligationCause<'tcx>,
Expand Down
76 changes: 76 additions & 0 deletions tests/ui/inference/stmts-as-exp-105431.rs
@@ -0,0 +1,76 @@
#![allow(unused)]

fn test_if() -> i32 {
let x = if true {
eprintln!("hello");
3;
}
else {
4;
};
x //~ ERROR mismatched types
}

fn test_if_without_binding() -> i32 {
if true { //~ ERROR mismatched types
eprintln!("hello");
3;
}
else { //~ ERROR mismatched types
4;
}
}

fn test_match() -> i32 {
let v = 1;
let res = match v {
1 => { 1; }
_ => { 2; }
};
res //~ ERROR mismatched types
}

fn test_match_match_without_binding() -> i32 {
let v = 1;
match v {
1 => { 1; } //~ ERROR mismatched types
_ => { 2; } //~ ERROR mismatched types
}
}

fn test_match_arm_different_types() -> i32 {
let v = 1;
let res = match v {
1 => { if 1 < 2 { 1 } else { 2 } }
_ => { 2; } //~ ERROR `match` arms have incompatible types
};
res
}

fn test_if_match_mixed() -> i32 {
let x = if true {
3;
} else {
match 1 {
1 => { 1 }
_ => { 2 }
};
};
x //~ ERROR mismatched types
}

fn test_if_match_mixed_failed() -> i32 {
let x = if true {
3;
} else {
// because this is a tailed expr, so we won't check deeper
match 1 {
1 => { 33; }
_ => { 44; }
}
};
x //~ ERROR mismatched types
}


fn main() {}
129 changes: 129 additions & 0 deletions tests/ui/inference/stmts-as-exp-105431.stderr
@@ -0,0 +1,129 @@
error[E0308]: mismatched types
--> $DIR/stmts-as-exp-105431.rs:11:5
|
LL | fn test_if() -> i32 {
| --- expected `i32` because of return type
...
LL | x
| ^ expected `i32`, found `()`
|
help: remove this semicolon to return this value
|
LL - 3;
LL + 3
|
help: remove this semicolon to return this value
|
LL - 4;
LL + 4
|

error[E0308]: mismatched types
--> $DIR/stmts-as-exp-105431.rs:15:13
|
LL | if true {
| _____________^
LL | | eprintln!("hello");
LL | | 3;
| | - help: remove this semicolon to return this value
LL | | }
| |_____^ expected `i32`, found `()`

error[E0308]: mismatched types
--> $DIR/stmts-as-exp-105431.rs:19:10
|
LL | else {
| __________^
LL | | 4;
| | - help: remove this semicolon to return this value
LL | | }
| |_____^ expected `i32`, found `()`

error[E0308]: mismatched types
--> $DIR/stmts-as-exp-105431.rs:30:5
|
LL | fn test_match() -> i32 {
| --- expected `i32` because of return type
...
LL | res
| ^^^ expected `i32`, found `()`
|
help: remove this semicolon to return this value
|
LL - 1 => { 1; }
LL + 1 => { 1 }
|
help: remove this semicolon to return this value
|
LL - _ => { 2; }
LL + _ => { 2 }
|

error[E0308]: mismatched types
--> $DIR/stmts-as-exp-105431.rs:36:14
|
LL | 1 => { 1; }
| ^^^-^^
| | |
| | help: remove this semicolon to return this value
| expected `i32`, found `()`

error[E0308]: mismatched types
--> $DIR/stmts-as-exp-105431.rs:37:14
|
LL | _ => { 2; }
| ^^^-^^
| | |
| | help: remove this semicolon to return this value
| expected `i32`, found `()`

error[E0308]: `match` arms have incompatible types
--> $DIR/stmts-as-exp-105431.rs:45:16
|
LL | let res = match v {
| _______________-
LL | | 1 => { if 1 < 2 { 1 } else { 2 } }
| | ------------------------- this is found to be of type `{integer}`
LL | | _ => { 2; }
| | ^-
| | ||
| | |help: consider removing this semicolon
| | expected integer, found `()`
LL | | };
| |_____- `match` arms have incompatible types

error[E0308]: mismatched types
--> $DIR/stmts-as-exp-105431.rs:59:5
|
LL | fn test_if_match_mixed() -> i32 {
| --- expected `i32` because of return type
...
LL | x
| ^ expected `i32`, found `()`
|
help: remove this semicolon to return this value
|
LL - 3;
LL + 3
|
help: remove this semicolon to return this value
|
LL - };
LL + }
|

error[E0308]: mismatched types
--> $DIR/stmts-as-exp-105431.rs:72:5
|
LL | fn test_if_match_mixed_failed() -> i32 {
| --- expected `i32` because of return type
LL | let x = if true {
LL | 3;
| - help: remove this semicolon to return this value
...
LL | x
| ^ expected `i32`, found `()`

error: aborting due to 9 previous errors

For more information about this error, try `rustc --explain E0308`.

0 comments on commit b8cdcfa

Please sign in to comment.