Decouple arity errors creation from throwing.

Source/bindings/templates/methods.cpp

Index: Source/bindings/templates/methods.cpp
diff --git a/Source/bindings/templates/methods.cpp b/Source/bindings/templates/methods.cpp
index 9aa74a8ce90f8eb0f665851643932da20ece15f3..a69f71461eeade9c14c0fefe7e4c8ac037c9f48c 100644
--- a/Source/bindings/templates/methods.cpp
+++ b/Source/bindings/templates/methods.cpp
@@ -10,7 +10,7 @@ static void {{method.name}}{{method.overload_index}}Method{{world_suffix}}(const
     {# Overloaded methods have length checked during overload resolution #}
     {% if method.number_of_required_arguments and not method.overload_index %}
     if (UNLIKELY(info.Length() < {{method.number_of_required_arguments}})) {
-        {{throw_minimum_arity_type_error(method, method.number_of_required_arguments)}};
+        {{throw_minimum_arity_type_error(method, method.number_of_required_arguments) | indent(8)}}
         return;
     }
     {% endif %}
@@ -324,13 +324,13 @@ exceptionState.throwTypeError({{error_message}});
 {% elif method.is_constructor %}
 {% if method.idl_type == 'Promise' %}

[Jens Widell, 2014/08/12 07:18:03]

This if/else block is repeated twice in this macro and once in
throw_minimum_arity_type_error(). Could we add a utility macro for it that takes
the method and a "create exception" expression as arguments?

[yhirano, 2014/08/12 07:43:19]

Done.

[Jens Widell, 2014/08/12 08:42:26]

I meant the "if promise, set return value, else throw" block. But this was also
a nice  improvement. And what I was going for isn't complete straight-forward.

 {# FIXME: reduce code duplication between sync / async exception handling. #}
-v8SetReturnValue(info, ScriptPromise::rejectWithTypeError(ScriptState::current(info.GetIsolate()), ExceptionMessages::failedToConstruct("{{interface_name}}", {{error_message}})).v8Value());
+v8SetReturnValue(info, ScriptPromise::rejecRaw(info.GetIsolate(), V8ThrowException::createTypeError(ExceptionMessages::failedToConstruct("{{interface_name}}", {{error_message}}), info.GetIsolate())));
 {% else %}
 V8ThrowException::throwTypeError(ExceptionMessages::failedToConstruct("{{interface_name}}", {{error_message}}), info.GetIsolate());
 {% endif %}
 {% else %}{# method.has_exception_state #}
 {% if method.idl_type == 'Promise' %}
-v8SetReturnValue(info, ScriptPromise::rejectWithTypeError(ScriptState::current(info.GetIsolate()), ExceptionMessages::failedToExecute("{{method.name}}", "{{interface_name}}", {{error_message}})).v8Value());
+v8SetReturnValue(info, ScriptPromise::rejectRaw(info.GetIsolate(), V8ThrowException::createTypeError(ExceptionMessages::failedToExecute("{{method.name}}", "{{interface_name}}", {{error_message}}), info.GetIsolate())));
 {% else %}
 V8ThrowException::throwTypeError(ExceptionMessages::failedToExecute("{{method.name}}", "{{interface_name}}", {{error_message}}), info.GetIsolate());
 {% endif %}
@@ -349,47 +349,28 @@ exceptionState.throwIfNeeded()
 
 
 {######################################}
-{% macro throw_arity_type_error(method, valid_arities) %}
-{% if method.idl_type == 'Promise' %}
-{% if method.has_exception_state %}
-v8SetReturnValue(info, ScriptPromise::rejectWithArityTypeError(ScriptState::current(info.GetIsolate()), exceptionState, {{valid_arities}}, info.Length()).v8Value())
-{%- elif method.is_constructor %}
-v8SetReturnValue(info, ScriptPromise::rejectWithArityTypeErrorForConstructor(ScriptState::current(info.GetIsolate()), "{{interface_name}}", {{valid_arities}}, info.Length()).v8Value())
-{%- else %}
-v8SetReturnValue(info, ScriptPromise::rejectWithArityTypeErrorForMethod(ScriptState::current(info.GetIsolate()), "{{method.name}}", "{{interface_name}}", {{valid_arities}}, info.Length()).v8Value())
-{%- endif %}
-{%- else %}{# methods.idl_type == 'Promise' #}
-{% if method.has_exception_state %}
-throwArityTypeError(exceptionState, {{valid_arities}}, info.Length())
-{%- elif method.is_constructor %}
-throwArityTypeErrorForConstructor("{{interface_name}}", {{valid_arities}}, info.Length(), info.GetIsolate())
+{% macro create_minimum_arity_type_error_without_exception_state(method, number_of_required_arguments) %}
+{% if method.is_constructor %}
+createMinimumArityTypeErrorForConstructor("{{interface_name}}", {{number_of_required_arguments}}, info.Length(), info.GetIsolate())
 {%- else %}
-throwArityTypeErrorForMethod("{{method.name}}", "{{interface_name}}", {{valid_arities}}, info.Length(), info.GetIsolate())
+createMinimumArityTypeErrorForMethod("{{method.name}}", "{{interface_name}}", {{number_of_required_arguments}}, info.Length(), info.GetIsolate())
 {%- endif %}
-{%- endif %}{# methods.idl_type == 'Promise' #}
-{% endmacro %}
+{%- endmacro %}
 
 
 {######################################}
 {% macro throw_minimum_arity_type_error(method, number_of_required_arguments) %}
-{% if method.idl_type == 'Promise' %}
 {% if method.has_exception_state %}
-v8SetReturnValue(info, ScriptPromise::rejectWithMinimumArityTypeError(ScriptState::current(info.GetIsolate()), exceptionState, {{number_of_required_arguments}}, info.Length()).v8Value())
-{%- elif method.is_constructor %}
-v8SetReturnValue(info, ScriptPromise::rejectWithMinimumArityTypeErrorForConstructor(ScriptState::current(info.GetIsolate()), "{{interface_name}}", {{number_of_required_arguments}}, info.Length()).v8Value())
+setMinimumArityTypeError(exceptionState, {{number_of_required_arguments}}, info.Length());
+{{throw_from_exception_state(method)}};
 {%- else %}
-v8SetReturnValue(info, ScriptPromise::rejectWithMinimumArityTypeErrorForMethod(ScriptState::current(info.GetIsolate()), "{{method.name}}", "{{interface_name}}", {{number_of_required_arguments}}, info.Length()).v8Value())
-{%- endif %}
-{%- else %}{# methods.idl_type == 'Promise' #}
-{% if method.has_exception_state %}
-throwMinimumArityTypeError(exceptionState, {{number_of_required_arguments}}, info.Length())
-{%- elif method.is_constructor %}
-throwMinimumArityTypeErrorForConstructor("{{interface_name}}", {{number_of_required_arguments}}, info.Length(), info.GetIsolate())
+{% if method.idl_type == 'Promise' %}
+v8SetReturnValue(info, ScriptPromise::rejectRaw(info.GetIsolate(), {{create_minimum_arity_type_error_without_exception_state(method, number_of_required_arguments)}}));
 {%- else %}
-throwMinimumArityTypeErrorForMethod("{{method.name}}", "{{interface_name}}", {{number_of_required_arguments}}, info.Length(), info.GetIsolate())
-{%- endif %}
-{%- endif %}{# methods.idl_type == 'Promise' #}
-{% endmacro %}
+V8ThrowException::throwException({{create_minimum_arity_type_error_without_exception_state(method, number_of_required_arguments)}}, info.GetIsolate());
+{%- endif %}{# method.idl_type == 'Promise' #}
+{%- endif %}{# method.has_exception_state #}
+{%- endmacro %}
 
 
 {##############################################################################}
@@ -433,7 +414,12 @@ static void {{overloads.name}}Method{{world_suffix}}(const v8::FunctionCallbackI
         {# Report full list of valid arities if gaps and above minimum #}
         {% if overloads.valid_arities %}
         if (info.Length() >= {{overloads.minarg}}) {
-            throwArityTypeError(exceptionState, "{{overloads.valid_arities}}", info.Length());
+            {# FIXME: We should return a rejected Promise here and the

[Jens Widell, 2014/08/12 07:18:03]

This FIXME applies to the code immediately following the if-statement too, so
technically it belongs above. Directly below {# Report full list ... #} would be
suitable, I think.

[yhirano, 2014/08/12 07:43:19]

This function has three blocks and all should be addressed, I think. I moved the
comment to the top of this macro.

+            the following throwIfNeeded when ALL methods return Promise. In
+            order to do so, we must ensure either ALL or NO methods in this
+            overload return Promise #}
+            setArityTypeError(exceptionState, "{{overloads.valid_arities}}", info.Length());
+            exceptionState.throwIfNeeded();
             return;
         }
         {% endif %}
@@ -614,7 +600,7 @@ static void {{name}}(const v8::FunctionCallbackInfo<v8::Value>& info)
     {# Overloaded constructors have length checked during overload resolution #}
     {% if constructor.number_of_required_arguments and not constructor.overload_index %}
     if (UNLIKELY(info.Length() < {{constructor.number_of_required_arguments}})) {
-        {{throw_minimum_arity_type_error(constructor, constructor.number_of_required_arguments)}};
+        {{throw_minimum_arity_type_error(constructor, constructor.number_of_required_arguments) | indent(8)}}
         return;
     }
     {% endif %}