Bug 62904 - Remove LegacyDefaultOptionalArguments flag from IDL files where it would not change behavior
Summary: Remove LegacyDefaultOptionalArguments flag from IDL files where it would not ...
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: Nobody
URL:
Keywords:
Depends on: 62925
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-17 13:01 PDT by Mark Pilgrim (Google)
Modified: 2011-06-20 12:22 PDT (History)
6 users (show)

See Also:


Attachments
Patch (171.28 KB, patch)
2011-06-17 13:53 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
warning fix (1.37 KB, patch)
2011-06-20 03:53 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (172.71 KB, patch)
2011-06-20 11:55 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2011-06-17 13:01:07 PDT
After bug 62750, there are many IDL files that contain the new LegacyDefaultOptionalArguments flag that don't actually need it. Some examples:

- the IDL file contains no functions
- the IDL file contains only functions with no arguments
- the IDL file contains functions, *all* of which use the [RequiresAllArguments=raise] extended attribute

This patch simplifies such IDL files by removing the LegacyDefaultOptionalArguments flag and (if needed) removing the [RequiresAllArguments=raise] extended attribute from each function declaration. This patch does not make any required arguments optional or any optional arguments required. It changes no behavior at all.
Comment 1 Mark Pilgrim (Google) 2011-06-17 13:53:08 PDT
Created attachment 97648 [details]
Patch
Comment 2 Darin Adler 2011-06-17 17:42:31 PDT
Comment on attachment 97648 [details]
Patch

=
Comment 3 WebKit Review Bot 2011-06-17 23:31:09 PDT
Comment on attachment 97648 [details]
Patch

Clearing flags on attachment: 97648

Committed r89189: <http://trac.webkit.org/changeset/89189>
Comment 4 WebKit Review Bot 2011-06-17 23:31:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Csaba Osztrogonác 2011-06-18 01:31:08 PDT
Reopen, because it broke Qt build and was rolled out by http://trac.webkit.org/changeset/89190

cc1plus: warnings being treated as errors
../../WebCore/generated/JSDOMCoreException.cpp: In function ‘JSC::EncodedJSValue WebCore::jsDOMCoreExceptionPrototypeFunctionToString(JSC::ExecState*)’:
../../WebCore/generated/JSDOMCoreException.cpp:265: error: comparison of unsigned expression < 0 is always false
Comment 6 Csaba Osztrogonác 2011-06-18 01:39:06 PDT
It broke other bots too, not only the Qt.

Please respect contributing rules try to keep the tree green:
http://www.webkit.org/coding/contributing.html

"Your change must at least compile on all platforms."
Comment 7 Adam Barth 2011-06-18 04:34:54 PDT
Thanks Ossy.  I wonder why the qt ews bot didn't warn us. :(
Comment 8 Darin Adler 2011-06-18 13:13:35 PDT
(In reply to comment #7)
> Thanks Ossy.  I wonder why the qt ews bot didn't warn us. :(

What about the commit bot? Is this happening because the commit bot now uses V8 instead of JSC?
Comment 9 Adam Barth 2011-06-18 22:05:43 PDT
> What about the commit bot? Is this happening because the commit bot now uses V8 instead of JSC?

As the patch built fine on gtk and win, it seems more likely to be a compiler difference than a JSC/V8 difference.
Comment 10 Darin Adler 2011-06-19 11:32:57 PDT
(In reply to comment #9)
> > What about the commit bot? Is this happening because the commit bot now uses V8 instead of JSC?
> 
> As the patch built fine on gtk and win, it seems more likely to be a compiler difference than a JSC/V8 difference.

It’s a combination of both. A compiler warning in JSC-generated code.
Comment 11 Csaba Osztrogonác 2011-06-20 03:53:37 PDT
Created attachment 97776 [details]
warning fix

"if (exec->argumentCount() < 0" was incorrect, because it is always false.
With the attached fix non-V8 builds will be happy.
Comment 12 Mark Pilgrim (Google) 2011-06-20 11:55:31 PDT
Created attachment 97832 [details]
Patch
Comment 13 Mark Pilgrim (Google) 2011-06-20 11:57:48 PDT
Combined patch with warning fix. Thanks Ossy for debugging.
Comment 14 Adam Barth 2011-06-20 11:57:57 PDT
Comment on attachment 97832 [details]
Patch

Ok.  Let's give this version a try.
Comment 15 WebKit Review Bot 2011-06-20 12:22:47 PDT
Comment on attachment 97832 [details]
Patch

Clearing flags on attachment: 97832

Committed r89269: <http://trac.webkit.org/changeset/89269>
Comment 16 WebKit Review Bot 2011-06-20 12:22:53 PDT
All reviewed patches have been landed.  Closing bug.