Skip to content

Commit

Permalink
Rollup merge of #116296 - compiler-errors:default-return, r=estebank
Browse files Browse the repository at this point in the history
More accurately point to where default return type should go

When getting the "default return type" span, instead of pointing to the low span of the next token, point to the high span of the previous token. This:

1. Makes forming return type suggestions more uniform, since we expect them all in the same place.
2. Arguably makes labels easier to understand, since we're pointing to where the implicit `-> ()` would've gone, rather than the starting brace or the semicolon.

r? ```@estebank```
  • Loading branch information
workingjubilee authored Oct 5, 2023
2 parents ea3454e + dd5f26c commit cfce3a9
Show file tree
Hide file tree
Showing 42 changed files with 105 additions and 89 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ fn report_trait_method_mismatch<'tcx>(
let ap = Applicability::MachineApplicable;
match sig.decl.output {
hir::FnRetTy::DefaultReturn(sp) => {
let sugg = format!("-> {} ", trait_sig.output());
let sugg = format!(" -> {}", trait_sig.output());
diag.span_suggestion_verbose(sp, msg, sugg, ap);
}
hir::FnRetTy::Return(hir_ty) => {
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_hir_typeck/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ pub(super) fn check_fn<'a, 'tcx>(

fcx.typeck_results.borrow_mut().liberated_fn_sigs_mut().insert(fn_id, fn_sig);

fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
let return_or_body_span = match decl.output {
hir::FnRetTy::DefaultReturn(_) => body.value.span,
hir::FnRetTy::Return(ty) => ty.span,
};
fcx.require_type_is_sized(declared_ret_ty, return_or_body_span, traits::SizedReturnType);
fcx.check_return_expr(&body.value, false);

// We insert the deferred_generator_interiors entry after visiting the body.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub struct AddressOfTemporaryTaken {
pub enum AddReturnTypeSuggestion {
#[suggestion(
hir_typeck_add_return_type_add,
code = "-> {found} ",
code = " -> {found}",
applicability = "machine-applicable"
)]
Add {
Expand All @@ -120,7 +120,7 @@ pub enum AddReturnTypeSuggestion {
},
#[suggestion(
hir_typeck_add_return_type_missing_here,
code = "-> _ ",
code = " -> _",
applicability = "has-placeholders"
)]
MissingHere {
Expand Down
38 changes: 18 additions & 20 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,8 +782,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
hir::FnRetTy::Return(hir_ty) => {
let span = hir_ty.span;

if let hir::TyKind::OpaqueDef(item_id, ..) = hir_ty.kind
&& let hir::Node::Item(hir::Item {
kind: hir::ItemKind::OpaqueTy(op_ty),
Expand All @@ -799,28 +797,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
debug!(?found);
if found.is_suggestable(self.tcx, false) {
if term.span.is_empty() {
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add { span, found: found.to_string() });
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add { span: term.span, found: found.to_string() });
return true;
} else {
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Other { span, expected });
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Other { span: term.span, expected });
}
}
}

