Commented IDL implements statements should not generate code

Commented IDL implements statements should not generate code

Fix preprocess_idls.py script to make sure that commented IDL implements
statements do not cause any code to be generated.

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

[haraken, 2013-06-24 13:00:46]

LGTM.

[commit-bot: I haz the power, 2013-06-24 13:01:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/17591008/1

[arv (Not doing code reviews), 2013-06-24 14:20:14]

LGTM

The IDL files allows /* */ comments too. Maybe we should have a preprocessing
pass that removes all comments?

[do-not-use, 2013-06-24 14:29:04]

On 2013/06/24 14:20:14, arv wrote:
> LGTM
> 
> The IDL files allows /* */ comments too. Maybe we should have a preprocessing
> pass that removes all comments?

The following will behave as expected:
// A implements B;
/* A implements B; */

However, the following would not:
/*
A implements B;
*/

I believe this kind of multiline comments would fool our other regular
expressions as well and this is thankfully not an issue currently.

Trimming the comments from the content before applying any regex is indeed a
viable solution. I seem to remember I did that for preprocessor lines already.
haraken, what do you think?

[haraken, 2013-06-24 14:58:20]

> I believe this kind of multiline comments would fool our other regular
> expressions as well and this is thankfully not an issue currently.
> 
> Trimming the comments from the content before applying any regex is indeed a
> viable solution. I seem to remember I did that for preprocessor lines already.
> haraken, what do you think?

I think we can assume that Blink IDL files have //-style comments only. It
sounds over-engineering to support /**/-style comments in the preprocessor (we
can no longer use regular expressions for performance), given that there would
be no strong reason to support /**/-style comments.

If you agree with the idea, it might be worth adding a warning when the parser
encounters /**/-style comments.

[commit-bot: I haz the power, 2013-06-24 15:09:22]

Change committed as 152950

[do-not-use, 2013-06-24 15:33:42]

On 2013/06/24 14:58:20, haraken wrote:
> > I believe this kind of multiline comments would fool our other regular
> > expressions as well and this is thankfully not an issue currently.
> > 
> > Trimming the comments from the content before applying any regex is indeed a
> > viable solution. I seem to remember I did that for preprocessor lines
already.
> > haraken, what do you think?
> 
> I think we can assume that Blink IDL files have //-style comments only. It
> sounds over-engineering to support /**/-style comments in the preprocessor (we
> can no longer use regular expressions for performance), given that there would
> be no strong reason to support /**/-style comments.
> 
> If you agree with the idea, it might be worth adding a warning when the parser
> encounters /**/-style comments.

Yes, I also think current support is enough and I don't want to make the script
more complex than it currently is. One note though regarding printing warnings
if /**/-style comments are used is that we currently use them for copyright
headers so we would have to detect those headers.

[Nils Barth (inactive), 2013-07-05 11:16:34]

To add 2 cents:
for this dependency resolution (finding 'implements'),
we could use a fast lexer (assuming we want to avoid doing full parsing as too
slow),
namely a lex-generated C lexer, instead of Python regexes.
This would be much faster and more correct, since it would handle multi-line
comments.

lexing is a relatively slow and simple part of parsing (just a few regexes and
token names),
so it's a good place for optimization (full parser in yacc-generated C is much
more work).
Once we land Python lexer+parser, it'll be easy to swap out the Python lexer to
a superfast C lexer
if we want, which'll help with both speeding up parsing IDL files and this
dependency resolution step.

This isn't urgent – preprocess_idls.py isn't *that* slow – but it's worth doing,
since we have to run it every time any IDL file is touched.

(On my "TODO later" list.)