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

When encountering unexpected closure return type, point at return type/expression #132156

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
}))),
terr,
false,
None,
);
return Err(diag.emit());
}
Expand Down Expand Up @@ -1056,6 +1057,7 @@ fn report_trait_method_mismatch<'tcx>(
}))),
terr,
false,
None,
);

diag.emit()
Expand Down Expand Up @@ -1853,6 +1855,7 @@ fn compare_const_predicate_entailment<'tcx>(
}))),
terr,
false,
None,
);
return Err(diag.emit());
};
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ pub fn check_function_signature<'tcx>(
}))),
err,
false,
None,
);
return Err(diag.emit());
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(self.param_env.and(trace.values)),
e,
true,
None,
);
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2343,6 +2343,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}))),
terr,
false,
None,
);
diag.emit();
self.abort.set(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1148,9 +1148,13 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
mut values: Option<ty::ParamEnvAnd<'tcx, ValuePairs<'tcx>>>,
terr: TypeError<'tcx>,
prefer_label: bool,
override_span: Option<Span>,
) {
let span = cause.span;

// We use `override_span` when we want the error to point at a `Span` other than
// `cause.span`. This is used in E0271, when a closure is passed in where the return type
// isn't what was expected. We want to point at the closure's return type (or expression),
// instead of the expression where the closure is passed as call argument.
let span = override_span.unwrap_or(cause.span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let span = override_span.unwrap_or(cause.span);
let span = override_span.unwrap_or(cause.span());

Turns out that .span and .span() do different things! This is kinda what I meant when I said that these APIs are unmaintainable today.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should fix the regression in spans, afaict.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#132243 removes the need for that pointless/confusing method.

// For some types of errors, expected-found does not make
// sense, so just ignore the values we were given.
if let TypeError::CyclicTy(_) = terr {
Expand Down Expand Up @@ -1807,6 +1811,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
Some(param_env.and(trace.values)),
terr,
false,
None,
);
diag
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
None,
TypeError::Sorts(ty::error::ExpectedFound::new(true, expected_ty, ct_ty)),
false,
None,
);
diag
}
Expand Down Expand Up @@ -910,12 +911,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}
}
let hir_id = self.tcx.local_def_id_to_hir_id(obligation.cause.body_id);
let body_id = match self.tcx.hir_node(hir_id) {
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, _, body_id), .. }) => body_id,
_ => return false,
};
let ControlFlow::Break(expr) = (FindMethodSubexprOfTry { search_span: span })
.visit_body(self.tcx.hir().body(*body_id))
let Some(body_id) = self.tcx.hir_node(hir_id).body_id() else { return false };
let ControlFlow::Break(expr) =
(FindMethodSubexprOfTry { search_span: span }).visit_body(self.tcx.hir().body(body_id))
else {
return false;
};
Expand Down Expand Up @@ -1366,22 +1364,54 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
_ => (None, error.err),
};

let msg = values
let (msg, span, closure_span) = values
.and_then(|(predicate, normalized_term, expected_term)| {
self.maybe_detailed_projection_msg(predicate, normalized_term, expected_term)
self.maybe_detailed_projection_msg(
obligation.cause.span,
predicate,
normalized_term,
expected_term,
)
})
.unwrap_or_else(|| {
let mut cx = FmtPrinter::new_with_limit(
self.tcx,
Namespace::TypeNS,
rustc_session::Limit(10),
);
with_forced_trimmed_paths!(format!("type mismatch resolving `{}`", {
self.resolve_vars_if_possible(predicate).print(&mut cx).unwrap();
cx.into_buffer()
}))
(
with_forced_trimmed_paths!(format!("type mismatch resolving `{}`", {
self.resolve_vars_if_possible(predicate).print(&mut cx).unwrap();
cx.into_buffer()
})),
obligation.cause.span,
None,
)
});
let mut diag = struct_span_code_err!(self.dcx(), obligation.cause.span, E0271, "{msg}");
let mut diag = struct_span_code_err!(self.dcx(), span, E0271, "{msg}");
if let Some(span) = closure_span {
// Mark the closure decl so that it is seen even if we are pointing at the return
// type or expression.
//
// error[E0271]: expected `{closure@foo.rs:41:16}` to be a closure that returns
// `Unit3`, but it returns `Unit4`
// --> $DIR/foo.rs:43:17
// |
// LL | let v = Unit2.m(
// | - required by a bound introduced by this call
// ...
// LL | f: |x| {
// | --- /* this span */
// LL | drop(x);
// LL | Unit4
// | ^^^^^ expected `Unit3`, found `Unit4`
// |
diag.span_label(span, "this closure");
if !span.overlaps(obligation.cause.span) {
// Point at the binding corresponding to the closure where it is used.
diag.span_label(obligation.cause.span, "closure used here");
}
}