// Only point to return type if the expected type is the return type, as if they
// are not, the expectation must have been caused by something else.
debug!("return type {:?}", hir_ty);
let ty = self.astconv().ast_ty_to_ty(hir_ty);
debug!("return type {:?}", ty);
debug!("expected type {:?}", expected);
let bound_vars = self.tcx.late_bound_vars(hir_ty.hir_id.owner.into());
let ty = Binder::bind_with_vars(ty, bound_vars);
let ty = self.normalize(span, ty);
let ty = self.tcx.erase_late_bound_regions(ty);
if self.can_coerce(expected, ty) {
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Other { span, expected });
self.try_suggest_return_impl_trait(err, expected, ty, fn_id);
return true;
} else {
// Only point to return type if the expected type is the return type, as if they
// are not, the expectation must have been caused by something else.
debug!("return type {:?}", hir_ty);
let ty = self.astconv().ast_ty_to_ty(hir_ty);
debug!("return type {:?}", ty);
debug!("expected type {:?}", expected);
let bound_vars = self.tcx.late_bound_vars(hir_ty.hir_id.owner.into());
let ty = Binder::bind_with_vars(ty, bound_vars);
let ty = self.normalize(hir_ty.span, ty);
let ty = self.tcx.erase_late_bound_regions(ty);
if self.can_coerce(expected, ty) {
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Other { span: hir_ty.span, expected });
self.try_suggest_return_impl_trait(err, expected, ty, fn_id);
return true;
}
}
}
_ => {}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_infer/src/errors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,13 @@ impl<'a> SourceKindMultiSuggestion<'a> {
data: &'a FnRetTy<'a>,
should_wrap_expr: Option<Span>,
) -> Self {
let (arrow, post) = match data {
FnRetTy::DefaultReturn(_) => ("-> ", " "),
_ => ("", ""),
let arrow = match data {
FnRetTy::DefaultReturn(_) => " -> ",
_ => "",
};
let (start_span, start_span_code, end_span) = match should_wrap_expr {
Some(end_span) => (data.span(), format!("{arrow}{ty_info}{post}{{ "), Some(end_span)),
None => (data.span(), format!("{arrow}{ty_info}{post}"), None),
Some(end_span) => (data.span(), format!("{arrow}{ty_info} {{"), Some(end_span)),
None => (data.span(), format!("{arrow}{ty_info}"), None),
};
Self::ClosureReturn { start_span, start_span_code, end_span }
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl<'a> Parser<'a> {
)?;
FnRetTy::Ty(ty)
} else {
FnRetTy::Default(self.token.span.shrink_to_lo())
FnRetTy::Default(self.prev_token.span.shrink_to_hi())
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,10 @@ pub(super) fn check(
]),
("None", "unwrap_or_else", _) => match args[0].kind {
hir::ExprKind::Closure(hir::Closure {
fn_decl:
hir::FnDecl {
output: hir::FnRetTy::DefaultReturn(span) | hir::FnRetTy::Return(hir::Ty { span, .. }),
..
},
body,
..
}) => Some(vec![
(expr.span.with_hi(span.hi()), String::new()),
(expr.span.with_hi(cx.tcx.hir().body(*body).value.span.lo()), String::new()),
(expr.span.with_lo(args[0].span.hi()), String::new()),
]),
_ => None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: this function has too many arguments (11/10)
--> $DIR/too_many_arguments.rs:4:1
|
LL | fn too_many(p1: u8, p2: u8, p3: u8, p4: u8, p5: u8, p6: u8, p7: u8, p8: u8, p9: u8, p10: u8, p11: u8) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::too-many-arguments` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::too_many_arguments)]`
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/tests/ui/functions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: this function has too many arguments (8/7)
--> $DIR/functions.rs:8:1
|
LL | fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::too-many-arguments` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::too_many_arguments)]`
Expand All @@ -17,7 +17,7 @@ LL | | two: u32,
... |
LL | | eight: ()
LL | | ) {
| |__^
| |_^

error: this function has too many arguments (8/7)
--> $DIR/functions.rs:48:5
Expand All @@ -29,7 +29,7 @@ error: this function has too many arguments (8/7)
--> $DIR/functions.rs:58:5
|
LL | fn bad_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: this public function might dereference a raw pointer but is not marked `unsafe`
--> $DIR/functions.rs:68:34
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/tests/ui/must_use_unit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: this unit-returning function has a `#[must_use]` attribute
LL | #[must_use]
| ----------- help: remove the attribute
LL | pub fn must_use_default() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::must-use-unit` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::must_use_unit)]`
Expand All @@ -23,7 +23,7 @@ error: this unit-returning function has a `#[must_use]` attribute
LL | #[must_use = "With note"]
| ------------------------- help: remove the attribute
LL | pub fn must_use_with_note() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors

4 changes: 2 additions & 2 deletions src/tools/clippy/tests/ui/unnecessary_literal_unwrap.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ fn unwrap_option_none() {
let _val: u16 = 234;
let _val: u16 = 234;
let _val: u16 = { 234 };
let _val: u16 = { 234 };
let _val: u16 = { 234 };

panic!();
panic!("this always happens");
String::default();
234;
234;
{ 234 };
{ 234 };
{ 234 };
}

fn unwrap_result_ok() {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/tests/ui/unnecessary_literal_unwrap.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ LL | let _val: u16 = None.unwrap_or_else(|| -> u16 { 234 });
help: remove the `None` and `unwrap_or_else()`
|
LL - let _val: u16 = None.unwrap_or_else(|| -> u16 { 234 });
LL + let _val: u16 = { 234 };
LL + let _val: u16 = { 234 };
|

error: used `unwrap()` on `None` value
Expand Down Expand Up @@ -187,7 +187,7 @@ LL | None::<u16>.unwrap_or_else(|| -> u16 { 234 });
help: remove the `None` and `unwrap_or_else()`
|
LL - None::<u16>.unwrap_or_else(|| -> u16 { 234 });
LL + { 234 };
LL + { 234 };
|

error: used `unwrap()` on `Ok` value
Expand Down
3 changes: 2 additions & 1 deletion src/tools/rustfmt/src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2599,7 +2599,8 @@ fn rewrite_fn_base(
if where_clause_str.is_empty() {
if let ast::FnRetTy::Default(ret_span) = fd.output {
match recover_missing_comment_in_span(
mk_sp(params_span.hi(), ret_span.hi()),
// from after the closing paren to right before block or semicolon
mk_sp(ret_span.lo(), span.hi()),
shape,
context,
last_line_width(&result),
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/associated-type-bounds/issue-71443-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0308]: mismatched types
--> $DIR/issue-71443-1.rs:6:5
|
LL | fn hello<F: for<'a> Iterator<Item: 'a>>() {
| - help: try adding a return type: `-> Incorrect`
| - help: try adding a return type: `-> Incorrect`
LL | Incorrect
| ^^^^^^^^^ expected `()`, found `Incorrect`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0308]: mismatched types
--> $DIR/block-must-not-have-result-res.rs:5:9
|
LL | fn drop(&mut self) {
| - expected `()` because of default return type
| - expected `()` because of default return type
LL | true
| ^^^^ expected `()`, found `bool`

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/block-result/issue-20862.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0308]: mismatched types
--> $DIR/issue-20862.rs:2:5
|
LL | fn foo(x: i32) {
| - help: a return type might be missing here: `-> _`
| - help: a return type might be missing here: `-> _`
LL | |y| x + y
| ^^^^^^^^^ expected `()`, found closure
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/block-result/issue-22645.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ error[E0308]: mismatched types
--> $DIR/issue-22645.rs:15:3
|
LL | fn main() {
| - expected `()` because of default return type
| - expected `()` because of default return type
LL | let b = Bob + 3.5;
LL | b + 3
| ^^^^^ expected `()`, found `Bob`
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/block-result/issue-5500.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0308]: mismatched types
--> $DIR/issue-5500.rs:2:5
|
LL | fn main() {
| - expected `()` because of default return type
| - expected `()` because of default return type
LL | &panic!()
| ^^^^^^^^^ expected `()`, found `&_`
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/closures/add_semicolon_non_block_closure.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0308]: mismatched types
--> $DIR/add_semicolon_non_block_closure.rs:8:12
|
LL | fn main() {
| - expected `()` because of default return type
| - expected `()` because of default return type
LL | foo(|| bar())
| ^^^^^ expected `()`, found `i32`
|
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/closures/binder/implicit-return.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: implicit types in closure signatures are forbidden when `for<...>` is present
--> $DIR/implicit-return.rs:4:34
--> $DIR/implicit-return.rs:4:33
|
LL | let _f = for<'a> |_: &'a ()| {};
| ------- ^
| ------- ^
| |
| `for<...>` is here

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/closures/binder/implicit-stuff.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ LL | let _ = for<'a> |x: &'a ()| -> &() { x };
| ^ explicit lifetime name needed here

error: implicit types in closure signatures are forbidden when `for<...>` is present
--> $DIR/implicit-stuff.rs:5:22
--> $DIR/implicit-stuff.rs:5:21
|
LL | let _ = for<> || {};
| ----- ^
| ----- ^
| |
| `for<...>` is here

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/codemap_tests/tab.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ error[E0308]: mismatched types
--> $DIR/tab.rs:8:2
|
LL | fn foo() {
| - help: try adding a return type: `-> &'static str`
| - help: try adding a return type: `-> &'static str`
LL | "bar boo"
| ^^^^^^^^^^^^^^^^^^^^ expected `()`, found `&str`

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/compare-method/bad-self-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ LL | fn foo(self);
found signature `fn(Box<MyFuture>)`

error[E0053]: method `bar` has an incompatible type for trait
--> $DIR/bad-self-type.rs:24:18
--> $DIR/bad-self-type.rs:24:17
|
LL | fn bar(self) {}
| ^ expected `Option<()>`, found `()`
| ^ expected `Option<()>`, found `()`
|
note: type in trait
--> $DIR/bad-self-type.rs:18:21
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/impl-trait/in-trait/refine.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ LL | fn bar() {}
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
help: replace the return type so that it matches the trait
|
LL | fn bar() -> impl Sized {}
| +++++++++++++
LL | fn bar()-> impl Sized {}
| +++++++++++++

error: impl trait in impl method signature does not match trait method signature
--> $DIR/refine.rs:22:17
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/issues/issue-66667-function-cmp-cycle.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ error[E0308]: mismatched types
--> $DIR/issue-66667-function-cmp-cycle.rs:2:5
|
LL | fn first() {
| - help: try adding a return type: `-> bool`
| - help: try adding a return type: `-> bool`
LL | second == 1
| ^^^^^^^^^^^ expected `()`, found `bool`

Expand All @@ -44,7 +44,7 @@ error[E0308]: mismatched types
--> $DIR/issue-66667-function-cmp-cycle.rs:8:5
|
LL | fn second() {
| - help: try adding a return type: `-> bool`
| - help: try adding a return type: `-> bool`
LL | first == 1
| ^^^^^^^^^^ expected `()`, found `bool`

Expand All @@ -69,7 +69,7 @@ error[E0308]: mismatched types
--> $DIR/issue-66667-function-cmp-cycle.rs:14:5
|
LL | fn bar() {
| - help: try adding a return type: `-> bool`
| - help: try adding a return type: `-> bool`
LL | bar == 1
| ^^^^^^^^ expected `()`, found `bool`

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/lang-items/start_lang_item_args.missing_ret.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0308]: lang item `start` function has wrong type
--> $DIR/start_lang_item_args.rs:29:84
--> $DIR/start_lang_item_args.rs:29:83
|
LL | fn start<T>(_main: fn() -> T, _argc: isize, _argv: *const *const u8, _sigpipe: u8) {}
| ^ expected `isize`, found `()`
| ^ expected `isize`, found `()`
|
= note: expected signature `fn(fn() -> _, _, _, _) -> isize`
found signature `fn(fn() -> _, _, _, _)`
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/loops/loop-break-value.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ error[E0308]: mismatched types
--> $DIR/loop-break-value.rs:159:15
|
LL | fn main() {
| - expected `()` because of this return type
| - expected `()` because of this return type
...
LL | loop { // point at the return type
| ---- this loop is expected to be of type `()`
Expand Down
Loading

0 comments on commit cfce3a9

Please sign in to comment.