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

Suboptimal error in case of duplicate , in struct constructor #50974

Closed
oli-obk opened this issue May 22, 2018 · 6 comments
Closed

Suboptimal error in case of duplicate , in struct constructor #50974

oli-obk opened this issue May 22, 2018 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented May 22, 2018

struct Foo {
    a: i32,
    b: i32,
}

fn main() {
    let x = Foo {
        a: 42,,
        b: 3,
    };
}

results in

error: expected identifier, found `,`
 --> src/main.rs:9:15
  |
8 |     let x = Foo {
  |             --- while parsing this struct
9 |         a: 42,,
  |               ^ expected identifier

error[E0063]: missing field `b` in initializer of `Foo`
 --> src/main.rs:8:13
  |
8 |     let x = Foo {
  |             ^^^ missing `b`

we may want to add a parser recovery that just reports the error, eats the comma and continues parsing as normally

@oli-obk oli-obk added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-diagnostics Area: Messages for errors, warnings, and lints labels May 22, 2018
@estebank
Copy link
Contributor

The parser gives up on parsing this error here:

https://github.com/estebank/rust/blob/47a8eb7c4e24b61a8a9ab4eaff60ef65291aaa56/src/libsyntax/parse/parser.rs#L2495-L2503

It could try to continue parsing if it encounters some simple tokens and a look ahead verifies that the next field is well formed. It could also recover from missing commas as well.

@oli-obk
Copy link
Contributor Author

oli-obk commented May 22, 2018

Missing commas have been fixed mere moments ago: #50914 (comment)

@PramodBisht
Copy link
Contributor

PramodBisht commented May 23, 2018

@estebank @oli-obk I checked above issue in master branch after merge of #50914, this issue still persists, let me know if I am missing anything, else I would like to take on this issue if nobody is working on this.

@estebank
Copy link
Contributor

@PramodBisht #50914 only addresses the case where a comma is missing in a struct definition.

The following broken code:

struct Foo {
    a: i32
    b: i32,
}

struct Bar {
    a: i32,,
    b: i32,
}

fn main() {
    let x = Foo {
        a: 42
        b: 3,
    };
    let y = Bar {
        a: 42,,
        b: 3,
    };
}

Should only have the following output:

error: expected `,`, or `}`, found `b`
 --> src/main.rs:2:11
  |
2 |     a: i32
  |           ^ help: try adding a comma: `,`

error: expected identifier, found `,`
 --> src/main.rs:7:12
  |
7 |     a: i32,,
  |            ^
  |            |
  |            expected identifier
  |            help: remove this comma

error: expected one of `,`, `.`, `?`, `}`, or an operator, found `b`
  --> src/main.rs:14:9
   |
13 |         a: 42
   |              - 
   |              |
   |              expected one of `,`, `.`, `?`, `}`, or an operator here
   |              help: add a comma here: `,`
14 |         b: 3,
   |         ^ unexpected token

error: expected identifier, found `,`
  --> src/main.rs:17:15
   |
16 |     let y = Bar {
   |             --- while parsing this struct
17 |         a: 42,,
   |               ^
   |               |
   |               expected identifier
   |               help: remove this comma

Note that there should be no error about missing fields, as the struct definition parser should recover after the parse error and include the following fields.

@PramodBisht
Copy link
Contributor

@estebank Ah, right. Got it now :)

PramodBisht added a commit to PramodBisht/rust that referenced this issue May 25, 2018
lambtowolf added a commit to lambtowolf/rust that referenced this issue May 29, 2018
@PramodBisht
Copy link
Contributor

@lambtowolf : I was already working on that, feel free to snatch it from me if I don't raise PR in next few days :P

lambtowolf added a commit to lambtowolf/rust that referenced this issue May 29, 2018
lambtowolf added a commit to lambtowolf/rust that referenced this issue May 29, 2018
lambtowolf added a commit to lambtowolf/rust that referenced this issue May 30, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 5, 2018
Issue rust-lang#50974 : Suboptimal error in case of duplicate `,` in struct constructor

Fixes rust-lang#50974
pietroalbini pushed a commit to lambtowolf/rust that referenced this issue Jun 22, 2018
pietroalbini pushed a commit to lambtowolf/rust that referenced this issue Jun 22, 2018
pietroalbini pushed a commit to lambtowolf/rust that referenced this issue Jun 22, 2018
pietroalbini pushed a commit to lambtowolf/rust that referenced this issue Jun 22, 2018
bors added a commit that referenced this issue Jun 22, 2018
Issue #50974 : Suboptimal error in case of duplicate `,` in struct constructor

Fixes #50974
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

3 participants