Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect bindings assigned blocks without tail expressions #106519

Merged
merged 3 commits into from
Jan 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut ret_span: MultiSpan = semi_span.into();
ret_span.push_span_label(
expr.span,
"this could be implicitly returned but it is a statement, not a \
tail expression",
"this could be implicitly returned but it is a statement, not a tail expression",
);
ret_span.push_span_label(ret, "the `match` arms can conform to this return type");
ret_span.push_span_label(
semi_span,
"the `match` is a statement because of this semicolon, consider \
removing it",
"the `match` is a statement because of this semicolon, consider removing it",
);
diag.span_note(ret_span, "you might have meant to return the `match` expression");
diag.tool_only_span_suggestion(
Expand Down
45 changes: 45 additions & 0 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.note_need_for_fn_pointer(err, expected, expr_ty);
self.note_internal_mutation_in_method(err, expr, expected, expr_ty);
self.check_for_range_as_method_call(err, expr, expr_ty, expected);
self.check_for_binding_assigned_block_without_tail_expression(err, expr, expr_ty, expected);
}

/// Requires that the two types unify, and prints an error message if
Expand Down Expand Up @@ -1670,4 +1671,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Applicability::MachineApplicable,
);
}

/// Identify when the type error is because `()` is found in a binding that was assigned a
/// block without a tail expression.
fn check_for_binding_assigned_block_without_tail_expression(
&self,
err: &mut Diagnostic,
expr: &hir::Expr<'_>,
checked_ty: Ty<'tcx>,
expected_ty: Ty<'tcx>,
) {
if !checked_ty.is_unit() {
return;
}
let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind else { return; };
let hir::def::Res::Local(hir_id) = path.res else { return; };
let Some(hir::Node::Pat(pat)) = self.tcx.hir().find(hir_id) else {
return;
};
let Some(hir::Node::Local(hir::Local {
ty: None,
init: Some(init),
..
})) = self.tcx.hir().find_parent(pat.hir_id) else { return; };
let hir::ExprKind::Block(block, None) = init.kind else { return; };
if block.expr.is_some() {
return;
}
let [.., stmt] = block.stmts else {
err.span_label(block.span, "this empty block is missing a tail expression");
return;
};
let hir::StmtKind::Semi(tail_expr) = stmt.kind else { return; };
let Some(ty) = self.node_ty_opt(tail_expr.hir_id) else { return; };
if self.can_eq(self.param_env, expected_ty, ty).is_ok() {
err.span_suggestion_short(
stmt.span.with_lo(tail_expr.span.hi()),
"remove this semicolon",
"",
Applicability::MachineApplicable,
);
} else {
err.span_label(block.span, "this block is missing a tail expression");
}
}
}
54 changes: 36 additions & 18 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
),
}
};

self.check_for_binding_assigned_block_without_tail_expression(
&obligation,
&mut err,
trait_predicate,
);
if self.suggest_add_reference_to_arg(
&obligation,
&mut err,
Expand Down Expand Up @@ -2267,23 +2271,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
if let (Some(body_id), Some(ty::subst::GenericArgKind::Type(_))) =
(body_id, subst.map(|subst| subst.unpack()))
{
struct FindExprBySpan<'hir> {
span: Span,
result: Option<&'hir hir::Expr<'hir>>,
}

impl<'v> hir::intravisit::Visitor<'v> for FindExprBySpan<'v> {
fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
if self.span == ex.span {
self.result = Some(ex);
} else {
hir::intravisit::walk_expr(self, ex);
}
}
}

let mut expr_finder = FindExprBySpan { span, result: None };

let mut expr_finder = FindExprBySpan::new(span);
expr_finder.visit_expr(&self.tcx.hir().body(body_id).value);

if let Some(hir::Expr {
Expand Down Expand Up @@ -2770,6 +2758,36 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
}
}

/// Crude way of getting back an `Expr` from a `Span`.
pub struct FindExprBySpan<'hir> {
pub span: Span,
pub result: Option<&'hir hir::Expr<'hir>>,
pub ty_result: Option<&'hir hir::Ty<'hir>>,
}

impl<'hir> FindExprBySpan<'hir> {
fn new(span: Span) -> Self {
Self { span, result: None, ty_result: None }
}
}

impl<'v> Visitor<'v> for FindExprBySpan<'v> {
fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
if self.span == ex.span {
self.result = Some(ex);
} else {
hir::intravisit::walk_expr(self, ex);
}
}
fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
if self.span == ty.span {
self.ty_result = Some(ty);
} else {
hir::intravisit::walk_ty(self, ty);
}
}
}

/// Look for type `param` in an ADT being used only through a reference to confirm that suggesting
/// `param: ?Sized` would be a valid constraint.
struct FindTypeParam {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// ignore-tidy-filelength

use super::{DefIdOrName, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation};
use super::{
DefIdOrName, FindExprBySpan, Obligation, ObligationCause, ObligationCauseCode,
PredicateObligation,
};

use crate::autoderef::Autoderef;
use crate::infer::InferCtxt;
Expand Down Expand Up @@ -196,6 +199,13 @@ pub trait TypeErrCtxtExt<'tcx> {
trait_pred: ty::PolyTraitPredicate<'tcx>,
) -> bool;

fn check_for_binding_assigned_block_without_tail_expression(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut Diagnostic,
trait_pred: ty::PolyTraitPredicate<'tcx>,
);

