Skip to content

Commit

Permalink
Unrolled build for rust-lang#123804
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#123804 - compiler-errors:podcrab-fix, r=jieyouxu

Stop using `HirId` for fn-like parents since closures are not `OwnerNode`s

This is a minimal fix for rust-lang#123273.

I'm overall pretty disappointed w/ the state of this code; although it's "just diagnostics", it still should be maintainable and understandable and neither of those are true. I believe this code really needs some major overhauling before anything more should be added to it, because there are subtle invariants that are being exercised and subsequently broken all over the place, and I don't think we should just paper over them (e.g.) by delaying bugs or things like that. I wouldn't be surprised if fixing up this code would also yield better diagnostics.
  • Loading branch information
rust-timer authored Apr 12, 2024
2 parents 616a8f8 + 68d7c83 commit fdd12c9
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 34 deletions.
15 changes: 5 additions & 10 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2004,16 +2004,17 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
}
}

let parent_id = fcx.tcx.hir().get_parent_item(id);
let mut parent_item = fcx.tcx.hir_node_by_def_id(parent_id.def_id);
let mut parent_id = fcx.tcx.hir().get_parent_item(id).def_id;
let mut parent_item = fcx.tcx.hir_node_by_def_id(parent_id);
// When suggesting return, we need to account for closures and async blocks, not just items.
for (_, node) in fcx.tcx.hir().parent_iter(id) {
match node {
hir::Node::Expr(&hir::Expr {
kind: hir::ExprKind::Closure(hir::Closure { .. }),
kind: hir::ExprKind::Closure(hir::Closure { def_id, .. }),
..
}) => {
parent_item = node;
parent_id = *def_id;
break;
}
hir::Node::Item(_) | hir::Node::TraitItem(_) | hir::Node::ImplItem(_) => break,
Expand All @@ -2023,13 +2024,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {

if let (Some(expr), Some(_), Some(fn_decl)) = (expression, blk_id, parent_item.fn_decl()) {
fcx.suggest_missing_break_or_return_expr(
&mut err,
expr,
fn_decl,
expected,
found,
id,
parent_id.into(),
&mut err, expr, fn_decl, expected, found, id, parent_id,
);
}

Expand Down
22 changes: 6 additions & 16 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub(in super::super) fn get_node_fn_decl(
&self,
node: Node<'tcx>,
) -> Option<(hir::HirId, &'tcx hir::FnDecl<'tcx>, Ident, bool)> {
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, Ident, bool)> {
match node {
Node::Item(&hir::Item {
ident,
Expand All @@ -953,25 +953,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// This is less than ideal, it will not suggest a return type span on any
// method called `main`, regardless of whether it is actually the entry point,
// but it will still present it as the reason for the expected type.
Some((
hir::HirId::make_owner(owner_id.def_id),
&sig.decl,
ident,
ident.name != sym::main,
))
Some((owner_id.def_id, &sig.decl, ident, ident.name != sym::main))
}
Node::TraitItem(&hir::TraitItem {
ident,
kind: hir::TraitItemKind::Fn(ref sig, ..),
owner_id,
..
}) => Some((hir::HirId::make_owner(owner_id.def_id), &sig.decl, ident, true)),
}) => Some((owner_id.def_id, &sig.decl, ident, true)),
Node::ImplItem(&hir::ImplItem {
ident,
kind: hir::ImplItemKind::Fn(ref sig, ..),
owner_id,
..
}) => Some((hir::HirId::make_owner(owner_id.def_id), &sig.decl, ident, false)),
}) => Some((owner_id.def_id, &sig.decl, ident, false)),
Node::Expr(&hir::Expr {
hir_id,
kind:
Expand Down Expand Up @@ -1001,12 +996,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}) => (ident, sig, owner_id),
_ => return None,
};
Some((
hir::HirId::make_owner(owner_id.def_id),
&sig.decl,
ident,
ident.name != sym::main,
))
Some((owner_id.def_id, &sig.decl, ident, ident.name != sym::main))
}
_ => None,
}
Expand All @@ -1017,7 +1007,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn get_fn_decl(
&self,
blk_id: hir::HirId,
) -> Option<(hir::HirId, &'tcx hir::FnDecl<'tcx>, bool)> {
) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, bool)> {
// Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
// `while` before reaching it, as block tail returns are not available in them.
self.tcx.hir().get_return_block(blk_id).and_then(|blk_id| {
Expand Down
18 changes: 10 additions & 8 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::method::probe::{IsSuggestion, Mode, ProbeScope};
use crate::rustc_middle::ty::Article;
use core::cmp::min;
use core::iter;
use hir::def_id::LocalDefId;
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
use rustc_data_structures::packed::Pu128;
use rustc_errors::{Applicability, Diag, MultiSpan};
Expand Down Expand Up @@ -796,7 +797,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected: Ty<'tcx>,
found: Ty<'tcx>,
can_suggest: bool,
fn_id: hir::HirId,
fn_id: LocalDefId,
) -> bool {
let found =
self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
Expand Down Expand Up @@ -923,7 +924,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err: &mut Diag<'_>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
fn_id: hir::HirId,
fn_id: LocalDefId,
) {
// Only apply the suggestion if:
// - the return type is a generic parameter
Expand All @@ -937,7 +938,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let ty::Param(expected_ty_as_param) = expected.kind() else { return };

let fn_node = self.tcx.hir_node(fn_id);
let fn_node = self.tcx.hir_node_by_def_id(fn_id);

let hir::Node::Item(hir::Item {
kind:
Expand Down Expand Up @@ -1031,7 +1032,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected: Ty<'tcx>,
found: Ty<'tcx>,
id: hir::HirId,
fn_id: hir::HirId,
fn_id: LocalDefId,
) {
if !expected.is_unit() {
return;
Expand Down Expand Up @@ -1083,11 +1084,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let can_return = match fn_decl.output {
hir::FnRetTy::Return(ty) => {
let ty = self.lowerer().lower_ty(ty);
let bound_vars = self.tcx.late_bound_vars(fn_id);
let bound_vars = self.tcx.late_bound_vars(self.tcx.local_def_id_to_hir_id(fn_id));
let ty = self
.tcx
.instantiate_bound_regions_with_erased(Binder::bind_with_vars(ty, bound_vars));
let ty = match self.tcx.asyncness(fn_id.owner) {
let ty = match self.tcx.asyncness(fn_id) {
ty::Asyncness::Yes => self.get_impl_future_output_ty(ty).unwrap_or_else(|| {
span_bug!(
fn_decl.output.span(),
Expand All @@ -1108,8 +1109,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => false,
};
if can_return
&& let Some(owner_node) = self.tcx.hir_node(fn_id).as_owner()
&& let Some(span) = expr.span.find_ancestor_inside(*owner_node.span())
&& let Some(span) = expr.span.find_ancestor_inside(
self.tcx.hir().span_with_body(self.tcx.local_def_id_to_hir_id(fn_id)),
)
{
err.multipart_suggestion(
"you might have meant to return this value",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//@ edition: 2021

fn call(_: impl Fn() -> bool) {}

async fn test() {
call(|| -> Option<()> {
//~^ ERROR expected
if true {
false
//~^ ERROR mismatched types
}
true
//~^ ERROR mismatched types
})
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
error[E0308]: mismatched types
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:9:13
|
LL | / if true {
LL | | false
| | ^^^^^ expected `()`, found `bool`
LL | |
LL | | }
| |_________- expected this to be `()`

error[E0308]: mismatched types
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:12:9
|
LL | true
| ^^^^ expected `Option<()>`, found `bool`
|
= note: expected enum `Option<()>`
found type `bool`

error[E0271]: expected `{closure@dont-ice-for-type-mismatch-in-closure-in-async.rs:6:10}` to be a closure that returns `bool`, but it returns `Option<()>`
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:6:10
|
LL | call(|| -> Option<()> {
| _____----_^
| | |
| | required by a bound introduced by this call
LL | |
LL | | if true {
LL | | false
... |
LL | |
LL | | })
| |_____^ expected `bool`, found `Option<()>`
|
= note: expected type `bool`
found enum `Option<()>`
note: required by a bound in `call`
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:3:25
|
LL | fn call(_: impl Fn() -> bool) {}
| ^^^^ required by this bound in `call`

error: aborting due to 3 previous errors

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

0 comments on commit fdd12c9

Please sign in to comment.