IDL parser: implement Stringifier production

IDL parser: implement Stringifier production

Emit a node with class 'Stringifier' for all stringifier variants.
If the stringifier is an attribute, this node has a child node that
represents the attribute. If the stringifier is an operation (named
or unnamed) the node has a child node that represents the operation.

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

[Jens Widell, 2014-06-11 15:28:51]

PTAL.

Since any use of 'stringifier' crashes the parser currently, it seems safe to
assume no IDL files anywhere depends on how it's handled, and that changing the
output is safe.

How I changed the outupt might still be completely wrong, of course, but at
least not for that reason. :-)

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

https://codereview.chromium.org/329163002/diff/10003/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:440: p[0] = self.BuildProduction('Stringifier',
p, 1, ListFromConcat(p[2]))
Modifying p_AttributeOrOperation() like this is slightly unfortunate, because
Blink's parser overrides it, and thus needs to duplicate this line, or otherwise
handle it itself.

I did it primarily so that I could use the 'stringifier' token's source code
location (the "p, 1," bit.)

[Nils Barth (inactive), 2014-06-12 04:20:27]

LGTM, thanks!
Noel?

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

https://codereview.chromium.org/329163002/diff/10003/tools/idl_parser/idl_par...
tools/idl_parser/idl_parser.py:440: p[0] = self.BuildProduction('Stringifier',
p, 1, ListFromConcat(p[2]))
On 2014/06/11 15:28:51, Jens Lindström wrote:
> Modifying p_AttributeOrOperation() like this is slightly unfortunate, because
> Blink's parser overrides it, and thus needs to duplicate this line, or
otherwise
> handle it itself.

This is fine; we only override this b/c we want to support
StaticAttribute,
but that's now part of the reference grammar:
http://heycam.github.io/webidl/#idl-grammar
...so we can move it into the base parser!
(Such cleanup will be *much* easier once repos are merged.)

Specifically:
  AttributeOrOperation
has been replaced by:
  AttributeOrOperationOrIterator
http://heycam.github.io/webidl/#proddef-AttributeOrOperationOrIterator

...which includes
  StaticMember
as a possible expansion.

[noelallen1, 2014-06-12 19:15:23]

Thanks!  LGTM

[Jens Widell, 2014-06-23 08:18:32]

On 2014/06/12 19:15:23, noelallen1 wrote:
> Thanks!  LGTM

Patch rebased. Since the grammar spec alignment patch changed things around
pretty much (actually removed the Stringifier production entirely) this patch
now looks like an initial implementation of it, rather than a fix to it. The
parser output is equivalent.

[Nils Barth (inactive), 2014-06-23 08:51:23]

On 2014/06/23 08:18:32, Jens Lindström wrote:
> On 2014/06/12 19:15:23, noelallen1 wrote:
> > Thanks!  LGTM
> 
> Patch rebased. Since the grammar spec alignment patch changed things around
> pretty much (actually removed the Stringifier production entirely) this patch
> now looks like an initial implementation of it, rather than a fix to it. The
> parser output is equivalent.

Thanks, looks great (indeed, better than before); feel free to commit!

[Jens Widell, 2014-06-23 09:21:32]

On 2014/06/23 08:51:23, Nils Barth wrote:
> On 2014/06/23 08:18:32, Jens Lindström wrote:
> > On 2014/06/12 19:15:23, noelallen1 wrote:
> > > Thanks!  LGTM
> > 
> > Patch rebased. Since the grammar spec alignment patch changed things around
> > pretty much (actually removed the Stringifier production entirely) this
patch
> > now looks like an initial implementation of it, rather than a fix to it. The
> > parser output is equivalent.
> 
> Thanks, looks great (indeed, better than before);

Thanks!

> feel free to commit!

I'm having an odd problem when I apply my follow-up in Blink that actually
implements stringifiers. I'll hold off on committing this until I've figure out
what's causing it, in case the problem is here.

Everything works fine in the bindings tests (generates the code I expect and
all) but if I try to use "stringifier" in a real IDL file, things just fall
apart. If I change the toString() in DOMTokenList.idl to "[NotEnumerable]
stringifier;", then it appears as if the script gets stuck in yacc.py. If I drop
the "[NotEnumerable]", thus having "stringifier;" last in the interface, I get
"Expected exactly 1 interface in file [...]/DOMTokenList.idl, but found 0". If I
move the stringifier so that it's not the last member, the script hangs in
yacc.py again.