fn suggest_add_reference_to_arg(
&self,
obligation: &PredicateObligation<'tcx>,
Expand Down Expand Up @@ -1032,6 +1042,66 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
true
}

fn check_for_binding_assigned_block_without_tail_expression(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut Diagnostic,
trait_pred: ty::PolyTraitPredicate<'tcx>,
) {
let mut span = obligation.cause.span;
while span.from_expansion() {
// Remove all the desugaring and macro contexts.
span.remove_mark();
}
let mut expr_finder = FindExprBySpan::new(span);
let Some(hir::Node::Expr(body)) = self.tcx.hir().find(obligation.cause.body_id) else { return; };
expr_finder.visit_expr(&body);
let Some(expr) = expr_finder.result else { return; };
let Some(typeck) = &self.typeck_results else { return; };
let Some(ty) = typeck.expr_ty_adjusted_opt(expr) else { return; };
if !ty.is_unit() {
return;
};
let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind else { return; };
let hir::def::Res::Local(hir_id) = path.res else { return; };
let Some(hir::Node::Pat(pat)) = self.tcx.hir().find(hir_id) else {
return;
};
let Some(hir::Node::Local(hir::Local {
ty: None,
init: Some(init),
..
})) = self.tcx.hir().find_parent(pat.hir_id) else { return; };
let hir::ExprKind::Block(block, None) = init.kind else { return; };
if block.expr.is_some() {
return;
}
let [.., stmt] = block.stmts else {
err.span_label(block.span, "this empty block is missing a tail expression");
return;
};
let hir::StmtKind::Semi(tail_expr) = stmt.kind else { return; };
let Some(ty) = typeck.expr_ty_opt(tail_expr) else {
err.span_label(block.span, "this block is missing a tail expression");
return;
};
let ty = self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(ty));
let trait_pred_and_self = trait_pred.map_bound(|trait_pred| (trait_pred, ty));

let new_obligation =
self.mk_trait_obligation_with_new_self_ty(obligation.param_env, trait_pred_and_self);
if self.predicate_must_hold_modulo_regions(&new_obligation) {
err.span_suggestion_short(
stmt.span.with_lo(tail_expr.span.hi()),
"remove this semicolon",
"",
Applicability::MachineApplicable,
);
} else {
err.span_label(block.span, "this block is missing a tail expression");
}
}

fn suggest_add_reference_to_arg(
&self,
obligation: &PredicateObligation<'tcx>,
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/type/binding-assigned-block-without-tail-expression.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
struct S;
fn main() {
let x = {
println!("foo");
42;
};
let y = {};
let z = {
"hi";
};
let s = {
S;
};
println!("{}", x); //~ ERROR E0277
println!("{}", y); //~ ERROR E0277
println!("{}", z); //~ ERROR E0277
println!("{}", s); //~ ERROR E0277
let _: i32 = x; //~ ERROR E0308
let _: i32 = y; //~ ERROR E0308
let _: i32 = z; //~ ERROR E0308
let _: i32 = s; //~ ERROR E0308
}
109 changes: 109 additions & 0 deletions src/test/ui/type/binding-assigned-block-without-tail-expression.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
error[E0277]: `()` doesn't implement `std::fmt::Display`
--> $DIR/binding-assigned-block-without-tail-expression.rs:14:20
|
LL | 42;
| - help: remove this semicolon
...
LL | println!("{}", x);
| ^ `()` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `()`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `()` doesn't implement `std::fmt::Display`
--> $DIR/binding-assigned-block-without-tail-expression.rs:15:20
|
LL | let y = {};
| -- this empty block is missing a tail expression
...
LL | println!("{}", y);
| ^ `()` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `()`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `()` doesn't implement `std::fmt::Display`
--> $DIR/binding-assigned-block-without-tail-expression.rs:16:20
|
LL | "hi";
| - help: remove this semicolon
...
LL | println!("{}", z);
| ^ `()` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `()`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `()` doesn't implement `std::fmt::Display`
--> $DIR/binding-assigned-block-without-tail-expression.rs:17:20
|
LL | let s = {
| _____________-
LL | | S;
LL | | };
| |_____- this block is missing a tail expression
...
LL | println!("{}", s);
| ^ `()` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `()`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
--> $DIR/binding-assigned-block-without-tail-expression.rs:18:18
|
LL | 42;
| - help: remove this semicolon
...
LL | let _: i32 = x;
| --- ^ expected `i32`, found `()`
| |
| expected due to this

error[E0308]: mismatched types
--> $DIR/binding-assigned-block-without-tail-expression.rs:19:18
|
LL | let y = {};
| -- this empty block is missing a tail expression
...
LL | let _: i32 = y;
| --- ^ expected `i32`, found `()`
| |
| expected due to this

error[E0308]: mismatched types
--> $DIR/binding-assigned-block-without-tail-expression.rs:20:18
|
LL | let z = {
| _____________-
LL | | "hi";
LL | | };
| |_____- this block is missing a tail expression
...
LL | let _: i32 = z;
| --- ^ expected `i32`, found `()`
| |
| expected due to this

error[E0308]: mismatched types
--> $DIR/binding-assigned-block-without-tail-expression.rs:21:18
|
LL | let s = {
| _____________-
LL | | S;
LL | | };
| |_____- this block is missing a tail expression
...
LL | let _: i32 = s;
| --- ^ expected `i32`, found `()`
| |
| expected due to this

error: aborting due to 8 previous errors

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.