Bug 74068 - Fixing support for static conditional overloaded functions
Summary: Fixing support for static conditional overloaded functions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: http://www.w3.org/TR/WebIDL/#idl-over...
Keywords:
Depends on:
Blocks: 71968 73365
  Show dependency treegraph
 
Reported: 2011-12-08 02:01 PST by Kaustubh Atrawalkar
Modified: 2011-12-08 10:55 PST (History)
6 users (show)

See Also:


Attachments
Patch (16.30 KB, patch)
2011-12-08 02:05 PST, Kaustubh Atrawalkar
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kaustubh Atrawalkar 2011-12-08 02:01:02 PST
Need to fix the support for static conditional overloaded functions in JSC. For all other frameworks its working.
When overloaded functions are conditional, there are two local static functions are created. The bindings generator script pushes this "static" keyword first and then pushes "#if {conditional}" later which causes incorrect bindings being generated.
Comment 1 Kaustubh Atrawalkar 2011-12-08 02:05:07 PST
Created attachment 118350 [details]
Patch
Comment 2 Adam Barth 2011-12-08 09:42:37 PST
Comment on attachment 118350 [details]
Patch

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

> Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp:909
> +#if ENABLE(Condition1)

It would be nice to have a blank line after this line, but that's really a minor nit.  :)
Comment 3 Kaustubh Atrawalkar 2011-12-08 09:56:19 PST
(In reply to comment #2)
> (From update of attachment 118350 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=118350&action=review
> 
> > Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp:909
> > +#if ENABLE(Condition1)
> 
> It would be nice to have a blank line after this line, but that's really a minor nit.  :)

Thanks Adam for quick review :)
Comment 4 Erik Arvidsson 2011-12-08 10:04:28 PST
Comment on attachment 118350 [details]
Patch

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

This does not do V8. I'm fine without V8 support in this patch. I can add it in another patch.

> Source/WebCore/bindings/scripts/test/TestObj.idl:171
> +        static [Conditional=Condition1] void overloadedMethod1(in long arg);
> +        static [Conditional=Condition1] void overloadedMethod1(in DOMString type);

Can this be called overloadedClassMethod instead?

> Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp:1288
>  
> +static v8::Handle<v8::Value> overloadedMethod11Callback(const v8::Arguments& args)

This needs #if ENABLE(Condition1) too.
Comment 5 WebKit Review Bot 2011-12-08 10:55:20 PST
Comment on attachment 118350 [details]
Patch

Clearing flags on attachment: 118350

Committed r102351: <http://trac.webkit.org/changeset/102351>
Comment 6 WebKit Review Bot 2011-12-08 10:55:25 PST
All reviewed patches have been landed.  Closing bug.