let secondary_span = (|| {
let ty::PredicateKind::Clause(ty::ClauseKind::Projection(proj)) =
Expand Down Expand Up @@ -1453,6 +1483,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}),
err,
false,
Some(span),
);
self.note_obligation_cause(&mut diag, obligation);
diag.emit()
Expand All @@ -1461,34 +1492,66 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {

fn maybe_detailed_projection_msg(
&self,
mut span: Span,
projection_term: ty::AliasTerm<'tcx>,
normalized_ty: ty::Term<'tcx>,
expected_ty: ty::Term<'tcx>,
) -> Option<String> {
) -> Option<(String, Span, Option<Span>)> {
let trait_def_id = projection_term.trait_def_id(self.tcx);
let self_ty = projection_term.self_ty();

with_forced_trimmed_paths! {
if self.tcx.is_lang_item(projection_term.def_id, LangItem::FnOnceOutput) {
let fn_kind = self_ty.prefix_string(self.tcx);
let (span, closure_span) = if let ty::Closure(def_id, _) = self_ty.kind() {
let def_span = self.tcx.def_span(def_id);
if let Some(local_def_id) = def_id.as_local()
&& let node = self.tcx.hir_node_by_def_id(local_def_id)
&& let Some(fn_decl) = node.fn_decl()
&& let Some(id) = node.body_id()
{
span = match fn_decl.output {
hir::FnRetTy::Return(ty) => ty.span,
hir::FnRetTy::DefaultReturn(_) => {
let body = self.tcx.hir().body(id);
match body.value.kind {
hir::ExprKind::Block(
hir::Block { expr: Some(expr), .. },
_,
) => expr.span,
hir::ExprKind::Block(
hir::Block {
expr: None, stmts: [.., last], ..
},
_,
) => last.span,
_ => body.value.span,
}
}
};
}
(span, Some(def_span))
} else {
(span, None)
};
let item = match self_ty.kind() {
ty::FnDef(def, _) => self.tcx.item_name(*def).to_string(),
_ => self_ty.to_string(),
};
Some(format!(
Some((format!(
"expected `{item}` to be a {fn_kind} that returns `{expected_ty}`, but it \
returns `{normalized_ty}`",
))
), span, closure_span))
} else if self.tcx.is_lang_item(trait_def_id, LangItem::Future) {
Some(format!(
Some((format!(
"expected `{self_ty}` to be a future that resolves to `{expected_ty}`, but it \
resolves to `{normalized_ty}`"
))
), span, None))
} else if Some(trait_def_id) == self.tcx.get_diagnostic_item(sym::Iterator) {
Some(format!(
Some((format!(
"expected `{self_ty}` to be an iterator that yields `{expected_ty}`, but it \
yields `{normalized_ty}`"
))
), span, None))
} else {
None
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,14 @@ LL | true
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
--> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:6:16
|
LL | call(|| -> Option<()> {
| _____----_^
| | |
| | required by a bound introduced by this call
LL | |
LL | | if true {
LL | | false
... |
LL | |
LL | | })
| |_____^ expected `bool`, found `Option<()>`
LL | call(|| -> Option<()> {
| ---- ------^^^^^^^^^^
| | | |
| | | expected `bool`, found `Option<()>`
| | this closure
| required by a bound introduced by this call
|
= note: expected type `bool`
found enum `Option<()>`
Expand Down
25 changes: 25 additions & 0 deletions tests/ui/closures/return-type-doesnt-match-bound.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use std::error::Error;
use std::process::exit;

fn foo<F>(f: F) -> ()
where
F: FnOnce() -> Result<(), Box<dyn Error>>,
{
f().or_else(|e| -> ! { //~ ERROR to be a closure that returns
eprintln!("{:?}", e);
exit(1)
});
}

fn bar<F>(f: F) -> ()
where
F: FnOnce() -> Result<(), Box<dyn Error>>,
{
let c = |e| -> ! { //~ ERROR to be a closure that returns
eprintln!("{:?}", e);
exit(1)
};
f().or_else(c);
}

fn main() {}
37 changes: 37 additions & 0 deletions tests/ui/closures/return-type-doesnt-match-bound.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
error[E0271]: expected `{closure@return-type-doesnt-match-bound.rs:8:17}` to be a closure that returns `Result<(), _>`, but it returns `!`
--> $DIR/return-type-doesnt-match-bound.rs:8:24
|
LL | f().or_else(|e| -> ! {
| ------- -------^
| | | |
| | | expected `Result<(), _>`, found `!`
| | this closure
| required by a bound introduced by this call
|
= note: expected enum `Result<(), _>`
found type `!`
note: required by a bound in `Result::<T, E>::or_else`
--> $SRC_DIR/core/src/result.rs:LL:COL

error[E0271]: expected `{closure@return-type-doesnt-match-bound.rs:18:13}` to be a closure that returns `Result<(), _>`, but it returns `!`
--> $DIR/return-type-doesnt-match-bound.rs:18:20
|
LL | let c = |e| -> ! {
| -------^
| | |
| | expected `Result<(), _>`, found `!`
| this closure
...
LL | f().or_else(c);
| ------- - closure used here
| |
| required by a bound introduced by this call
|
= note: expected enum `Result<(), _>`
found type `!`
note: required by a bound in `Result::<T, E>::or_else`
--> $SRC_DIR/core/src/result.rs:LL:COL

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0271`.
6 changes: 2 additions & 4 deletions tests/ui/higher-ranked/trait-bounds/issue-62203-hrtb-ice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,10 @@ trait Ty<'a> {

fn main() {
let v = Unit2.m(
L {
//~^ ERROR to be a closure that returns `Unit3`, but it returns `Unit4`
//~| ERROR type mismatch
L { //~ ERROR type mismatch
f: |x| {
drop(x);
Unit4
Unit4 //~ ERROR to be a closure that returns `Unit3`, but it returns `Unit4`
},
},
);
Expand Down
Loading
Loading