IDL parser: align with current Web IDL specification

IDL parser: align with current Web IDL specification

Mostly renumbered and restructured grammar productions.  Notable changes:

- add support for static attributes via StaticMember
- add support for built-in types ByteString and RegExp
- allow trailing comma in enum declarations

Depends on:
IDL parser: drop AttributeOrOperation and StaticAttribute overrides
https://codereview.chromium.org/336733002/
...or else breaks Blink compile.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278365

[Jens Widell, 2014-06-12 13:29:16]

PTAL when convenient.

Note that this is an RFC as much as anything else.

This patch is incompatible with ToT Blink IDL parser (breaks compilation.) All
that is required to fix that appears to be to drop the Blink parser's overrides
of the AttributeOrOperation and StaticAttribute productions that become unused
with this patch. But we can't drop them first, of course. (Yes, merged
repositories will be nice.)

Simplest migration path ought to be to just "upstream" Blink's current static
attribute support, then drop it from Blink, and then land this patch. If we want
this patch, that is.

I'm adding a whole bunch of commented-out productions. This is mostly because I
went through the grammar and added (almost) all missing productions / adjusted
all modified ones, and then commented out those that I didn't want to implement
(and that weren't necessary to implement at this point.) Will be happy to throw
them away if we don't want the cruft.

[Jens Widell, 2014-06-12 13:34:37]

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser.py
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:196: # [2] Add INLINE definition
I don't know what this comment is about. :-) There doesn't seem to be any
"inline" anywhere.

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:772: # [77] Moved BYTESTRING, DOMSTRING, OBJECT,
DATE, REGEXP to PrimitiveType
I don't know why any of these are moved, but I guessed that moving all the ones
I added too made sense. All I can think of is that it avoids duplicating
TypeSuffix once for each type.

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:909: #def p_Identifiers(self, p):
Note that this is identical to [37]. Reported
https://www.w3.org/Bugs/Public/show_bug.cgi?id=26075 about that.

[noelallen1, 2014-06-12 19:33:37]

Started to review, but not sure which IDL spec you are using since the
production numbers [ X ] don't match the provided link.

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_lexer.py
File tools/idl_parser/idl_lexer.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_lexer.p...
tools/idl_parser/idl_lexer.py:88: #'iterator': 'ITERATOR',
Did you mean to leave this in?

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser.py
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:196: # [2] Add INLINE definition
On 2014/06/12 13:34:36, Jens Lindström wrote:
> I don't know what this comment is about. :-) There doesn't seem to be any
> "inline" anywhere.

This really should just be:
 # [2] 
To signify it's defining the 2nd item in the WebIDL grammar.

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:375: # [24]
This does not match:
   http://www.w3.org/TR/WebIDL/#idl-grammar
   [24]	Typedef	"typedef" ExtendedAttributeList Type identifier ";"

The number [x] refers to the equivalent WebIDL definition.  If you are using a
newer or experimental spec other than: http://www.w3.org/TR/WebIDL/#idl-grammar
then make use to update the comment above referencing this link.

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:449: # def p_Serializer(self, p):
I have mixed feeling about these place holders.  I think it's great to update
the numbers above to match the spec, but adding these NOT_IMPLEMENTED
definitions implies an updating burden for something that's a comment.

Would it be just as easy to understand yet simpler to maintain if we just had a
single comment:

# [32 - 39]  NOT IMPLEMENTED

I could be convinced either way.

[Jens Widell, 2014-06-12 20:10:00]

On 2014/06/12 19:33:37, noelallen1 wrote:
> Started to review, but not sure which IDL spec you are using since the
> production numbers [ X ] don't match the provided link.

Yes, sorry about. I meant to update the spec link too, but forgot to actually do
it.

The spec I used was the editor's draft at http://heycam.github.io/webidl/.

[noelallen1, 2014-06-12 20:39:31]

The commented out productions are open question still, maybe Nils wants to
comment?  But I can live with it as is.

Still need to update:
  http://www.w3.org/TR/WebIDL/#idl-grammar

othewise LGTM

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser.py
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:772: # [77] Moved BYTESTRING, DOMSTRING, OBJECT,
DATE, REGEXP to PrimitiveType
On 2014/06/12 13:34:36, Jens Lindström wrote:
> I don't know why any of these are moved, but I guessed that moving all the
ones
> I added too made sense. All I can think of is that it avoids duplicating
> TypeSuffix once for each type.

Moved just to make parsing easier.  Otherwise it's harder to differentiate
'identifier' and one of these 'PrimitiveType'.

[Jens Widell, 2014-06-13 06:19:12]

Updated patch to change spec links to point to http://heycam.github.io/webidl/,
and removed the commented out placeholders.

Thanks for reviewing!

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_lexer.py
File tools/idl_parser/idl_lexer.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_lexer.p...
tools/idl_parser/idl_lexer.py:88: #'iterator': 'ITERATOR',
On 2014/06/12 19:33:36, noelallen1 wrote:
> Did you mean to leave this in?

Not really. Same here as with the commented out productions; I first added
everything in the spec, then removed what I didn't implement, and then removed
more things until PLY stopped warning about unused/unreachable things.

[noelallen1, 2014-06-13 18:40:53]

slgtm

[Nils Barth (inactive), 2014-06-16 06:50:48]

LGTM with comment nits -- ok to commit once Blink parser transition CL lands.

Thanks for doing this, and for updating to (useful) spec changes!

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser.py
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:772: # [77] Moved BYTESTRING, DOMSTRING, OBJECT,
DATE, REGEXP to PrimitiveType
On 2014/06/12 20:39:31, noelallen1 wrote:
> On 2014/06/12 13:34:36, Jens Lindström wrote:
> > I don't know why any of these are moved, but I guessed that moving all the
> ones
> > I added too made sense. All I can think of is that it avoids duplicating
> > TypeSuffix once for each type.
> 
> Moved just to make parsing easier.  Otherwise it's harder to differentiate
> 'identifier' and one of these 'PrimitiveType'.

Thanks for explaining!
Could this be aligned with spec, or is that difficult?

Jens:
Could you add a comment (and possibly FIXME) to explain this?

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:448: # [32-39] NOT IMPLEMENTED
Could you link to the stringifier bug?
// FIXME: Not implemented (stringifier) http://crbug.com/306606

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:529: # [51-53] NOT IMPLEMENTED
Could you elab as "NOT IMPLEMENTED (iterator)"
Do we have any plans to implement this? (Fine if not.)

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:734: # [79]
Could you note that we've added BYTESTRING etc.?

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:838: # [89-90] NOT IMPLEMENTED
Gloss: (IdentifierList)

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:857: # [95]
# [94] NOT IMPLEMENTED (ExtendedAttributeIdentList)

[Nils Barth (inactive), 2014-06-16 06:51:48]

On 2014/06/12 20:39:31, noelallen1 wrote:
> The commented out productions are open question still, maybe Nils wants to
> comment?  But I can live with it as is.

To agree with Noel:
we generally avoid commented out code for exactly the maintenance issue.
(Rare exceptions if going to uncomment in immediate followup.)

[Jens Widell, 2014-06-16 10:39:49]

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:529: # [51-53] NOT IMPLEMENTED
On 2014/06/16 06:50:48, Nils Barth wrote:
> Do we have any plans to implement this? (Fine if not.)

I had no plans right now, at least. Implementing support for the syntax here
should be simple enough that I could do it here if we think it's meaningful
(assuming that we one day will want to use it in our IDL files too.)

[Jens Widell, 2014-06-16 10:53:11]

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser.py
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:772: # [77] Moved BYTESTRING, DOMSTRING, OBJECT,
DATE, REGEXP to PrimitiveType
On 2014/06/16 06:50:47, Nils Barth wrote:
> Could this be aligned with spec, or is that difficult?

Aligning with spec would not more difficult than doing

  if type(p[1]) is str:
    if p[1] in ('ByteString' et al):
      p[0] = self.BuildNamed('PrimitiveType', p, 1)
    else:
      p[0] = self.BuildNamed('Typeref', p, 1)

I think. (Not sure 'PrimitiveType' strictly makes sense, but that's the name
with the current code.)

[Nils Barth (inactive), 2014-06-17 05:08:18]

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser.py
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:772: # [77] Moved BYTESTRING, DOMSTRING, OBJECT,
DATE, REGEXP to PrimitiveType
On 2014/06/16 10:53:11, Jens Lindström wrote:
> On 2014/06/16 06:50:47, Nils Barth wrote:
> > Could this be aligned with spec, or is that difficult?
> 
> Aligning with spec would not more difficult than doing
> 
>   if type(p[1]) is str:
>     if p[1] in ('ByteString' et al):
>       p[0] = self.BuildNamed('PrimitiveType', p, 1)
>     else:
>       p[0] = self.BuildNamed('Typeref', p, 1)
> 
> I think. (Not sure 'PrimitiveType' strictly makes sense, but that's the name
> with the current code.)

That sounds great!
(Agreed that "primitive type" is a pretty confusing name at this point.)

Noel, WDYT?

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/60001/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:529: # [51-53] NOT IMPLEMENTED
On 2014/06/16 10:39:49, Jens Lindström wrote:
> On 2014/06/16 06:50:48, Nils Barth wrote:
> > Do we have any plans to implement this? (Fine if not.)
> 
> I had no plans right now, at least. Implementing support for the syntax here
> should be simple enough that I could do it here if we think it's meaningful
> (assuming that we one day will want to use it in our IDL files too.)

Got it; as a rule we don't add support for anything we're not using
(helps avoid rotten dead code), so if we're not implementing it yet,
let's not add it.
(As you say, it's easy enough to add.)

[Nils Barth (inactive), 2014-06-17 05:11:55]

https://codereview.chromium.org/329853005/diff/80001/tools/idl_parser/idl_ppa...
File tools/idl_parser/idl_ppapi_lexer.py (right):

https://codereview.chromium.org/329853005/diff/80001/tools/idl_parser/idl_ppa...
tools/idl_parser/idl_ppapi_lexer.py:69:
lexer.Tokenize(open('test_parser/inline_ppapi.idl').read())
For bonus points, could you replace this resource-leaking open() with a |with|
statement?

with open('test_parser/inline_ppapi.idl') as test_file:
    ...test_file.read()
?

Thanks!

[Jens Widell, 2014-06-17 06:18:37]

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser.py
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:772: # [77] Moved BYTESTRING, DOMSTRING, OBJECT,
DATE, REGEXP to PrimitiveType
On 2014/06/17 05:08:18, Nils Barth wrote:
> On 2014/06/16 10:53:11, Jens Lindström wrote:
> > On 2014/06/16 06:50:47, Nils Barth wrote:
> > > Could this be aligned with spec, or is that difficult?
> > 
> > Aligning with spec would not more difficult than doing

"Not more difficult" was apparently a bit optimistic. Two more reasons to do
this cropped up:

- The Pepper IDL lexer undefines these keyword tokens, and its parser overrides
PrimitiveType, but it doesn't override NonAnyType. Thus, using these keywords in
NonAnyType makes the grammar definition invalid due to BYTESTRING et al being
undefined. (This can of course be worked around by overriding NonAnyType as
well.)

- ConstType references PrimitiveType directly, meaning moving these types into
the PrimitiveType production makes them available for constants. And we at least
have tests (here and in Blink) for DOMString constants.

[Nils Barth (inactive), 2014-06-17 06:28:25]

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser.py
File tools/idl_parser/idl_parser.py (right):

https://codereview.chromium.org/329853005/diff/1/tools/idl_parser/idl_parser....
tools/idl_parser/idl_parser.py:772: # [77] Moved BYTESTRING, DOMSTRING, OBJECT,
DATE, REGEXP to PrimitiveType
On 2014/06/17 06:18:37, Jens Lindström wrote:
> "Not more difficult" was apparently a bit optimistic. Two more reasons to do
> this cropped up:

Got it; that's a lot of work for minor spec compliance fix,
so fine to omit for now.
(FIXME isn't necessary, as there's a comment explaining the difference.)

[Jens Widell, 2014-06-19 11:40:06]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-06-19 11:43:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jl@opera.com/329853005/100001

[commit-bot: I haz the power, 2014-06-19 14:43:28]

Change committed as 278365