IDL compiler: [Clamp] for arguments

IDL compiler: [Clamp] for arguments

In practice used 2 places, for unsigned short and unsigned long.

One weird point (hence documented) is that here we use the literal IDL
type for a C++ variable.

I believe this is ok b/c |unsigned short| and |unsigned long| are legitimate
C++ types, even though we otherwise internally store them as |unsigned|,
but it does mean there's implicit type conversion going on when the
local variable is used later (since it's unsigned).

It might be more correct and safer to use the C++ type for the local variable,
only using the IDL type for the clampTo itself, and making the conversion
explicit here.

Docs:
http://www.w3.org/TR/WebIDL/#Clamp
http://www.chromium.org/blink/webidl/blink-idl-extended-attributes#TOC-Clamp-a-p-

BUG=239771
NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161016

[Nils Barth (inactive), 2013-10-31 03:22:44]

Straightforward, but I've a question about C++ types vs. IDL type;
see description and inline comments.

https://codereview.chromium.org/53483004/diff/1/Source/bindings/scripts/unsta...
File Source/bindings/scripts/unstable/v8_methods.py (right):

https://codereview.chromium.org/53483004/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_methods.py:62: extended_attributes =
argument.extended_attributes
We're going to be using this for the rest of the extended attributes.

https://codereview.chromium.org/53483004/diff/1/Source/bindings/templates/met...
File Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/53483004/diff/1/Source/bindings/templates/met...
Source/bindings/templates/methods.cpp:21: {{argument.idl_type}}
{{argument.name}} = 0;
We might want to use |cpp_type| here, so it's the type we assume later.

https://codereview.chromium.org/53483004/diff/1/Source/bindings/templates/met...
Source/bindings/templates/methods.cpp:24: {{argument.name}} =
clampTo<{{argument.idl_type}}>({{argument.name}}NativeValue);
OTOH, we need |idl_type| here for the actual clamping.

[haraken, 2013-10-31 03:28:43]

LGTM

[Nils Barth (inactive), 2013-10-31 04:09:52]

On 2013/10/31 03:22:44, Nils Barth wrote:
> Straightforward, but I've a question about C++ types vs. IDL type;
> see description and inline comments.

Discussed offline; will fix in followup CL so casting happens explicitly
at the start, rather than implicitly later.

[Nils Barth (inactive), 2013-10-31 04:18:19]

https://codereview.chromium.org/53483004/diff/1/Source/bindings/templates/met...
File Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/53483004/diff/1/Source/bindings/templates/met...
Source/bindings/templates/methods.cpp:21: {{argument.idl_type}}
{{argument.name}} = 0;
On 2013/10/31 03:22:44, Nils Barth wrote:
> We might want to use |cpp_type| here, so it's the type we assume later.

I've put a FIXME here.

https://codereview.chromium.org/53483004/diff/1/Source/bindings/templates/met...
Source/bindings/templates/methods.cpp:24: {{argument.name}} =
clampTo<{{argument.idl_type}}>({{argument.name}}NativeValue);
On 2013/10/31 03:22:44, Nils Barth wrote:
> OTOH, we need |idl_type| here for the actual clamping.

I've put a comment here.

[commit-bot: I haz the power, 2013-10-31 04:19:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/53483004/80001

[commit-bot: I haz the power, 2013-10-31 04:20:23]

Change committed as 161016