[wasm] Throw real Error on wasm trap

src/runtime/runtime-internal.cc

Index: src/runtime/runtime-internal.cc
diff --git a/src/runtime/runtime-internal.cc b/src/runtime/runtime-internal.cc
index 8a5bdeb60dc1ca292a58db550f0f5332fb94fdab..06d52246769f34c833b4bfd25fa90523dab672c6 100644
--- a/src/runtime/runtime-internal.cc
+++ b/src/runtime/runtime-internal.cc
@@ -96,6 +96,16 @@ RUNTIME_FUNCTION(Runtime_ThrowStackOverflow) {
   return isolate->StackOverflow();
 }
 
+RUNTIME_FUNCTION(Runtime_ThrowWasmError) {
+  HandleScope scope(isolate);
+  DCHECK(args.length() == 1);

[Michael Starzinger, 2016/04/11 18:13:26]

nit: I know we are inconsistent, but lets use "DCHECK_EQ(1, args.length())" for
new code.

[Clemens Hammacher, 2016/04/12 09:33:44]

Sure, didn't see that.

+  CONVERT_SMI_ARG_CHECKED(message_id, 0);
+  // TODO(clemensh): discuss with titzer whether a WasmError class would make
+  // sense here
+  Handle<Object> exception = isolate->factory()->NewError(
+      static_cast<MessageTemplate::Template>(message_id));
+  return isolate->Throw(*exception);

[Michael Starzinger, 2016/04/11 18:13:26]

nit: This can also be written as ...

THROW_NEW_ERROR_RETURN_FAILURE(isolate,
NewError(static_cast<MessageTemplate::Template>(message_id)));

[Clemens Hammacher, 2016/04/12 09:33:44]

and that's considered nicer? ;)
but OK, I changed it :)

[Michael Starzinger, 2016/04/12 10:47:56]

We can discuss which version is nicer and change the existing code to use
Isolate::Throw directly instead of the macro. But having part of the code use
the macro and other parts of the code use Isolate::Throw is not a "nice"
situation. IMHO keeping this call-site in sync with existing code trumps having
this one call-site be "nice".

Also I realize that I am contradicting myself here with the comment above where
I say DCHECK_EQ should be used "at this one call-site". But that DCHECK_EQ above
is one incremental cleanup that some of is collectively agreed to, to eventually
get into a state where the "readability/check" linter check passes. We did not
(yet) agree to cleanup Isolate::Throw in favor of the
THROW_NEW_ERROR_RETURN_FAILURE macro.

Does that make sense or do you think I am completely off here?

+}
 
 RUNTIME_FUNCTION(Runtime_UnwindAndFindExceptionHandler) {
   SealHandleScope shs(isolate);