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

Make E0121's suggestion more robust (+ fix E0308's suggestion) #80847

Closed
wants to merge 2 commits into from
Closed
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
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
match cause.code {
ObligationCauseCode::Pattern { origin_expr: true, span: Some(span), root_ty } => {
let ty = self.resolve_vars_if_possible(root_ty);
if ty.is_suggestable() {
if ty.is_suggestable(self.tcx) {
// don't show type `_`
err.span_label(span, format!("this expression has type `{}`", ty));
}
Expand Down
53 changes: 41 additions & 12 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
//! Diagnostics related methods for `TyS`.

use crate::ty::subst::SubstsRef;
use crate::ty::TyKind::*;
use crate::ty::{InferTy, TyCtxt, TyS};
use crate::ty::{FnSig, InferTy, PolyFnSig, TyCtxt, TyS, VariantDef};
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -61,17 +62,45 @@ impl<'tcx> TyS<'tcx> {
}

/// Whether the type can be safely suggested during error recovery.
pub fn is_suggestable(&self) -> bool {
!matches!(
self.kind(),
Opaque(..)
| FnDef(..)
| FnPtr(..)
| Dynamic(..)
| Closure(..)
| Infer(..)
| Projection(..)
)
pub fn is_suggestable(&self, tcx: TyCtxt<'tcx>) -> bool {
debug!("is_suggestable: checking if {:?} is suggestable", self.kind());
match self.kind() {
Opaque(..) | FnDef(..) | Dynamic(..) | Closure(..) | Infer(..) | Projection(..)
| Generator(..) | GeneratorWitness(..) | Error(..) => false,

Adt(def, substs) => {
def.variants.iter().all(|variants| variants.is_suggestable(tcx, substs))
}

Array(ty, _) | Slice(ty) => ty.is_suggestable(tcx),
FnPtr(sig) => sig.is_suggestable(tcx),
RawPtr(ty_and_mut) => ty_and_mut.ty.is_suggestable(tcx),
Ref(_, ty, _) => ty.is_suggestable(tcx),
Tuple(_) => self.tuple_fields().all(|ty| ty.is_suggestable(tcx)),

_ => true,
}
}
}

impl<'tcx> FnSig<'tcx> {
/// Whether this function signature can be safely suggested during error recovery.
pub fn is_suggestable(&self, tcx: TyCtxt<'tcx>) -> bool {
self.inputs_and_output.iter().all(|ty| ty.is_suggestable(tcx))
}
}

impl<'tcx> PolyFnSig<'tcx> {
/// Whether this function signature can be safely suggested during error recovery.
pub fn is_suggestable(&self, tcx: TyCtxt<'tcx>) -> bool {
self.as_ref().skip_binder().is_suggestable(tcx)
}
}

impl VariantDef {
/// Whether this variant can be safely suggested during error recovery.
pub fn is_suggestable<'tcx>(&self, tcx: TyCtxt<'tcx>, substs: SubstsRef<'tcx>) -> bool {
self.fields.iter().all(|field| field.ty(tcx, substs).is_suggestable(tcx))
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/astconv/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let param_hir_id = tcx.hir().local_def_id_to_hir_id(param_local_id);
let param_name = tcx.hir().ty_param_name(param_hir_id);
let param_type = tcx.type_of(param.def_id);
if param_type.is_suggestable() {
if param_type.is_suggestable(tcx) {
err.span_suggestion(
tcx.def_span(src_def_id),
"consider changing this type paramater to a `const`-generic",
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> bool {
// Only suggest changing the return type for methods that
// haven't set a return type at all (and aren't `fn main()` or an impl).
match (&fn_decl.output, found.is_suggestable(), can_suggest, expected.is_unit()) {
debug!("suggest_missing_return_type: about to determine if {:?} is suggestable", found);
match (&fn_decl.output, found.is_suggestable(self.tcx), can_suggest, expected.is_unit()) {
(&hir::FnRetTy::DefaultReturn(span), true, true, true) => {
err.span_suggestion(
span,
Expand Down
55 changes: 35 additions & 20 deletions compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1663,26 +1663,41 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
let mut diag = bad_placeholder_type(tcx, visitor.0);
let ret_ty = fn_sig.output();
if ret_ty != tcx.ty_error() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this check can now be removed thanks to is_suggestable accounting for Error.

if !ret_ty.is_closure() {
let ret_ty_str = match ret_ty.kind() {
// Suggest a function pointer return type instead of a unique function definition
// (e.g. `fn() -> i32` instead of `fn() -> i32 { f }`, the latter of which is invalid
// syntax)
ty::FnDef(..) => ret_ty.fn_sig(tcx).to_string(),
_ => ret_ty.to_string(),
};
diag.span_suggestion(
ty.span,
"replace with the correct return type",
ret_ty_str,
Applicability::MaybeIncorrect,
);
} else {
// We're dealing with a closure, so we should suggest using `impl Fn` or trait bounds
// to prevent the user from getting a papercut while trying to use the unique closure
// syntax (e.g. `[closure@src/lib.rs:2:5: 2:9]`).
diag.help("consider using an `Fn`, `FnMut`, or `FnOnce` trait bound");
diag.note("for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html");
match ret_ty.kind() {
ty::FnDef(..) if ret_ty.fn_sig(tcx).is_suggestable(tcx) => {
diag.span_suggestion(
ty.span,
"replace with the correct return type",
ret_ty.fn_sig(tcx).to_string(),
Applicability::MaybeIncorrect,
);
}

ty::Closure(..) => {
diag.help(
"consider using an `Fn`, `FnMut`, or `FnOnce` trait bound",
);
diag.note("for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html");
}

ty::Generator(..) if tcx.features().generators => {
diag.help("consider using a `Generator` trait bound");
// FIXME: link elsewhere if/when generators are stabilized
diag.note("for more information on generators, see https://doc.rust-lang.org/beta/unstable-book/language-features/generators.html");
}

_ if ret_ty.is_suggestable(tcx) => {
diag.span_suggestion(
ty.span,
"replace with the correct return type",
ret_ty.to_string(),
// FIXME: should this be MachineApplicable?
// (cf. `rustc_typeck::check::fn_ctxt::suggestions::suggest_missing_return_type()`)
Applicability::MaybeIncorrect,
);
}

_ => (),
}
}
diag.emit();
Expand Down
51 changes: 51 additions & 0 deletions src/test/ui/fn/issue-80844-e0121.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#![feature(generators)]

// Functions with a type placeholder `_` as the return type should
// not suggest returning the unnameable type of generators.
// This is a regression test of #80844

struct Container<T>(T);

fn returns_generator() -> _ {
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures [E0121]
//~| NOTE not allowed in type signatures
//~| HELP consider using a `Generator` trait bound
//~| NOTE for more information on generators
|| yield 0i32
}

fn returns_returns_generator() -> _ {
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures [E0121]
//~| NOTE not allowed in type signatures
returns_generator
}

fn returns_option_closure() -> _ {
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures [E0121]
//~| NOTE not allowed in type signatures
Some(|| 0i32)
}

fn returns_option_i32() -> _ {
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures [E0121]
//~| NOTE not allowed in type signatures
//~| HELP replace with the correct return type
//~| SUGGESTION Option<i32>
Some(0i32)
}

fn returns_container_closure() -> _ {
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures [E0121]
//~| NOTE not allowed in type signatures
Container(|| 0i32)
}

fn returns_container_i32() -> _ {
//~^ ERROR the type placeholder `_` is not allowed within types on item signatures [E0121]
//~| NOTE not allowed in type signatures
//~| HELP replace with the correct return type
//~| SUGGESTION Container<i32>
Container(0i32)
}

fn main() {}
48 changes: 48 additions & 0 deletions src/test/ui/fn/issue-80844-e0121.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
--> $DIR/issue-80844-e0121.rs:9:27
|
LL | fn returns_generator() -> _ {
| ^ not allowed in type signatures
|
= help: consider using a `Generator` trait bound
= note: for more information on generators, see https://doc.rust-lang.org/beta/unstable-book/language-features/generators.html

error[E0121]: the type placeholder `_` is not allowed within types on item signatures
--> $DIR/issue-80844-e0121.rs:17:35
|
LL | fn returns_returns_generator() -> _ {
| ^ not allowed in type signatures

error[E0121]: the type placeholder `_` is not allowed within types on item signatures
--> $DIR/issue-80844-e0121.rs:23:32
|
LL | fn returns_option_closure() -> _ {
| ^ not allowed in type signatures

error[E0121]: the type placeholder `_` is not allowed within types on item signatures
--> $DIR/issue-80844-e0121.rs:29:28
|
LL | fn returns_option_i32() -> _ {
| ^
| |
| not allowed in type signatures
| help: replace with the correct return type: `Option<i32>`

error[E0121]: the type placeholder `_` is not allowed within types on item signatures
--> $DIR/issue-80844-e0121.rs:37:35
|
LL | fn returns_container_closure() -> _ {
| ^ not allowed in type signatures

error[E0121]: the type placeholder `_` is not allowed within types on item signatures
--> $DIR/issue-80844-e0121.rs:43:31
|
LL | fn returns_container_i32() -> _ {
| ^
| |
| not allowed in type signatures
| help: replace with the correct return type: `Container<i32>`

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0121`.
61 changes: 61 additions & 0 deletions src/test/ui/fn/issue-80844-e0308.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#![feature(generators)]

// Functions with a type placeholder `_` as the return type should
// not suggest returning the unnameable type of generators.
// This is a regression test of #80844

struct Container<T>(T);

fn expected_unit_got_generator() {
//~^ NOTE possibly return type missing here?
|| yield 0i32
//~^ ERROR mismatched types [E0308]
//~| NOTE expected unit type `()`
//~| NOTE expected `()`, found generator
}

fn expected_unit_got_closure() {
//~^ NOTE possibly return type missing here?
|| 0i32
//~^ ERROR mismatched types [E0308]
//~| NOTE expected unit type `()`
//~| NOTE expected `()`, found closure
}

fn expected_unit_got_option_closure() {
//~^ NOTE possibly return type missing here?
Some(|| 0i32)
//~^ ERROR mismatched types [E0308]
//~| NOTE expected unit type `()`
//~| NOTE expected `()`, found enum `Option`
//~| HELP try adding a semicolon
}

fn expected_unit_got_option_i32() {
//~^ NOTE possibly return type missing here?
Some(0i32)
//~^ ERROR mismatched types [E0308]
//~| NOTE expected unit type `()`
//~| NOTE expected `()`, found enum `Option`
//~| HELP try adding a semicolon
}

fn expected_unit_got_container_closure() {
//~^ NOTE possibly return type missing here?
Container(|| 0i32)
//~^ ERROR mismatched types [E0308]
//~| NOTE expected unit type `()`
//~| NOTE expected `()`, found struct `Container`
//~| HELP try adding a semicolon
}

fn expected_unit_got_container_i32() {
//~^ NOTE possibly return type missing here?
Container(0i32)
//~^ ERROR mismatched types [E0308]
//~| NOTE expected unit type `()`
//~| NOTE expected `()`, found struct `Container`
//~| HELP try adding a semicolon
}

fn main() {}
Loading