Rust: Comma after struct expansion is a syntax error

Created on 8 May 2017  路  6Comments  路  Source: rust-lang/rust

On rustc 1.17:

struct Foo {
    one: i32,
    two: i32,
    three: i32,
}

impl Default for Foo {
    fn default() -> Self {
        Foo { one : 1, two: 2, three: 3 }
    }
}

fn main() {
    // this works
    let foo = Foo {
        one: 11,
        ..Foo::default()
    };
    // this is a syntax error
    let foo = Foo {
        one: 111,
        ..Foo::default(),
    };
}

(playground link: https://is.gd/2wvenA)

This feels very inconsistent, as Rust has no problems with extra commas anywhere else. It is also an extremely "natural" mistake to make, so even if it's rightly signaled & easily corrected, it is still quite annoying.

A-diagnostics A-parser C-enhancement E-mentor WG-compiler-errors hacktoberfest

Most helpful comment

So we discussed this some in the @rust-lang/lang meeting. I would say that opinions were mixed, but in general I think that we are inclined (for now) to leave this as is.

The arguments basically were as follows. First, everyone agreed that we actively do not want to allow ..Foo anywhere but at the end of a struct expression.

Given that, the argument in favor of allowing commas is essentially that people might expect to put it there because they always end lines with a comma (habit, essentially).

The argument against is that a trailing comma's main purpose is to avoid messy diffs and simplify macro authors lives, and @petrochenkov made a convincing case that neither applies here. Moreover, a trailing comma suggests that you can move things around in the list, and that you might put something afterwards, neither of which are true here. So including the comma is somewhat misleading.

Therefore, I think we're inclined to leave the grammar as is, and prohibit trailing commas after a ..Foo syntax in a struct.

All 6 comments

I agree this is annoying and need to be fixed.
@petrochenkov , I guess this minor issue doesn't require RFC process, right?
This issue seems related to https://github.com/rust-lang/rust/blob/master/src/libsyntax/parse/parser.rs#L2369 . I'll submit a fix soon.

Rust has no problems with extra commas anywhere else.

Just for remark: the ... in extern "C" { fn foo(x: &str, ...); } doesn't allow a trailing comma either (in current Rust stable/nightly). I like the trailing comma for struct update syntax though!

So we discussed this some in the @rust-lang/lang meeting. I would say that opinions were mixed, but in general I think that we are inclined (for now) to leave this as is.

The arguments basically were as follows. First, everyone agreed that we actively do not want to allow ..Foo anywhere but at the end of a struct expression.

Given that, the argument in favor of allowing commas is essentially that people might expect to put it there because they always end lines with a comma (habit, essentially).

The argument against is that a trailing comma's main purpose is to avoid messy diffs and simplify macro authors lives, and @petrochenkov made a convincing case that neither applies here. Moreover, a trailing comma suggests that you can move things around in the list, and that you might put something afterwards, neither of which are true here. So including the comma is somewhat misleading.

Therefore, I think we're inclined to leave the grammar as is, and prohibit trailing commas after a ..Foo syntax in a struct.

Leaving this open to track improving the diagnostic here.

In order to avoid multiple errors after finding this comma, as it happens now, the comma has to be specifically _accepted_ and a diagnostic generated for this case in particular, along the lines of

error: can't use a comma after struct expansion
  --> src/main.rs:22:25
   |
22 |         ..Foo::default(),
   |                         ^ help: remove this comma

The method libsyntax/parse/parser.rs:Parser::parse_struct_expr needs to be modified to accept expressions that end in a comma, but generate and emit a diagnostic error. You might need to modify the call to self.parse_expr and you can look at self.parse_field to see what might be the difference in handling. Adding an err.span_suggestion_short(comma_span, "remove this comma", "".to_owned()) to the generated error is enough to add the help line as shown above.

Triage: I think this should be closed now, because #45178 has now been merged into master as of 3 days ago.

Was this page helpful?
0 / 5 - 0 ratings