Bug 157060 - [Web IDL] Clean up support for [Clamp] IDL extended attribute
Summary: [Web IDL] Clean up support for [Clamp] IDL extended attribute
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-26 22:03 PDT by Chris Dumez
Modified: 2016-04-27 10:05 PDT (History)
7 users (show)

See Also:


Attachments
Patch (35.32 KB, patch)
2016-04-26 22:09 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (35.18 KB, patch)
2016-04-27 09:20 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-04-26 22:03:40 PDT
Clean up support for [Clamp] IDL extended attribute.
Comment 1 Chris Dumez 2016-04-26 22:09:30 PDT
Created attachment 277442 [details]
Patch
Comment 2 WebKit Commit Bot 2016-04-26 22:10:27 PDT
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.
Comment 3 Chris Dumez 2016-04-27 08:09:17 PDT
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.
Comment 4 Darin Adler 2016-04-27 08:38:27 PDT
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.
Comment 5 Chris Dumez 2016-04-27 09:18:26 PDT
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.
Comment 6 Chris Dumez 2016-04-27 09:20:54 PDT
Created attachment 277483 [details]
Patch
Comment 7 WebKit Commit Bot 2016-04-27 09:22:25 PDT
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.
Comment 8 Chris Dumez 2016-04-27 10:05:11 PDT
Comment on attachment 277483 [details]
Patch

Clearing flags on attachment: 277483

Committed r200131: <http://trac.webkit.org/changeset/200131>
Comment 9 Chris Dumez 2016-04-27 10:05:16 PDT
All reviewed patches have been landed.  Closing bug.