[Nils Barth (inactive), 2014-06-23 09:26:01]

On 2014/06/23 09:21:32, Jens Lindström wrote:
> I'm having an odd problem when I apply my follow-up in Blink that actually
> implements stringifiers. I'll hold off on committing this until I've figure
out
> what's causing it, in case the problem is here.
> 
> Everything works fine in the bindings tests (generates the code I expect and
> all) but if I try to use "stringifier" in a real IDL file, things just fall
> apart. If I change the toString() in DOMTokenList.idl to "[NotEnumerable]
> stringifier;", then it appears as if the script gets stuck in yacc.py. If I
drop
> the "[NotEnumerable]", thus having "stringifier;" last in the interface, I get
> "Expected exactly 1 interface in file [...]/DOMTokenList.idl, but found 0". If
I
> move the stringifier so that it's not the last member, the script hangs in
> yacc.py again.

Ow, that looks bad.
It's possible to debug PLY (debug=True, and look at dumped parsing to see where
it's getting stuck), but it's icky.

[Jens Widell, 2014-06-23 09:33:27]

On 2014/06/23 09:26:01, Nils Barth wrote:
> On 2014/06/23 09:21:32, Jens Lindström wrote:
> > I'm having an odd problem when I apply my follow-up in Blink that actually
> > implements stringifiers. I'll hold off on committing this until I've figure
> out
> > what's causing it, in case the problem is here.
> > 
> > Everything works fine in the bindings tests (generates the code I expect and
> > all) but if I try to use "stringifier" in a real IDL file, things just fall
> > apart. If I change the toString() in DOMTokenList.idl to "[NotEnumerable]
> > stringifier;", then it appears as if the script gets stuck in yacc.py. If I
> drop
> > the "[NotEnumerable]", thus having "stringifier;" last in the interface, I
get
> > "Expected exactly 1 interface in file [...]/DOMTokenList.idl, but found 0".
If
> I
> > move the stringifier so that it's not the last member, the script hangs in
> > yacc.py again.
> 
> Ow, that looks bad.
> It's possible to debug PLY (debug=True, and look at dumped parsing to see
where
> it's getting stuck), but it's icky.

If I replace TestInterface2.idl with the contents of DOMTokenList.idl (just
renaming the interface,) run-bindings-tests still runs fine and generates the
expected content. I'm confused. :-)

[Jens Widell, 2014-06-23 09:36:30]

On 2014/06/23 09:33:27, Jens Lindström wrote:
> On 2014/06/23 09:26:01, Nils Barth wrote:
> > It's possible to debug PLY (debug=True, and look at dumped parsing to see
> where
> > it's getting stuck), but it's icky.

Setting debug=True fixed the problem... :-/

[Nils Barth (inactive), 2014-06-23 09:43:44]

On 2014/06/23 09:36:30, Jens Lindström wrote:
> On 2014/06/23 09:33:27, Jens Lindström wrote:
> > On 2014/06/23 09:26:01, Nils Barth wrote:
> > > It's possible to debug PLY (debug=True, and look at dumped parsing to see
> > where
> > > it's getting stuck), but it's icky.
> 
> Setting debug=True fixed the problem... :-/

Idea:
It's possible that your build is using outdated cached lexer or parser table
files,
and thus isn't lexing 'stringifier' correctly (it's just an identifier),
hence the *parser* failure?

