Recover invalid return type until {, } or ; - #163059
inkreasing wants to merge 4 commits into
Conversation
|
The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease |
rustbot
added
S-waiting-on-review
| { | ||
| self.bump(); | ||
| } | ||
| return Ok(FnRetTy::Default(self.prev_token.span)); |
There was a problem hiding this comment.
unsure about this span.
maybe this should be the complete return type that was skipped?
I don't think i can do anything other than make the return type (). It can lead to wrong type mismatches.
There was a problem hiding this comment.
Oh i should probably make the type an err.
edit: done. still unsure about the span.
| | | ||
| LL - fn foo<T>() where T: Default -> { | ||
| LL + fn foo<T>() -> where T: Default { | ||
| | |
There was a problem hiding this comment.
very unsure why this calls the where bounds function parameters?
Otherwise the suggestion isn't too bad.
There was a problem hiding this comment.
This suggestion now triggers since you've updated parse_ret_ty to return Ok(_) in this case and so in error_fn_body_not_found the code believes it can use the parsed return type to offer the suggestion "since everything went well". Previously, it noped outta there as it used to be an Err(_).
There was a problem hiding this comment.
Ohh i didn't even notice that parse_return_type was called if parsing of the function body failed. ty
966513a to
aa112af
Compare
| LL - fn foo<T>() where T: Default -> { | ||
| LL + fn foo<T>() -> where T: Default { |
There was a problem hiding this comment.
This suggestion needs to be fixed since it's obviously incorrect: The "insertion span" probably needs to include the span of the where clause if available. Not sure if feasible.
There was a problem hiding this comment.
Is it obviously incorrect? i mean that suggested code doesn't compile, yes, but moving the return arrow before the where clause is correct.
Maybe i can try to insert something like a {return_type} into the suggestion after the arrow if there is a where?
There was a problem hiding this comment.
If i add a incorrect return type the suggestion looks a lot better.
fn foo<T>() where T: Default -> 1 + 1 {
help: place the return type after the function parameters
|
LL - fn foo<T>() where T: Default -> 1 + 1 {
LL + fn foo<T>() -> 1 + 1 where T: Default {
|
| | | ||
| LL - fn foo<T>() where T: Default -> { | ||
| LL + fn foo<T>() -> where T: Default { | ||
| | |
There was a problem hiding this comment.
This suggestion now triggers since you've updated parse_ret_ty to return Ok(_) in this case and so in error_fn_body_not_found the code believes it can use the parsed return type to offer the suggestion "since everything went well". Previously, it noped outta there as it used to be an Err(_).
fmease
added
S-waiting-on-author
aa112af to
ee1d083
Compare
inkreasing
changed the title
{, } or ;{, } or ;
|
@rustbot ready Either addressed or responded to the comments. |
rustbot
added
S-waiting-on-review
| --> $DIR/invalid-ret-ty-issue-162947.rs:13:15 | ||
| | | ||
| LL | fn a() -> 1 + 1 { | ||
| | ^ expected type |
There was a problem hiding this comment.
ideally i would like to change the span to point to the whole invalid return type.
I think for that i would need to edit the span of the Diag after it was created, but i don't know if that is even possible.
There was a problem hiding this comment.
There are APIs to modify the Diag's state, but it is generally safer to create a new one to emit, and delay_as_bug the previous one when you're significantly changing the error.
There was a problem hiding this comment.
I've thought about this and i don't think it is really possible (at least not without also changing the type parser).
I don't think i can differentiate between the -> 1 + 1 { and the -> A<B { error, but i would need to, since i only want to do that span expansion for the first case.
ee1d083 to
7e09eb2
Compare
4 participants
View all comments
Fixes #162947 (this time completely i think)
r? estebank (since you suggested this fix)
No AI used.