bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #98784 - compiler-errors:forgot-to-return-binding, r=estebank

Browse source 
Suggest returning local on "expected `ty`, found `()`" due to expr-less block

Putting this up for _initial_ review. Notably, this doesn't consider if the value has possibly been moved, or whether the type is `Copy`. It also provides a structured suggestion if there's one "preferred" binding that matches the type (i.e. one binding in the block or its parent), otherwise it just points them out if there's fewer than 4 of them.

Fixes #98177

r? `@estebank`
This commit is contained in:
Dylan DPC 2022-07-20 11:29:37 +05:30 • committed by GitHub
commit 97c9f16a45
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 314 additions and 52 deletions
Download patch file Download diff file Expand all files Collapse all files

44
compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
View file

@ -31,9 +31,7 @@ use rustc_middle::ty::{self, DefIdTree, IsSuggestable, Ty};
use rustc_session::Session;
use rustc_span::symbol::Ident;
use rustc_span::{self, Span};
use rustc_trait_selection::traits::{
self, ObligationCauseCode, SelectionContext, StatementAsExpression,
};
use rustc_trait_selection::traits::{self, ObligationCauseCode, SelectionContext};
use std::iter;
use std::slice;
@ -1410,7 +1408,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self.misc(sp),
&mut |err| {
if let Some(expected_ty) = expected.only_has_type(self) {
self.consider_hint_about_removing_semicolon(blk, expected_ty, err);
if !self.consider_removing_semicolon(blk, expected_ty, err) {
self.consider_returning_binding(blk, expected_ty, err);
}
if expected_ty == self.tcx.types.bool {
// If this is caused by a missing `let` in a `while let`,
// silence this redundant error, as we already emit E0070.
@ -1478,42 +1478,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty
}
/// 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.
fn consider_hint_about_removing_semicolon(
&self,
blk: &'tcx hir::Block<'tcx>,
expected_ty: Ty<'tcx>,
err: &mut Diagnostic,
) {
if let Some((span_semi, boxed)) = self.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",
"",
Applicability::MachineApplicable,
);
}
}
}
fn parent_item_span(&self, id: hir::HirId) -> Option<Span> {
let node = self.tcx.hir().get_by_def_id(self.tcx.hir().get_parent_item(id));
match node {

159
compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs
View file

@ -3,6 +3,7 @@ use crate::astconv::AstConv;
use crate::errors::{AddReturnTypeSuggestion, ExpectedReturnTypeLabel};
use rustc_ast::util::parser::ExprPrecedence;
use rustc_data_structures::stable_set::FxHashSet;
use rustc_errors::{Applicability, Diagnostic, MultiSpan};
use rustc_hir as hir;
use rustc_hir::def::{CtorOf, DefKind};
@ -11,12 +12,12 @@ use rustc_hir::{
Expr, ExprKind, GenericBound, Node, Path, QPath, Stmt, StmtKind, TyKind, WherePredicate,
};
use rustc_infer::infer::{self, TyCtxtInferExt};
use rustc_infer::traits;
use rustc_infer::traits::{self, StatementAsExpression};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, Binder, IsSuggestable, Subst, ToPredicate, Ty};
use rustc_middle::ty::{self, Binder, IsSuggestable, Subst, ToPredicate, Ty, TypeVisitable};
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub(in super::super) fn suggest_semicolon_at_end(&self, span: Span, err: &mut Diagnostic) {
@ -864,4 +865,156 @@ 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 Diagnostic,
) -> bool {
if let Some((span_semi, boxed)) = self.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",
"",
Applicability::MachineApplicable,
);
}
true
} else {
false
}
}
pub(crate) fn consider_returning_binding(
&self,
blk: &'tcx hir::Block<'tcx>,
expected_ty: Ty<'tcx>,
err: &mut Diagnostic,
) {
let mut shadowed = FxHashSet::default();
let mut candidate_idents = vec![];
let mut find_compatible_candidates = |pat: &hir::Pat<'_>| {
if let hir::PatKind::Binding(_, hir_id, ident, _) = &pat.kind
&& let Some(pat_ty) = self.typeck_results.borrow().node_type_opt(*hir_id)
{
let pat_ty = self.resolve_vars_if_possible(pat_ty);
if self.can_coerce(pat_ty, expected_ty)
&& !(pat_ty, expected_ty).references_error()
&& shadowed.insert(ident.name)
{
candidate_idents.push((*ident, pat_ty));
}
}
true
};
let hir = self.tcx.hir();
for stmt in blk.stmts.iter().rev() {
let StmtKind::Local(local) = &stmt.kind else { continue; };
local.pat.walk(&mut find_compatible_candidates);
}
match hir.find(hir.get_parent_node(blk.hir_id)) {
Some(hir::Node::Expr(hir::Expr { hir_id, .. })) => {
match hir.find(hir.get_parent_node(*hir_id)) {
Some(hir::Node::Arm(hir::Arm { pat, .. })) => {
pat.walk(&mut find_compatible_candidates);
}
Some(
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body), .. })
| hir::Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Fn(_, body),
..
})
| hir::Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Fn(_, hir::TraitFn::Provided(body)),
..
})
| hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Closure(hir::Closure { body, .. }),
..
}),
) => {
for param in hir.body(*body).params {
param.pat.walk(&mut find_compatible_candidates);
}
}
Some(hir::Node::Expr(hir::Expr {
kind:
hir::ExprKind::If(
hir::Expr { kind: hir::ExprKind::Let(let_), .. },
then_block,
_,
),
..
})) if then_block.hir_id == *hir_id => {
let_.pat.walk(&mut find_compatible_candidates);
}
_ => {}
}
}
_ => {}
}
match &candidate_idents[..] {
[(ident, _ty)] => {
let sm = self.tcx.sess.source_map();
if let Some(stmt) = blk.stmts.last() {
let stmt_span = sm.stmt_span(stmt.span, blk.span);
let sugg = if sm.is_multiline(blk.span)
&& let Some(spacing) = sm.indentation_before(stmt_span)
{
format!("\n{spacing}{ident}")
} else {
format!(" {ident}")
};
err.span_suggestion_verbose(
stmt_span.shrink_to_hi(),
format!("consider returning the local binding `{ident}`"),
sugg,
Applicability::MachineApplicable,
);
} else {
let sugg = if sm.is_multiline(blk.span)
&& let Some(spacing) = sm.indentation_before(blk.span.shrink_to_lo())
{
format!("\n{spacing} {ident}\n{spacing}")
} else {
format!(" {ident} ")
};
let left_span = sm.span_through_char(blk.span, '{').shrink_to_hi();
err.span_suggestion_verbose(
sm.span_extend_while(left_span, |c| c.is_whitespace()).unwrap_or(left_span),
format!("consider returning the local binding `{ident}`"),
sugg,
Applicability::MachineApplicable,
);
}
}
values if (1..3).contains(&values.len()) => {
let spans = values.iter().map(|(ident, _)| ident.span).collect::<Vec<_>>();
err.span_note(spans, "consider returning one of these bindings");
}
_ => {}
}
}
}