IDL compiler: [Constructor] argument types

IDL compiler: [Constructor] argument types

This adds support for arguments of all types.

This uses the existing method.cpp template, but requires one change,
namely returning failedToConstruct instead of failedToExecute
(which also have different signatures), so factoring out a macro for this,
as it occurs in 4 places in the argument check.

BUG=239771
NOTRY=true

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

[Nils Barth (inactive), 2013-12-12 08:56:07]

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

https://codereview.chromium.org/99323009/diff/1/Source/bindings/templates/met...
Source/bindings/templates/methods.cpp:113: {% macro
throw_type_error(error_message) %}
Nested so don't have to pass context explicitly.

[haraken, 2013-12-12 09:10:26]

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/scripts/u...
File Source/bindings/scripts/unstable/v8_interface.py (right):

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/scripts/u...
Source/bindings/scripts/unstable/v8_interface.py:117: 'constructor_method': {

Where is it used?

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
Source/bindings/templates/interface.cpp:132: {% from 'methods.cpp' import
generate_argument with context %}

What is this change?

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
File Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
Source/bindings/templates/methods.cpp:113: {% macro
throw_type_error(error_message) %}

IMHO, I'm not that happy with making macros for these trivial statements. Since
the template file is already complicated, we might not want to use unnecessary
indirection.

[Nils Barth (inactive), 2013-12-12 09:34:12]

(replies)

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/scripts/u...
File Source/bindings/scripts/unstable/v8_interface.py (right):

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/scripts/u...
Source/bindings/scripts/unstable/v8_interface.py:117: 'constructor_method': {
On 2013/12/12 09:10:27, haraken wrote:
> 
> Where is it used?

In method.cpp, to select between failedToConstruct and failedToExecute.

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
Source/bindings/templates/interface.cpp:132: {% from 'methods.cpp' import
generate_argument with context %}
On 2013/12/12 09:10:27, haraken wrote:
> What is this change?

|generate_argument| needs |interface_name| when throwing errors;
by default macros can't access the calling scope.

We always import with context (in interface_base.cpp,
and now in interface.cpp), to avoid lots of parameter passing;
I just didn't include it at first b/c it wasn't necessary,
but now it is.

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
File Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
Source/bindings/templates/methods.cpp:113: {% macro
throw_type_error(error_message) %}
On 2013/12/12 09:10:27, haraken wrote:
> IMHO, I'm not that happy with making macros for these trivial statements.
Since
> the template file is already complicated, we might not want to use unnecessary
> indirection.

Understood, but in this case I think it makes it a lot simpler.

I wouldn't do it just for the
  throwTypeError(... failedToExecute ...)
call (even though there's lots of redundancy there),
which is why we didn't have it earlier.

However, in this case it's replacing 4 if blocks:
otherwise we'd need an {% if is_constructor %}...{% else %}...{% endif %} block
at each place, each 5 lines long.

Normally I'd try to have a {% set %} statement for these trivial switches,
but it takes a parameter (the error message), hence a macro.

If you feel strongly about this I can replace the macro calls with if blocks.

[haraken, 2013-12-12 09:39:51]

On 2013/12/12 09:34:12, Nils Barth wrote:
> (replies)
> 
>
https://codereview.chromium.org/99323009/diff/20001/Source/bindings/scripts/u...
> File Source/bindings/scripts/unstable/v8_interface.py (right):
> 
>
https://codereview.chromium.org/99323009/diff/20001/Source/bindings/scripts/u...
> Source/bindings/scripts/unstable/v8_interface.py:117: 'constructor_method': {
> On 2013/12/12 09:10:27, haraken wrote:
> > 
> > Where is it used?
> 
> In method.cpp, to select between failedToConstruct and failedToExecute.
> 
>
https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
> File Source/bindings/templates/interface.cpp (right):
> 
>
https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
> Source/bindings/templates/interface.cpp:132: {% from 'methods.cpp' import
> generate_argument with context %}
> On 2013/12/12 09:10:27, haraken wrote:
> > What is this change?
> 
> |generate_argument| needs |interface_name| when throwing errors;
> by default macros can't access the calling scope.
> 
> We always import with context (in interface_base.cpp,
> and now in interface.cpp), to avoid lots of parameter passing;
> I just didn't include it at first b/c it wasn't necessary,
> but now it is.
> 
>
https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
> File Source/bindings/templates/methods.cpp (right):
> 
>
https://codereview.chromium.org/99323009/diff/20001/Source/bindings/templates...
> Source/bindings/templates/methods.cpp:113: {% macro
> throw_type_error(error_message) %}
> On 2013/12/12 09:10:27, haraken wrote:
> > IMHO, I'm not that happy with making macros for these trivial statements.
> Since
> > the template file is already complicated, we might not want to use
unnecessary
> > indirection.
> 
> Understood, but in this case I think it makes it a lot simpler.
> 
> I wouldn't do it just for the
>   throwTypeError(... failedToExecute ...)
> call (even though there's lots of redundancy there),
> which is why we didn't have it earlier.
> 
> However, in this case it's replacing 4 if blocks:
> otherwise we'd need an {% if is_constructor %}...{% else %}...{% endif %}
block
> at each place, each 5 lines long.
> 
> Normally I'd try to have a {% set %} statement for these trivial switches,
> but it takes a parameter (the error message), hence a macro.
> 
> If you feel strongly about this I can replace the macro calls with if blocks.

LGTM, although I'm neutral about the macro. I'm fine with using macros for
non-trivial blocks, but finer macros make the template code less readable.

[Nils Barth (inactive), 2013-12-12 09:52:26]

On 2013/12/12 09:39:51, haraken wrote:
> LGTM, although I'm neutral about the macro. I'm fine with using macros for
> non-trivial blocks, but finer macros make the template code less readable.

Got it; I'll try to avoid them or see how we can eliminate them when possible.
(Hopefully won't be much template refactoring after this, as most code already
being produced.)

[commit-bot: I haz the power, 2013-12-12 09:52:40]

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

[commit-bot: I haz the power, 2013-12-12 09:52:53]

Change committed as 163773