Bug 157060 – [Web IDL] Clean up support for [Clamp] IDL extended attribute

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Chris Dumez]

Clean up support for [Clamp] IDL extended attribute.

[Chris Dumez]

Created attachment 277442 [details]
Patch

[WebKit Commit Bot]

Attachment 277442 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1043:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1058:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:2298:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:2315:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 4 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 277442 [details]
Patch

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

> Source/WebCore/bindings/js/JSDOMBinding.h:375
> +    switch (configuration) {

Note that I tried to keep this patch minimal but I think we should move IntegerConversionConfiguration to be a template parameter instead since all the call sites know the configuration at compile time and this would optimize out the branches.

[Darin Adler]

Comment on attachment 277442 [details]
Patch

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

EFL build is failing, maybe because of a difference in the version of gcc it’s using?

Why no use of this new feature in any IDL file yet?

>> Source/WebCore/bindings/js/JSDOMBinding.h:375
>> +    switch (configuration) {
> 
> Note that I tried to keep this patch minimal but I think we should move IntegerConversionConfiguration to be a template parameter instead since all the call sites know the configuration at compile time and this would optimize out the branches.

Often, inlining and constant folding is enough to optimize out the branches. So maybe we don’t have to do that.

[Chris Dumez]

Comment on attachment 277442 [details]
Patch

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

[Clamp] is used in our IDL although not a lot at the moment.

>>> Source/WebCore/bindings/js/JSDOMBinding.h:375
>>> +    switch (configuration) {
>> 
>> Note that I tried to keep this patch minimal but I think we should move IntegerConversionConfiguration to be a template parameter instead since all the call sites know the configuration at compile time and this would optimize out the branches.
> 
> Often, inlining and constant folding is enough to optimize out the branches. So maybe we don’t have to do that.

Well, but only 2 of them are inlined in the header. All of the others are in the cpp.

[Chris Dumez]

Created attachment 277483 [details]
Patch

[WebKit Commit Bot]

Attachment 277483 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1043:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1058:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:2298:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:2315:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 4 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 277483 [details]
Patch

Clearing flags on attachment: 277483

Committed r200131: <http://trac.webkit.org/changeset/200131>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.