IDL compiler: [Default] arguments

IDL compiler: [Default] arguments

V. widely used.
Long-term we should instead use the Web IDL standard:
optional type identifier = value
instead of
[Default=value] optional type identifier
...which is much cleaner and clearer,
but that requires parser support and isn't worth doing in Perl.

Docs:
http://www.w3.org/TR/WebIDL/#dfn-optional-argument-default-value
https://sites.google.com/a/chromium.org/dev/blink/webidl/blink-idl-extended-attributes#TOC-Default-p-

BUG=239771
NOTRY=true

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

[Nils Barth (inactive), 2013-10-31 05:52:20]

https://codereview.chromium.org/47023015/diff/1/Source/bindings/scripts/code_...
File Source/bindings/scripts/code_generator_v8.pm (left):

https://codereview.chromium.org/47023015/diff/1/Source/bindings/scripts/code_...
Source/bindings/scripts/code_generator_v8.pm:2457: # Optional arguments with
[Optional=...] should not generate the early call.
Outdated comment ([Optional] no longer exists).

[haraken, 2013-10-31 06:05:14]

LGTM

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

https://codereview.chromium.org/47023015/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_methods.py:126: v8_value =
'argumentOrNull(args, %s)' % index

We might want add an ASSERT to verify that argument.idl_type is 'DOMString'.
[Default=NullString] works only for DOMString parameters.

[Nils Barth (inactive), 2013-10-31 06:26:27]

Thanks!
(Comment about adding checks/asserts.)

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

https://codereview.chromium.org/47023015/diff/1/Source/bindings/scripts/unsta...
Source/bindings/scripts/unstable/v8_methods.py:126: v8_value =
'argumentOrNull(args, %s)' % index
On 2013/10/31 06:05:14, haraken wrote:
> We might want add an ASSERT to verify that argument.idl_type is 'DOMString'.
> [Default=NullString] works only for DOMString parameters.

(In this case I'll explicitly add a check for IDL type.)

I've been avoiding putting validation into the CG,
since there are a huge number of actual constraints
(if we implemented all of the MUST and MUST NOT in the spec),
it clutters the code a lot, and I'm not sure there's actual benefit.

For example, to do this properly we'd need to make it into an auxiliary function
(v8_value) and write:
if 'Default' in extended_attributes:
    if not argument.is_optional:
        raise 'Can only use [Default] on optional arguments'
    if extended_attributes['Default'] == 'NullString':
        if argument.idl_type != 'DOMString':
            raise 'Can only use [Default=NullString] on DOMString arguments'
        return 'argumentOrNull(args, %s)' % index
return 'args[%s]' % index

...which is a lot of overhead, and obscures the main point,
which is |args[%s]| vs. |argumentOrNull(args, %s)|.

We *do* validate extended attribute names and values, since these
have been a source of bugs (since silently ignored if invalid),
but other ones generally just make the code fail, right?

Longer-term, we could have a separate validation step that checks
compatibility of extended attributes with IDL types etc.,
which gives us the benefits without cluttering the CG,
and we could even run it separately (say as PRESUBMIT),
which should speed up the build some.

How does this sound?
(I'm happy to add checks if it's useful, but I want to avoid gratuitous cruft.)

[commit-bot: I haz the power, 2013-10-31 06:27:34]

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

[commit-bot: I haz the power, 2013-10-31 06:27:47]

Change committed as 161026

[haraken, 2013-10-31 06:34:26]

> How does this sound?
> (I'm happy to add checks if it's useful, but I want to avoid gratuitous
cruft.)

Makes sense. We don't want to mess up the code generator just for validation. On
the other hand, we might want to add some validation for common mistakes (e.g.,
[Default=NullString] for DOMString parameters). Either way, it's OK to not have
the validation for now.

[Nils Barth (inactive), 2013-10-31 07:09:02]

On 2013/10/31 06:34:26, haraken wrote:
> Makes sense. We don't want to mess up the code generator just for validation.
On
> the other hand, we might want to add some validation for common mistakes
(e.g.,
> [Default=NullString] for DOMString parameters). Either way, it's OK to not
have
> the validation for now.

Got it!