Bug 108565 – [CPP,GObject,ObjC] Add 'enum' skip to CodeGenerator{CPP,GObject,ObjC}.pm

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Nils Barth]

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.

[Nils Barth]

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?)

[Kentaro Hara]

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.

[Nils Barth]

(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?

[Kentaro Hara]

> > 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.

[Nils Barth]

(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

[Nils Barth]

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.

[Kentaro Hara]

Comment on attachment 185941 [details]
Patch

LGTM

[WebKit Review Bot]

Comment on attachment 185941 [details]
Patch

Clearing flags on attachment: 185941

Committed r141541: <http://trac.webkit.org/changeset/141541>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.