Bug 25612 - Add support for feature conditionals in the generated JSC bindings code
Summary: Add support for feature conditionals in the generated JSC bindings code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-07 01:50 PDT by Simon Hausmann
Modified: 2009-05-25 05:39 PDT (History)
2 users (show)

See Also:


Attachments
Patch for moving feature #ifdefs into generated JSC bindings code (6.58 KB, patch)
2009-05-07 01:52 PDT, Simon Hausmann
no flags Details | Formatted Diff | Diff
Patch to add support for feature #ifdefs in the genertaed tag factory code (4.57 KB, patch)
2009-05-07 01:53 PDT, Simon Hausmann
no flags Details | Formatted Diff | Diff
Patch to add support for feature #ifdefs in the genertaed tag factory code (take 2) (5.95 KB, patch)
2009-05-24 11:17 PDT, Simon Hausmann
vestbo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Hausmann 2009-05-07 01:50:53 PDT
Currently features in IDL files are configured through preprocessor macros. The patches attached to this bug report add support for moving these #ifdefs into the generated code, making the generated code independent from the features are code generation time.
Comment 1 Simon Hausmann 2009-05-07 01:52:20 PDT
Created attachment 30093 [details]
Patch for moving feature #ifdefs into generated JSC bindings code

This patch adds support for the conditional attribute for IDL attributes, which places feature #ifdefs into the generated code, replacing #ifdefs in the IDL files.
Comment 2 Simon Hausmann 2009-05-07 01:53:29 PDT
Created attachment 30094 [details]
Patch to add support for feature #ifdefs in the genertaed tag factory code

This patch allows moving #ifdefs in the tagnames.in files into the generated tag factory code.
Comment 3 Simon Hausmann 2009-05-07 02:02:24 PDT
Comment on attachment 30094 [details]
Patch to add support for feature #ifdefs in the genertaed tag factory code

Clearing review, this patch doesn't work entirely anymore. Will update soon.
Comment 4 Maciej Stachowiak 2009-05-21 20:06:08 PDT
It might be good for Sam to look at this. Sounds like a good idea.
Comment 5 Eric Seidel (no email) 2009-05-21 20:21:46 PDT
Won't we still end up re-generating the code every time you change FEATURE_DEFINES?  We'll certainly do a full recompile (at least in xcode/gcc land), so this savings seems minimal.  It's not necessarily a bad idea though.
Comment 6 Simon Hausmann 2009-05-22 08:50:22 PDT
(In reply to comment #5)
> Won't we still end up re-generating the code every time you change
> FEATURE_DEFINES?  We'll certainly do a full recompile (at least in xcode/gcc
> land), so this savings seems minimal.  It's not necessarily a bad idea though.

Yes, that is currently the case. However my long term goal is to have generated files that can be used with different configurations without the need to re-generate them. The generation can take a long time on older platforms :).

We're _almost_ there with the existing #ifdef's that exist for example for the SVG files. AFAICS it's just a few IDL files that need tweaking :)
Comment 7 Maciej Stachowiak 2009-05-22 21:31:47 PDT
Comment on attachment 30093 [details]
Patch for moving feature #ifdefs into generated JSC bindings code

Looks good.
Comment 8 Simon Hausmann 2009-05-24 11:11:37 PDT
Comment on attachment 30093 [details]
Patch for moving feature #ifdefs into generated JSC bindings code

Thanks for the review, landed this one in r44116. Keeping the bug open though because of the pending tags patch.
Comment 9 Simon Hausmann 2009-05-24 11:17:06 PDT
Created attachment 30631 [details]
Patch to add support for feature #ifdefs in the genertaed tag factory code (take 2)
Comment 10 Laszlo Gombos 2009-05-24 12:41:30 PDT
(In reply to comment #8)
> (From update of attachment 30093 [details] [review])
> Thanks for the review, landed this one in r44116. Keeping the bug open though
> because of the pending tags patch.
> 

JSDOMWindow.cpp fails to build for me, when VIDEO is off after changeset 44116.

generated/release/JSDOMWindow.cpp:91:32: error: JSHTMLAudioElement.h: No such file or directory
generated/release/JSDOMWindow.cpp:125:32: error: JSHTMLMediaElement.h: No such file or directory
generated/release/JSDOMWindow.cpp:149:32: error: JSHTMLVideoElement.h: No such file or directory
generated/release/JSDOMWindow.cpp:151:26: error: JSMediaError.h: No such file or directory

#include's might need guards as well.
Comment 11 Simon Hausmann 2009-05-25 05:36:35 PDT
(In reply to comment #10)
> (In reply to comment #8)
> > (From update of attachment 30093 [details] [review] [review])
> > Thanks for the review, landed this one in r44116. Keeping the bug open though
> > because of the pending tags patch.
> > 
> 
> JSDOMWindow.cpp fails to build for me, when VIDEO is off after changeset 44116.
> 
> generated/release/JSDOMWindow.cpp:91:32: error: JSHTMLAudioElement.h: No such
> file or directory
> generated/release/JSDOMWindow.cpp:125:32: error: JSHTMLMediaElement.h: No such
> file or directory
> generated/release/JSDOMWindow.cpp:149:32: error: JSHTMLVideoElement.h: No such
> file or directory
> generated/release/JSDOMWindow.cpp:151:26: error: JSMediaError.h: No such file
> or directory
> 
> #include's might need guards as well.
> 

Well spotted, fixed in r44126.
Comment 12 Simon Hausmann 2009-05-25 05:39:25 PDT
Landed in r44128