(debug=True doesn't used cached tables, hence why that would fix it)

That *shouldn't* happen (I believe dependencies are correct for caching);
if it does, that's an important issue to catch else it might break bots.
(If it's hard to resolve you can just add a landmine to force a clobber build,
which should fix it.)

[Jens Widell, 2014-06-23 09:44:56]

On 2014/06/23 09:36:30, Jens Lindström wrote:
> On 2014/06/23 09:33:27, Jens Lindström wrote:
> > On 2014/06/23 09:26:01, Nils Barth wrote:
> > > It's possible to debug PLY (debug=True, and look at dumped parsing to see
> > where
> > > it's getting stuck), but it's icky.
> 
> Setting debug=True fixed the problem... :-/

Right, now the problem is gone completely. With debug=False too. Some kind of
bad caching of parser tables or what not? I'm clueless.

[Jens Widell, 2014-06-23 09:52:03]

On 2014/06/23 09:44:56, Jens Lindström wrote:
> On 2014/06/23 09:36:30, Jens Lindström wrote:
> > On 2014/06/23 09:33:27, Jens Lindström wrote:
> > > On 2014/06/23 09:26:01, Nils Barth wrote:
> > > > It's possible to debug PLY (debug=True, and look at dumped parsing to
see
> > > where
> > > > it's getting stuck), but it's icky.
> > 
> > Setting debug=True fixed the problem... :-/
> 
> Right, now the problem is gone completely. With debug=False too. Some kind of
> bad caching of parser tables or what not? I'm clueless.

I seem to be able to reproduce it using these steps:

1) Start with master (from an hour or so ago) in src and third_party/WebKit
2) Build the bindings targets (bindings_core_v8_generated +
bindings_modules_v8_generated)
3) Apply this patch
4) Build the bindings targets again, without changing any IDL files or the Blink
IDL parser
5) Change DOMTokenList.idl to use the stringifier keyword
6) Build again => script hangs

Will investigate what's going on here.

[Jens Widell, 2014-06-23 10:50:37]

On 2014/06/23 09:52:03, Jens Lindström wrote:
> On 2014/06/23 09:44:56, Jens Lindström wrote:
> > On 2014/06/23 09:36:30, Jens Lindström wrote:
> > > On 2014/06/23 09:33:27, Jens Lindström wrote:
> > > > On 2014/06/23 09:26:01, Nils Barth wrote:
> > > > > It's possible to debug PLY (debug=True, and look at dumped parsing to
> see
> > > > where
> > > > > it's getting stuck), but it's icky.
> > > 
> > > Setting debug=True fixed the problem... :-/
> > 
> > Right, now the problem is gone completely. With debug=False too. Some kind
of
> > bad caching of parser tables or what not? I'm clueless.
> 
> I seem to be able to reproduce it using these steps:
> 
> 1) Start with master (from an hour or so ago) in src and third_party/WebKit
> 2) Build the bindings targets (bindings_core_v8_generated +
> bindings_modules_v8_generated)
> 3) Apply this patch
> 4) Build the bindings targets again, without changing any IDL files or the
Blink
> IDL parser
> 5) Change DOMTokenList.idl to use the stringifier keyword
> 6) Build again => script hangs
> 
> Will investigate what's going on here.

Issue found in Blink. Fix here: https://codereview.chromium.org/332683004

Will go ahead and commit this.

[Jens Widell, 2014-06-23 10:50:40]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-06-23 10:51:50]

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

[Nils Barth (inactive), 2014-06-23 11:11:29]

The CQ bit was unchecked by nbarth@chromium.org

[Nils Barth (inactive), 2014-06-23 11:12:17]

On 2014/06/23 10:50:37, Jens Lindström wrote:
> Issue found in Blink. Fix here: https://codereview.chromium.org/332683004
> 
> Will go ahead and commit this.

I think it's safer to wait for the Blink fix to land and get rolled,
just in case, right?

(Hence removed commit.)

[Nils Barth (inactive), 2014-06-23 11:12:46]

The CQ bit was checked by nbarth@chromium.org

[Nils Barth (inactive), 2014-06-23 11:13:42]

On 2014/06/23 11:12:17, Nils Barth wrote:
> I think it's safer to wait for the Blink fix to land and get rolled,
> just in case, right?
> 
> (Hence removed commit.)

Sorry, I'm wrong: this CL doesn't affect the build itself,
since not using |stringifier| in any real IDL files.
We just need the Blink fix to land before you start making
changes to real IDL files.

(Rechecked bit.)

[commit-bot: I haz the power, 2014-06-23 11:13:53]

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

[Jens Widell, 2014-06-23 11:19:56]

On 2014/06/23 11:13:42, Nils Barth wrote:
> On 2014/06/23 11:12:17, Nils Barth wrote:
> > I think it's safer to wait for the Blink fix to land and get rolled,
> > just in case, right?
> > 
> > (Hence removed commit.)
> 
> Sorry, I'm wrong: this CL doesn't affect the build itself,
> since not using |stringifier| in any real IDL files.
> We just need the Blink fix to land before you start making
> changes to real IDL files.

Yes, I don't think this causes any trouble on its own. It's only when
stringifier is used that weird things happen.

[commit-bot: I haz the power, 2014-06-23 13:03:18]

Change committed as 279097