Skip to content

Commit e875391

Browse files
borsbors
committed
Auto merge of #123812 - compiler-errors:additional-fixes, r=fmease
Follow-up fixes to `report_return_mismatched_types` Some renames, simplifications, fixes, etc. Follow-ups to #123804. I don't think it totally disentangles this code, but it does remove some of the worst offenders on the "I am so confused" scale (e.g. `get_node_fn_decl`).
2 parents 1d0e4af + a2a6fe7 commit e875391

File tree

12 files changed

+170
-173
lines changed

12 files changed

+170
-173
lines changed

Diff for: ‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
992992
}
993993
}
994994

995-
if look_at_return && hir.get_return_block(closure_id).is_some() {
995+
if look_at_return && hir.get_fn_id_for_return_block(closure_id).is_some() {
996996
// ...otherwise we are probably in the tail expression of the function, point at the
997997
// return type.
998998
match self.infcx.tcx.hir_node_by_def_id(hir.get_parent_item(fn_call_id).def_id) {

Diff for: ‎compiler/rustc_hir_typeck/src/coercion.rs

+47-57
Original file line numberDiff line numberDiff line change
@@ -1591,31 +1591,28 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
15911591
err.span_label(cause.span, "return type is not `()`");
15921592
}
15931593
ObligationCauseCode::BlockTailExpression(blk_id, ..) => {
1594-
let parent_id = fcx.tcx.parent_hir_id(blk_id);
15951594
err = self.report_return_mismatched_types(
15961595
cause,
15971596
expected,
15981597
found,
15991598
coercion_error,
16001599
fcx,
1601-
parent_id,
1600+
blk_id,
16021601
expression,
1603-
Some(blk_id),
16041602
);
16051603
if !fcx.tcx.features().unsized_locals {
16061604
unsized_return = self.is_return_ty_definitely_unsized(fcx);
16071605
}
16081606
}
1609-
ObligationCauseCode::ReturnValue(id) => {
1607+
ObligationCauseCode::ReturnValue(return_expr_id) => {
16101608
err = self.report_return_mismatched_types(
16111609
cause,
16121610
expected,
16131611
found,
16141612
coercion_error,
16151613
fcx,
1616-
id,
1614+
return_expr_id,
16171615
expression,
1618-
None,
16191616
);
16201617
if !fcx.tcx.features().unsized_locals {
16211618
unsized_return = self.is_return_ty_definitely_unsized(fcx);
@@ -1809,13 +1806,14 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
18091806
found: Ty<'tcx>,
18101807
ty_err: TypeError<'tcx>,
18111808
fcx: &FnCtxt<'a, 'tcx>,
1812-
id: hir::HirId,
1809+
block_or_return_id: hir::HirId,
18131810
expression: Option<&'tcx hir::Expr<'tcx>>,
1814-
blk_id: Option<hir::HirId>,
18151811
) -> Diag<'a> {
18161812
let mut err = fcx.err_ctxt().report_mismatched_types(cause, expected, found, ty_err);
18171813

1818-
let parent_id = fcx.tcx.parent_hir_id(id);
1814+
let due_to_block = matches!(fcx.tcx.hir_node(block_or_return_id), hir::Node::Block(..));
1815+
1816+
let parent_id = fcx.tcx.parent_hir_id(block_or_return_id);
18191817
let parent = fcx.tcx.hir_node(parent_id);
18201818
if let Some(expr) = expression
18211819
&& let hir::Node::Expr(hir::Expr {
@@ -1829,72 +1827,64 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
18291827
// Verify that this is a tail expression of a function, otherwise the
18301828
// label pointing out the cause for the type coercion will be wrong
18311829
// as prior return coercions would not be relevant (#57664).
1832-
let fn_decl = if let (Some(expr), Some(blk_id)) = (expression, blk_id) {
1830+
if let Some(expr) = expression
1831+
&& due_to_block
1832+
{
18331833
fcx.suggest_missing_semicolon(&mut err, expr, expected, false);
1834-
let pointing_at_return_type =
1835-
fcx.suggest_mismatched_types_on_tail(&mut err, expr, expected, found, blk_id);
1836-
if let (Some(cond_expr), true, false) = (
1837-
fcx.tcx.hir().get_if_cause(expr.hir_id),
1838-
expected.is_unit(),
1839-
pointing_at_return_type,
1840-
)
1834+
let pointing_at_return_type = fcx.suggest_mismatched_types_on_tail(
1835+
&mut err,
1836+
expr,
1837+
expected,
1838+
found,
1839+
block_or_return_id,
1840+
);
1841+
if let Some(cond_expr) = fcx.tcx.hir().get_if_cause(expr.hir_id)
1842+
&& expected.is_unit()
1843+
&& !pointing_at_return_type
18411844
// If the block is from an external macro or try (`?`) desugaring, then
18421845
// do not suggest adding a semicolon, because there's nowhere to put it.
18431846
// See issues #81943 and #87051.
18441847
&& matches!(
18451848
cond_expr.span.desugaring_kind(),
18461849
None | Some(DesugaringKind::WhileLoop)
1847-
) && !in_external_macro(fcx.tcx.sess, cond_expr.span)
1848-
&& !matches!(
1849-
cond_expr.kind,
1850-
hir::ExprKind::Match(.., hir::MatchSource::TryDesugar(_))
1851-
)
1850+
)
1851+
&& !in_external_macro(fcx.tcx.sess, cond_expr.span)
1852+
&& !matches!(
1853+
cond_expr.kind,
1854+
hir::ExprKind::Match(.., hir::MatchSource::TryDesugar(_))
1855+
)
18521856
{
18531857
err.span_label(cond_expr.span, "expected this to be `()`");
18541858
if expr.can_have_side_effects() {
18551859
fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
18561860
}
18571861
}
1858-
fcx.get_node_fn_decl(parent)
1859-
.map(|(fn_id, fn_decl, _, is_main)| (fn_id, fn_decl, is_main))
1860-
} else {
1861-
fcx.get_fn_decl(parent_id)
18621862
};
18631863

1864-
if let Some((fn_id, fn_decl, can_suggest)) = fn_decl {
1865-
if blk_id.is_none() {
1866-
fcx.suggest_missing_return_type(
1867-
&mut err,
1868-
fn_decl,
1869-
expected,
1870-
found,
1871-
can_suggest,
1872-
fn_id,
1873-
);
1874-
}
1875-
}
1876-
1877-
let mut parent_id = fcx.tcx.hir().get_parent_item(id).def_id;
1878-
let mut parent_item = fcx.tcx.hir_node_by_def_id(parent_id);
1879-
// When suggesting return, we need to account for closures and async blocks, not just items.
1880-
for (_, node) in fcx.tcx.hir().parent_iter(id) {
1881-
match node {
1882-
hir::Node::Expr(&hir::Expr {
1883-
kind: hir::ExprKind::Closure(hir::Closure { def_id, .. }),
1884-
..
1885-
}) => {
1886-
parent_item = node;
1887-
parent_id = *def_id;
1888-
break;
1889-
}
1890-
hir::Node::Item(_) | hir::Node::TraitItem(_) | hir::Node::ImplItem(_) => break,
1891-
_ => {}
1892-
}
1864+
// If this is due to an explicit `return`, suggest adding a return type.
1865+
if let Some((fn_id, fn_decl, can_suggest)) = fcx.get_fn_decl(parent_id)
1866+
&& !due_to_block
1867+
{
1868+
fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, can_suggest, fn_id);
18931869
}
18941870

1895-
if let (Some(expr), Some(_), Some(fn_decl)) = (expression, blk_id, parent_item.fn_decl()) {
1871+
// If this is due to a block, then maybe we forgot a `return`/`break`.
1872+
if due_to_block
1873+
&& let Some(expr) = expression
1874+
&& let Some((parent_fn_decl, parent_id)) = fcx
1875+
.tcx
1876+
.hir()
1877+
.parent_iter(block_or_return_id)
1878+
.find_map(|(_, node)| Some((node.fn_decl()?, node.associated_body()?.0)))
1879+
{
18961880
fcx.suggest_missing_break_or_return_expr(
1897-
&mut err, expr, fn_decl, expected, found, id, parent_id,
1881+
&mut err,
1882+
expr,
1883+
parent_fn_decl,
1884+
expected,
1885+
found,
1886+
block_or_return_id,
1887+
parent_id,
18981888
);
18991889
}
19001890

Diff for: ‎compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

+68-75
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use rustc_middle::{bug, span_bug};
3333
use rustc_session::lint;
3434
use rustc_span::def_id::LocalDefId;
3535
use rustc_span::hygiene::DesugaringKind;
36-
use rustc_span::symbol::{kw, sym, Ident};
36+
use rustc_span::symbol::{kw, sym};
3737
use rustc_span::Span;
3838
use rustc_target::abi::FieldIdx;
3939
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
@@ -867,76 +867,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
867867
)
868868
}
869869

870-
/// Given a function `Node`, return its `HirId` and `FnDecl` if it exists. Given a closure
871-
/// that is the child of a function, return that function's `HirId` and `FnDecl` instead.
872-
/// This may seem confusing at first, but this is used in diagnostics for `async fn`,
873-
/// for example, where most of the type checking actually happens within a nested closure,
874-
/// but we often want access to the parent function's signature.
875-
///
876-
/// Otherwise, return false.
877-
pub(crate) fn get_node_fn_decl(
878-
&self,
879-
node: Node<'tcx>,
880-
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, Ident, bool)> {
881-
match node {
882-
Node::Item(&hir::Item {
883-
ident,
884-
kind: hir::ItemKind::Fn(ref sig, ..),
885-
owner_id,
886-
..
887-
}) => {
888-
// This is less than ideal, it will not suggest a return type span on any
889-
// method called `main`, regardless of whether it is actually the entry point,
890-
// but it will still present it as the reason for the expected type.
891-
Some((owner_id.def_id, &sig.decl, ident, ident.name != sym::main))
892-
}
893-
Node::TraitItem(&hir::TraitItem {
894-
ident,
895-
kind: hir::TraitItemKind::Fn(ref sig, ..),
896-
owner_id,
897-
..
898-
}) => Some((owner_id.def_id, &sig.decl, ident, true)),
899-
Node::ImplItem(&hir::ImplItem {
900-
ident,
901-
kind: hir::ImplItemKind::Fn(ref sig, ..),
902-
owner_id,
903-
..
904-
}) => Some((owner_id.def_id, &sig.decl, ident, false)),
905-
Node::Expr(&hir::Expr {
906-
hir_id,
907-
kind:
908-
hir::ExprKind::Closure(hir::Closure {
909-
kind: hir::ClosureKind::Coroutine(..), ..
910-
}),
911-
..
912-
}) => {
913-
let (ident, sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
914-
Node::Item(&hir::Item {
915-
ident,
916-
kind: hir::ItemKind::Fn(ref sig, ..),
917-
owner_id,
918-
..
919-
}) => (ident, sig, owner_id),
920-
Node::TraitItem(&hir::TraitItem {
921-
ident,
922-
kind: hir::TraitItemKind::Fn(ref sig, ..),
923-
owner_id,
924-
..
925-
}) => (ident, sig, owner_id),
926-
Node::ImplItem(&hir::ImplItem {
927-
ident,
928-
kind: hir::ImplItemKind::Fn(ref sig, ..),
929-
owner_id,
930-
..
931-
}) => (ident, sig, owner_id),
932-
_ => return None,
933-
};
934-
Some((owner_id.def_id, &sig.decl, ident, ident.name != sym::main))
935-
}
936-
_ => None,
937-
}
938-
}
939-
940870
/// Given a `HirId`, return the `HirId` of the enclosing function, its `FnDecl`, and whether a
941871
/// suggestion can be made, `None` otherwise.
942872
pub fn get_fn_decl(
@@ -945,10 +875,73 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
945875
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, bool)> {
946876
// Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
947877
// `while` before reaching it, as block tail returns are not available in them.
948-
self.tcx.hir().get_return_block(blk_id).and_then(|blk_id| {
949-
let parent = self.tcx.hir_node(blk_id);
950-
self.get_node_fn_decl(parent)
951-
.map(|(fn_id, fn_decl, _, is_main)| (fn_id, fn_decl, is_main))
878+
self.tcx.hir().get_fn_id_for_return_block(blk_id).and_then(|item_id| {
879+
match self.tcx.hir_node(item_id) {
880+
Node::Item(&hir::Item {
881+
ident,
882+
kind: hir::ItemKind::Fn(ref sig, ..),
883+
owner_id,
884+
..
885+
}) => {
886+
// This is less than ideal, it will not suggest a return type span on any
887+
// method called `main`, regardless of whether it is actually the entry point,
888+
// but it will still present it as the reason for the expected type.
889+
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
890+
}
891+
Node::TraitItem(&hir::TraitItem {
892+
kind: hir::TraitItemKind::Fn(ref sig, ..),
893+
owner_id,
894+
..
895+
}) => Some((owner_id.def_id, sig.decl, true)),
896+
// FIXME: Suggestable if this is not a trait implementation
897+
Node::ImplItem(&hir::ImplItem {
898+
kind: hir::ImplItemKind::Fn(ref sig, ..),
899+
owner_id,
900+
..
901+
}) => Some((owner_id.def_id, sig.decl, false)),
902+
Node::Expr(&hir::Expr {
903+
hir_id,
904+
kind: hir::ExprKind::Closure(&hir::Closure { def_id, kind, fn_decl, .. }),
905+
..
906+
}) => {
907+
match kind {
908+
hir::ClosureKind::CoroutineClosure(_) => {
909+
// FIXME(async_closures): Implement this.
910+
return None;
911+
}
912+
hir::ClosureKind::Closure => Some((def_id, fn_decl, true)),
913+
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
914+
_,
915+
hir::CoroutineSource::Fn,
916+
)) => {
917+
let (ident, sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
918+
Node::Item(&hir::Item {
919+
ident,
920+
kind: hir::ItemKind::Fn(ref sig, ..),
921+
owner_id,
922+
..
923+
}) => (ident, sig, owner_id),
924+
Node::TraitItem(&hir::TraitItem {
925+
ident,
926+
kind: hir::TraitItemKind::Fn(ref sig, ..),
927+
owner_id,
928+
..
929+
}) => (ident, sig, owner_id),
930+
Node::ImplItem(&hir::ImplItem {
931+
ident,
932+
kind: hir::ImplItemKind::Fn(ref sig, ..),
933+
owner_id,
934+
..
935+
}) => (ident, sig, owner_id),
936+
_ => return None,
937+
};
938+
Some((owner_id.def_id, sig.decl, ident.name != sym::main))
939+
}
940+
_ => None,
941+
}
942+
}
943+
_ => None,
944+
}
952945
})
953946
}
954947

Diff for: ‎compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

+2-11
Original file line numberDiff line numberDiff line change
@@ -1774,15 +1774,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17741774
// that highlight errors inline.
17751775
let mut sp = blk.span;
17761776
let mut fn_span = None;
1777-
if let Some((decl, ident)) = self.get_parent_fn_decl(blk.hir_id) {
1777+
if let Some((fn_def_id, decl, _)) = self.get_fn_decl(blk.hir_id) {
17781778
let ret_sp = decl.output.span();
17791779
if let Some(block_sp) = self.parent_item_span(blk.hir_id) {
17801780
// HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the
17811781
// output would otherwise be incorrect and even misleading. Make sure
17821782
// the span we're aiming at correspond to a `fn` body.
17831783
if block_sp == blk.span {
17841784
sp = ret_sp;
1785-
fn_span = Some(ident.span);
1785+
fn_span = self.tcx.def_ident_span(fn_def_id);
17861786
}
17871787
}
17881788
}
@@ -1897,15 +1897,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
18971897
None
18981898
}
18991899

1900-
/// Given a function block's `HirId`, returns its `FnDecl` if it exists, or `None` otherwise.
1901-
pub(crate) fn get_parent_fn_decl(
1902-
&self,
1903-
blk_id: HirId,
1904-
) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident)> {
1905-
let parent = self.tcx.hir_node_by_def_id(self.tcx.hir().get_parent_item(blk_id).def_id);
1906-
self.get_node_fn_decl(parent).map(|(_, fn_decl, ident, _)| (fn_decl, ident))
1907-
}
1908-
19091900
/// If `expr` is a `match` expression that has only one non-`!` arm, use that arm's tail
19101901
/// expression's `Span`, otherwise return `expr.span`. This is done to give better errors
19111902
/// when given code like the following:

Diff for: ‎compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
800800
can_suggest: bool,
801801
fn_id: LocalDefId,
802802
) -> bool {
803+
// Can't suggest `->` on a block-like coroutine
804+
if let Some(hir::CoroutineKind::Desugared(_, hir::CoroutineSource::Block)) =
805+
self.tcx.coroutine_kind(fn_id)
806+
{
807+
return false;
808+
}
809+
803810
let found =
804811
self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
805812
// Only suggest changing the return type for methods that
@@ -1909,7 +1916,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
19091916
let returned = matches!(
19101917
self.tcx.parent_hir_node(expr.hir_id),
19111918
hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Ret(_), .. })
1912-
) || map.get_return_block(expr.hir_id).is_some();
1919+
) || map.get_fn_id_for_return_block(expr.hir_id).is_some();
19131920
if returned
19141921
&& let ty::Adt(e, args_e) = expected.kind()
19151922
&& let ty::Adt(f, args_f) = found.kind()

0 commit comments

Comments
 (0)