D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 2477 - Trailing comma in array literal sometimes accepted, sometimes not
Summary: Trailing comma in array literal sometimes accepted, sometimes not
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: x86 All
: P2 enhancement
Assignee: No Owner
URL:
Keywords: diagnostic, patch, spec
Depends on:
Blocks:
 
Reported: 2008-11-28 23:04 UTC by Bill Baxter
Modified: 2010-08-28 13:51 UTC (History)
6 users (show)

See Also:


Attachments
allow trailing commas in various places (1.30 KB, patch)
2010-04-30 08:49 UTC, Ellery Newcomer
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Bill Baxter 2008-11-28 23:04:29 UTC
void main() {
    int[] data = [1,2,3,];  // OK

    data = [ 1,2,3, ];  // fails
/+
// error messages
trailcomma.d(20): found ';' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
trailcomma.d(29): found 'EOF' when expecting ','
+/
}

The repetition of the EOF error message 20 times is also rather odd.

I don't know if trailing comma is spec'ed as being allowed or not (I much prefer allowing it), but either way it should be consistent.
Comment 1 Stewart Gordon 2008-11-30 19:20:39 UTC
The problem is that the spec is inconsistent.  In the first case, it's not an array literal, but an array initializer.

ArrayInitializer:
	[ ]
	[ ArrayMemberInitializations ]

ArrayMemberInitializations:
	ArrayMemberInitialization
	ArrayMemberInitialization ,
	ArrayMemberInitialization , ArrayMemberInitializations

versus

ArrayLiteral:
	[ ArgumentList ]

ArgumentList:
	AssignExpression
	AssignExpression , ArgumentList


Making these consistent is technically an enhancement request rather than a bug.  I nearly set this to enhancement, but the second issue (repetition of the error message) qualifies as a bug IMO.
Comment 2 Bill Baxter 2008-11-30 19:38:35 UTC
Thanks for the spec digging, Stewart.  So I see three options:

1) change the ArrayMemberInitializations production to:

ArrayMemberInitializations:
        ArrayMemberInitialization
        ArrayMemberInitialization , ArrayMemberInitializations

(get rid of the trailing comma case for initializations)

2) change the ArrayLiteral productions to 

ArrayLiteral:
        [ ArrayLiteralMemberList ]

ArrayLiteralMemberList:
        AssignExpression
        AssignExpression ,
        AssignExpression , ArrayLiteralMemberList

3) Just make all argument lists have an optional trailing comma.  This is based on the thinking that if allowing a trailing comma is a nice feature for arrays, then why not general argument lists (which can be used as general array constructors with typesafe variadic A[]... style parameters.)?

ArgumentList:
        AssignExpression
        AssignExpression ,
        AssignExpression , ArgumentList

I vote for #2.
Comment 3 Brian Kropf 2009-07-17 18:24:47 UTC
I'd originally submitted this on the ldc bugtracker and got forwarded here (since it is using the dmd front-end).  Here's a bit more info on the issue:

When using sabledd generated code, I'm seeing an error with ldc that was not present when I used gdc. It boils down to ldc not accepting only part of the endings of array initializers having commas. The following minimal test case does not build successfully on ldc x64 (but it will if you add a comma to the end of the second array initializer (like '[ 4, 5, 6, ],'). Curiously enough it also builds cleanly if I remove the commas after the 3 and the 6 in the example.

test.d: int arr[][] = [
    [ 1, 2, 3, ], [ 4, 5, 6, ]
];

int main(char[][] args) {
    return 0;
}

Error output:

test.d(7): expression expected, not ']' test.d(7): comma expected separating array initializers, not ; test.d(9): semicolon expected, not 'int' Error: Command failed, aborting.
Comment 4 Aziz Köksal 2010-04-30 06:54:22 UTC
I agree, ArgumentLists, ArrayLiterals etc. should allow a trailing comma, just like ParameterLists or enums.

Fixing this should be possible in 6 to 10 lines of code. Why is this issue still open?
Comment 5 Don 2010-04-30 07:18:03 UTC
(In reply to comment #4)
> I agree, ArgumentLists, ArrayLiterals etc. should allow a trailing comma, just
> like ParameterLists or enums.
> 
> Fixing this should be possible in 6 to 10 lines of code. Why is this issue
> still open?

Because there are over 1000 open bugs, and nearly all of them are more important than this one. Make a patch for it, if you care about it.
Comment 6 Ellery Newcomer 2010-04-30 08:49:05 UTC
Created attachment 615 [details]
allow trailing commas in various places

modifications for ArrayLiteral, Arguments, TemplateParameterList, and TemplateArguments.

I haven't tested it too heavily, but the simple stuff works. There might be latent trouble in the ambiguity between ArrayInitializer and ArrayLiteral (aside from what's already present), as I'm not well versed in what happens past the parser.
Comment 7 Aziz Köksal 2010-04-30 09:05:23 UTC
Don, I know it's not extremely important. I care about it, but I can't make patches for the DMD front- or back-end. Not because I don't want to, but because I'm working on a similar project; my own D compiler, remember? :-)

Ellery, thanks for taking care of this.
Comment 8 Ellery Newcomer 2010-04-30 10:02:04 UTC
(In reply to comment #7)
> Don, I know it's not extremely important. I care about it, but I can't make
> patches for the DMD front- or back-end. Not because I don't want to, but
> because I'm working on a similar project; my own D compiler, remember? :-)
> 

Come to think of it, so am I, or I was. I don't care much about the licensing, though. Or releasing it, if I ever finish it.

> Ellery, thanks for taking care of this.

Only needed to change 4 lines :)
Comment 9 Walter Bright 2010-08-28 13:19:43 UTC
Since this changes the language, I'm going to do it for D2 only and mark it as an enhancement request.
Comment 10 Walter Bright 2010-08-28 13:20:58 UTC
A more comprehensive test case:

void foo(T,)(T t) { T x; }

void main() {
    int[] data = [1,2,3,];
    data = [ 1,2,3, ];
    auto i = data[1,];
    foo!(int)(3);
    foo!(int,)(3);
    foo!(int,)(3,);
}
Comment 11 Walter Bright 2010-08-28 13:41:10 UTC
For D2:

http://www.dsource.org/projects/dmd/changeset/644
Comment 12 Walter Bright 2010-08-28 13:51:24 UTC
Phobos changeset 1939