Bug 81345 - IDLParser.pm should support sequence<T> type
Summary: IDLParser.pm should support sequence<T> type
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 80696
  Show dependency treegraph
 
Reported: 2012-03-16 06:28 PDT by Kentaro Hara
Modified: 2012-03-16 20:15 PDT (History)
5 users (show)

See Also:


Attachments
Patch (34.00 KB, patch)
2012-03-16 06:36 PDT, Kentaro Hara
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2012-03-16 06:28:01 PDT
Currently IDLParser.pm cannot understand sequence<T> type. We should support it. This is a preparing patch for fixing 80696.
Comment 1 Kentaro Hara 2012-03-16 06:36:54 PDT
Created attachment 132270 [details]
Patch
Comment 2 WebKit Review Bot 2012-03-16 06:39:58 PDT
Attachment 132270 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:154:  Extra space before )  [whitespace/parens] [2]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:63:  "webkit/WebKitDOMsequence<ScriptProfile" already included at Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:62  [build/include] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:175:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:188:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:819:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:832:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 6 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Kentaro Hara 2012-03-16 06:39:59 PDT
Comment on attachment 132270 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=132270&action=review

> Source/WebCore/bindings/scripts/IDLStructure.pm:106
> +our $interfaceSelector = 'interface\s*((?:' . $extendedAttributeSyntax . ' )?)(' . $idlIdNs . '*)\s*(?::(\s*[^{]*))?{([-a-zA-Z0-9_"=\s(),;:\[\]<>&\|]*)';

It took me one hour to notice this change...:-) Maybe it is time to refactor the IDL parser to a recursive parser.
Comment 4 Adam Barth 2012-03-16 06:56:40 PDT
Comment on attachment 132270 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=132270&action=review

I suspect you need to update the CodeGenerator pm files to do something sane with this type.  Maybe the way we handle Foo[] is a good example to build from there?

> Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:46
> +#include "JSsequence<ScriptProfile>.h"

This looks pretty broken.

> Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp:55
> +#include "V8sequence<ScriptProfile>.h"

This too.
Comment 5 Adam Barth 2012-03-16 06:57:03 PDT
I see.  You're just going to do the code generation parts later....   Ok.
Comment 6 Adam Barth 2012-03-16 06:57:49 PDT
Comment on attachment 132270 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=132270&action=review

>> Source/WebCore/bindings/scripts/IDLStructure.pm:106
>> +our $interfaceSelector = 'interface\s*((?:' . $extendedAttributeSyntax . ' )?)(' . $idlIdNs . '*)\s*(?::(\s*[^{]*))?{([-a-zA-Z0-9_"=\s(),;:\[\]<>&\|]*)';
> 
> It took me one hour to notice this change...:-) Maybe it is time to refactor the IDL parser to a recursive parser.

We have a recursive version in Python, but I'm not sure we want to re-write everything in Python...
Comment 7 Kentaro Hara 2012-03-16 06:58:23 PDT
(In reply to comment #5)
> I see.  You're just going to do the code generation parts later....   Ok.

Yes, I was just asked to fix the parser side.
Comment 8 Kentaro Hara 2012-03-16 07:01:32 PDT
Committed r111004: <http://trac.webkit.org/changeset/111004>
Comment 9 Vineet Chaudhary (vineetc) 2012-03-16 20:15:03 PDT
(In reply to comment #7)
> (In reply to comment #5)
> > I see.  You're just going to do the code generation parts later....   Ok.
> 
> Yes, I was just asked to fix the parser side.

Thank you haraken for the fix.
I am to provide codegenerator fix.