Bug 108565 - [CPP,GObject,ObjC] Add 'enum' skip to CodeGenerator{CPP,GObject,ObjC}.pm
Summary: [CPP,GObject,ObjC] Add 'enum' skip to CodeGenerator{CPP,GObject,ObjC}.pm
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-31 19:03 PST by Nils Barth
Modified: 2013-01-31 23:09 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.44 KB, patch)
2013-01-31 21:24 PST, Nils Barth
no flags Details | Formatted Diff | Diff
Patch (5.57 KB, patch)
2013-01-31 21:57 PST, Nils Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nils Barth 2013-01-31 19:03:32 PST
Make legacy code generators skip enum types (added to V8 CodeGen in Bug 106553).
This allows us to not use macros like "#if defined(TESTING_JS) || defined(TESTING_V8)" in IDL files.
Concretely it's just adding an IsEnumType check to the SkipAttribute functions, and then removing macros from the test IDL file.
Comment 1 Nils Barth 2013-01-31 21:24:32 PST
Created attachment 185936 [details]
Patch

Simple patch -- adds IsEnumType test to SkipAttribute in legacy code generators
so we can remove #if macro from IDL test file.
Also minor associated code cleaning (b/c we're checking type):
aux var: $type = $attribute->signature->type
(is this ok?)
Comment 2 Kentaro Hara 2013-01-31 21:32:52 PST
Comment on attachment 185936 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Add IsEnumType test to SkipAttribute in legacy code generators.

Please explain the rationale for the change, not only what you did but also why you did it.

e.g. Since CodeGenerator{CPP,GObject,ObjC}.pm does not support 'enum', this patch adds code to skip DOM attributes that return 'enum'.

> Source/WebCore/bindings/scripts/test/TestObj.idl:47
>      static readonly attribute long     staticReadOnlyLongAttr;
>      static attribute DOMString         staticStringAttr;
>      static readonly attribute TestSubObjConstructor TestSubObj;
> -    attribute TestEnumType             enumAttr;
>  #endif

I think you can remove this #if too by skipping 'static' in CodeGenerator{ObjC,GObject,CPP}.pm. You can do it in a follow-up patch.
Comment 3 Nils Barth 2013-01-31 21:35:08 PST
(In reply to comment #2)
> (From update of attachment 185936 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=185936&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Add IsEnumType test to SkipAttribute in legacy code generators.
> 
> Please explain the rationale for the change, not only what you did but also why you did it.
> 
> e.g. Since CodeGenerator{CPP,GObject,ObjC}.pm does not support 'enum', this patch adds code to skip DOM attributes that return 'enum'.

Got it -- will update ChangeLog.

> > Source/WebCore/bindings/scripts/test/TestObj.idl:47
> >      static readonly attribute long     staticReadOnlyLongAttr;
> >      static attribute DOMString         staticStringAttr;
> >      static readonly attribute TestSubObjConstructor TestSubObj;
> > -    attribute TestEnumType             enumAttr;
> >  #endif
> 
> I think you can remove this #if too by skipping 'static' in CodeGenerator{ObjC,GObject,CPP}.pm. You can do it in a follow-up patch.

I was wondering about that; should I open a different bug for that?
Comment 4 Kentaro Hara 2013-01-31 21:36:47 PST
> > I think you can remove this #if too by skipping 'static' in CodeGenerator{ObjC,GObject,CPP}.pm. You can do it in a follow-up patch.
> 
> I was wondering about that; should I open a different bug for that?

Yes.
Comment 5 Nils Barth 2013-01-31 21:44:16 PST
(In reply to comment #4)
> > > I think you can remove this #if too by skipping 'static' in CodeGenerator{ObjC,GObject,CPP}.pm. You can do it in a follow-up patch.
> > 
> > I was wondering about that; should I open a different bug for that?
> 
> Yes.

Got it: Bug 108578
Comment 6 Nils Barth 2013-01-31 21:57:17 PST
Created attachment 185941 [details]
Patch

(In reply to comment #2)
> (From update of attachment 185936 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=185936&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Add IsEnumType test to SkipAttribute in legacy code generators.
> 
> Please explain the rationale for the change, not only what you did but also why you did it.
> 
> e.g. Since CodeGenerator{CPP,GObject,ObjC}.pm does not support 'enum', this patch adds code to skip DOM attributes that return 'enum'.
Got it -- fixed.
Comment 7 Kentaro Hara 2013-01-31 21:58:32 PST
Comment on attachment 185941 [details]
Patch

LGTM
Comment 8 WebKit Review Bot 2013-01-31 23:09:07 PST
Comment on attachment 185941 [details]
Patch

Clearing flags on attachment: 185941

Committed r141541: <http://trac.webkit.org/changeset/141541>
Comment 9 WebKit Review Bot 2013-01-31 23:09:12 PST
All reviewed patches have been landed.  Closing bug.