Bug 62904 – Remove LegacyDefaultOptionalArguments flag from IDL files where it would not change behavior

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Mark Pilgrim (Google)]

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.

[Mark Pilgrim (Google)]

Created attachment 97648 [details]
Patch

[Darin Adler]

Comment on attachment 97648 [details]
Patch

=

[WebKit Review Bot]

Comment on attachment 97648 [details]
Patch

Clearing flags on attachment: 97648

Committed r89189: <http://trac.webkit.org/changeset/89189>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

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

[Adam Barth]

Thanks Ossy.  I wonder why the qt ews bot didn't warn us. :(

[Darin Adler]

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

[Adam Barth]

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

[Darin Adler]

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

[Csaba Osztrogonác]

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.

[Mark Pilgrim (Google)]

Created attachment 97832 [details]
Patch

[Mark Pilgrim (Google)]

Combined patch with warning fix. Thanks Ossy for debugging.

[Adam Barth]

Comment on attachment 97832 [details]
Patch

Ok.  Let's give this version a try.

[WebKit Review Bot]

Comment on attachment 97832 [details]
Patch

Clearing flags on attachment: 97832

Committed r89269: <http://trac.webkit.org/changeset/89269>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.