IDL: Use IdlReader to compute interface_info_individual

IDL: Use IdlReader to compute interface_info_individual

Before this CL, we used regex to compute interface depenencies and other
information which is required to generate bindings code. We now also want
to gather all union types, but regex wouldn't work well to collect union
types. Instead using regex, use IdlReader to compute them. This way,
we can easily collect necessary information including union types.

This CL doesn't change generated bindings code; follow-up CLs will add
union types collection.

BUG=240176
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184138

[bashi, 2014-10-21 05:46:55]

bashi@chromium.org changed reviewers:
+ haraken@chromium.org, tasak@google.com

[bashi, 2014-10-21 05:46:56]

Haraken-san, tasak-san, PTAL? (shiino-san, FYI)

This CL slows down run-bindings-tests.

w/o this CL
2.22s user 0.23s system 99% cpu 2.454 total

w/
3.94s user 0.25s system 99% cpu 4.193 total

However, we will gain benefit from this and this shouldn't be a big problem, as
we chatted offline.

https://codereview.chromium.org/670663002/diff/1/Source/bindings/core/generat...
File Source/bindings/core/generated.gyp (right):

https://codereview.chromium.org/670663002/diff/1/Source/bindings/core/generat...
Source/bindings/core/generated.gyp:62: 'cache_directory':
'<(bindings_core_output_dir)/../scripts',
'cache_directory' contains 'lextab.py', which is required by IdlReader. (Or
IdlReader generate it if it doesn't exist)

[Jens Widell, 2014-10-21 10:59:39]

jl@opera.com changed reviewers:
+ jl@opera.com

[Jens Widell, 2014-10-21 10:59:39]

https://codereview.chromium.org/670663002/diff/1/Source/bindings/core/generat...
File Source/bindings/core/generated.gyp (right):

https://codereview.chromium.org/670663002/diff/1/Source/bindings/core/generat...
Source/bindings/core/generated.gyp:62: 'cache_directory':
'<(bindings_core_output_dir)/../scripts',
On 2014/10/21 05:46:55, bashi1 wrote:
> 'cache_directory' contains 'lextab.py', which is required by IdlReader. (Or
> IdlReader generate it if it doesn't exist)

The gyp target that generates lextab.py is 'cache_lex_yacc_tables'.

You should add an explicit dependency on it, unless this target already does
depend on it in some other reliable way. The reason for that is that 'lextab.py'
is only regenerated automatically if it doesn't exist; it isn't regenerated if
it's "just" out-of-date.

The 'cache_lex_yacc_tables' target takes care of that. It also avoids the
possibility of a race between multiple targets generating lextab.py in parallel.

[bashi, 2014-10-21 23:58:32]

On 2014/10/21 10:59:39, Jens Widell wrote:
>
https://codereview.chromium.org/670663002/diff/1/Source/bindings/core/generat...
> File Source/bindings/core/generated.gyp (right):
> 
>
https://codereview.chromium.org/670663002/diff/1/Source/bindings/core/generat...
> Source/bindings/core/generated.gyp:62: 'cache_directory':
> '<(bindings_core_output_dir)/../scripts',
> On 2014/10/21 05:46:55, bashi1 wrote:
> > 'cache_directory' contains 'lextab.py', which is required by IdlReader. (Or
> > IdlReader generate it if it doesn't exist)
> 
> The gyp target that generates lextab.py is 'cache_lex_yacc_tables'.
> 
> You should add an explicit dependency on it, unless this target already does
> depend on it in some other reliable way. The reason for that is that
'lextab.py'
> is only regenerated automatically if it doesn't exist; it isn't regenerated if
> it's "just" out-of-date.
> 
> The 'cache_lex_yacc_tables' target takes care of that. It also avoids the
> possibility of a race between multiple targets generating lextab.py in
parallel.

Done. Thank you for pointing this out!

[haraken, 2014-10-22 02:09:13]

LGTM (but I'm not familiar with how jinja cache works)

[bashi, 2014-10-22 02:17:58]

The CQ bit was checked by bashi@chromium.org

[commit-bot: I haz the power, 2014-10-22 02:18:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/670663002/40001

[commit-bot: I haz the power, 2014-10-22 03:16:11]

Committed patchset #3 (id:40001) as 184138

[Nils Barth (inactive), 2014-10-22 14:19:51]

nbarth@chromium.org changed reviewers:
+ nbarth@chromium.org

[Nils Barth (inactive), 2014-10-22 14:19:52]

bashi: Minor style nit

BTW, this is quite exciting -- we'd thought about using the proper reader
instead of regexes for these earlier steps, and it's great to see it actually
being done -- thanks!
This really helps with consistency, legibility, and correctness -- we've had
endless subtle bugs with regex-based parsing, often missed for months or years.

https://codereview.chromium.org/670663002/diff/40001/Source/bindings/scripts/...
File Source/bindings/scripts/compute_interfaces_info_individual.py (right):

https://codereview.chromium.org/670663002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/compute_interfaces_info_individual.py:147: if
len(definitions.interfaces) > 0:
Style nit:
Use implicit true/false (truthy/falsy):
https://google-styleguide.googlecode.com/svn/trunk/pyguide.html?showone=True/...

...so this can simply be:
if definitions.interfaces:

https://codereview.chromium.org/670663002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/compute_interfaces_info_individual.py:160: elif
len(definitions.dictionaries) > 0:
Ditto.

[bashi, 2014-10-23 22:53:32]

On 2014/10/22 14:19:52, Nils Barth (inactive) wrote:
> bashi: Minor style nit
> 
> BTW, this is quite exciting -- we'd thought about using the proper reader
> instead of regexes for these earlier steps, and it's great to see it actually
> being done -- thanks!
> This really helps with consistency, legibility, and correctness -- we've had
> endless subtle bugs with regex-based parsing, often missed for months or
years.
> 
>
https://codereview.chromium.org/670663002/diff/40001/Source/bindings/scripts/...
> File Source/bindings/scripts/compute_interfaces_info_individual.py (right):
> 
>
https://codereview.chromium.org/670663002/diff/40001/Source/bindings/scripts/...
> Source/bindings/scripts/compute_interfaces_info_individual.py:147: if
> len(definitions.interfaces) > 0:
> Style nit:
> Use implicit true/false (truthy/falsy):
>
https://google-styleguide.googlecode.com/svn/trunk/pyguide.html?showone=True/...
> 
> ...so this can simply be:
> if definitions.interfaces:
> 
>
https://codereview.chromium.org/670663002/diff/40001/Source/bindings/scripts/...
> Source/bindings/scripts/compute_interfaces_info_individual.py:160: elif
> len(definitions.dictionaries) > 0:
> Ditto.

Thank you! I uploaded a follow-up CL.
https://codereview.chromium.org/678453004