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

Permit `-> _` return types for improved diagnostics #56132

Open
varkor opened this Issue Nov 21, 2018 · 14 comments

Comments

Projects
None yet
10 participants
@varkor
Member

varkor commented Nov 21, 2018

Consider (the invalid Rust code containing) a function that returns a value, but has no return type:

fn foo() {
    0u8
}

fn main() {
    let _: u8 = foo();
}

Currently, the compiler produces these errors:

error[E0308]: mismatched types
 --> src/main.rs:2:5
  |
1 | fn foo() {
  |          - help: try adding a return type: `-> u8`
2 |     0u8
  |     ^^^ expected (), found u8
  |
  = note: expected type `()`
             found type `u8`

error[E0308]: mismatched types
 --> src/main.rs:6:17
  |
6 |     let _: u8 = foo();
  |                 ^^^^^ expected u8, found ()
  |
  = note: expected type `u8`
             found type `()`

error: aborting due to 2 previous errors

In theory, the only problem is the missing return type, which, if provided, would cause the code to compile successfully.

The compiler knows what the return type should be here, and could potentially interpret the rest of the program as if the correct return type were given.

This issue proposes giving special treatment in the compiler for diagnostics only (note that this is not a language change) to the syntax -> _ for return types. This would report a normal error for a missing return type (no extra code would compile after this change), but would otherwise treat the function as having the missing return type. A machine-applicable suggestion would be provided to replace _ with the correct return type.

For example:

fn foo() -> _ {
    0u8
}

fn main() {
    let _: u8 = foo();
}
error[E0121]: the type placeholder `_` is not allowed within types on item signatures
 --> src/main.rs:1:13
  |
1 | fn foo() -> _ {
  |             ^ not allowed in type signatures
  |             - help: replace `_` with the correct return type: `u8`
@varkor

This comment has been minimized.

Member

varkor commented Nov 21, 2018

This change would potentially set precedent for treating specific invalid/incomplete code specially in ways that are not reflected in valid code. In other words:

eddyb: we need to write up something about "guaranteed-error/diagnostic-only" "features", where the compiler does things it shouldn't do in strict terms of the Rust language, knowing it will error anyway, so any power the user gets is only in diagnostics, not in compiled code

This was originally proposed by @eddyb on Discord.

Relevant conversation:

eddyb: if you can convince people to make -> _ a compiler error that contains the type to put in there and provides it to callers (NB dependency cycles disallowed), I'll do it
varkor: I would have thought that -> _ could be given a type-inference based diagnostic approach right now, without an RFC or anything
varkor: it seems like a reasonable diagnostic improvement
eddyb: well, I want someone to confirm that :P

@varkor

This comment has been minimized.

Member

varkor commented Nov 21, 2018

As this potentially sets precedent for diagnostic-motivated behaviour changes, it seems prudent to check that there aren't any concerns with doing this.

@rfcbot merge

@rfcbot

This comment has been minimized.

rfcbot commented Nov 21, 2018

Team member @varkor has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@eddyb

This comment has been minimized.

Member

eddyb commented Nov 21, 2018

Note that the implementation difficulty of this should be negligible, becuase we alrrady have on-demand queries, and we can switch between "check body against signature" and "get signature from body" trivially.

@scottmcm

This comment has been minimized.

Member

scottmcm commented Nov 21, 2018

@rfcbot reviewed Oops, not my team 😆

I support this precedent, and think it obviates things like rust-lang/rfcs#2479

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Nov 21, 2018

Why is -> _ necessary in fn foo() -> _ { 0u8 }?
We can give exactly the same machine-applicable suggestion for fn foo() { 0u8 } without requiring typing extra characters.

@oli-obk

This comment has been minimized.

Contributor

oli-obk commented Nov 21, 2018

The difference is that calls to the function would also know about the return type and not produce additional fallout. Basically return type inference like impl Trait, but transparent and only if an error was reported.

@estebank

This comment has been minimized.

Contributor

estebank commented Nov 21, 2018

@rfcbot concern unneeded

An alternative would be to carry optional inferred type information on fn (and potentially all other) blocks. This way, if we present the above suggestion to change the return type to u8, we could, for the presented above code display something along the lines of:

error[E0308]: mismatched types
 --> src/main.rs:2:5
  |
1 | fn foo() {
  |          - help: try adding a return type: `-> u8`
2 |     0u8
  |     ^^^ expected return type (), found u8
...
6 |     let _: u8 = foo();
  |                 ----- expected u8 here as well
  |
  = note: expected type `()`
             found type `u8`

or just completely elide the error in line 6 by checking against both the resolved type (() here) and the suggested type (u8) if either match, do not emit that diagnostic. We already do something along these lines on blocks that have either parse or type errors, by marking them as TyErr and not emitting mismatched types errors that involve TyErr (as they have already being emitted in the source of the problem).

@leonardo-m

This comment has been minimized.

leonardo-m commented Nov 21, 2018

I think in the Haskell world for something like this they invent a pretty name (something like Return Type Holes) and write a paper on it.

@eddyb

This comment has been minimized.

Member

eddyb commented Nov 21, 2018

@estebank That's not actionable because it breaks with cycles (think recursive function) - and we don't want to add recovery from dependency cycles.

The way it works with -> _ is that it's a switch for flipping a dependency edge in the compiler.

@estebank

This comment has been minimized.

Contributor

estebank commented Nov 21, 2018

Couldn't this (changing -> () to -> _) be done implicitly by performing that change once the suggestion is emitted?

@eddyb

This comment has been minimized.

Member

eddyb commented Nov 21, 2018

@estebank No, that sort of side-effect is incompatible with the incremental dataflow ("query") model.

@Centril

This comment has been minimized.

Contributor

Centril commented Nov 22, 2018

@scottmcm

I support this precedent, and think it obviates things like rust-lang/rfcs#2479

Obviously I disagree that it obviates that change; this will still error, not letting you finish cargo check nor cargo test so the experience is not the same.

That said I fully support this change.

@FrankSD

This comment has been minimized.

FrankSD commented Dec 9, 2018

Is this issue already taken? Please confirm I would like to take this on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment