[debug] Don't notify listener of exceptions internal to a desugaring.
In the parser, we desugar yield* with the help of a regular yield. One
particular implementation detail of this desugaring is that when the user calls
the generator's throw method, this throws an exception that we immediately
catch. This exception should not be visible to the user, but through Devtools'
"Pause on Caught Exceptions" feature it used to be.
This CL extends the type of catch predictions with a new value for such internal
exceptions and uses that for the offending try-catch statement in yield*. It
instruments the debugger to _not_ trigger an exception event in that case.
R=yangguo@chromium.org
TBR=littledan@chromium.org
BUG=v8:5218
Review-Url: https://codereview.chromium.org/2203803002
Cr-Commit-Position: refs/heads/master@{#38286}
diff --git a/src/ast/ast.h b/src/ast/ast.h
index 19e530c..ad02c96 100644
--- a/src/ast/ast.h
+++ b/src/ast/ast.h
@@ -3166,6 +3166,16 @@
HandlerTable::PROMISE, pos);
}
+ TryCatchStatement* NewTryCatchStatementForDesugaring(Block* try_block,
+ Scope* scope,
+ Variable* variable,
+ Block* catch_block,
+ int pos) {
+ return new (local_zone_)
+ TryCatchStatement(local_zone_, try_block, scope, variable, catch_block,
+ HandlerTable::DESUGARING, pos);
+ }
+
TryFinallyStatement* NewTryFinallyStatement(Block* try_block,
Block* finally_block, int pos) {
return new (local_zone_)
diff --git a/src/ast/prettyprinter.cc b/src/ast/prettyprinter.cc
index 40a60fc..1a8d73e 100644
--- a/src/ast/prettyprinter.cc
+++ b/src/ast/prettyprinter.cc
@@ -897,6 +897,9 @@
case HandlerTable::PROMISE:
prediction = "PROMISE";
break;
+ case HandlerTable::DESUGARING:
+ prediction = "DESUGARING";
+ break;
}
Print(" %s\n", prediction);
}
diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index de29b83..4556206 100644
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -1722,8 +1722,11 @@
void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
- // In our prediction, try-finally is not considered to catch.
Isolate::CatchType catch_type = isolate_->PredictExceptionCatcher();
+
+ // Don't notify listener of exceptions that are internal to a desugaring.
+ if (catch_type == Isolate::CAUGHT_BY_DESUGARING) return;
+
bool uncaught = (catch_type == Isolate::NOT_CAUGHT);
if (promise->IsJSObject()) {
Handle<JSObject> jspromise = Handle<JSObject>::cast(promise);
diff --git a/src/isolate.cc b/src/isolate.cc
index cc8c8b6..5832430 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1142,8 +1142,9 @@
// embedder didn't specify a custom uncaught exception callback,
// or if the custom callback determined that V8 should abort, then
// abort.
+ CatchType prediction = PredictExceptionCatcher();
if (FLAG_abort_on_uncaught_exception &&
- PredictExceptionCatcher() != CAUGHT_BY_JAVASCRIPT &&
+ (prediction == NOT_CAUGHT || prediction == CAUGHT_BY_EXTERNAL) &&
(!abort_on_uncaught_exception_callback_ ||
abort_on_uncaught_exception_callback_(
reinterpret_cast<v8::Isolate*>(this)))) {
@@ -1339,9 +1340,9 @@
// For JavaScript frames we perform a lookup in the handler table.
if (frame->is_java_script()) {
JavaScriptFrame* js_frame = static_cast<JavaScriptFrame*>(frame);
- if (PredictException(js_frame) != HandlerTable::UNCAUGHT) {
- return CAUGHT_BY_JAVASCRIPT;
- }
+ HandlerTable::CatchPrediction prediction = PredictException(js_frame);
+ if (prediction == HandlerTable::DESUGARING) return CAUGHT_BY_DESUGARING;
+ if (prediction != HandlerTable::UNCAUGHT) return CAUGHT_BY_JAVASCRIPT;
}
// The exception has been externally caught if and only if there is an
@@ -1750,12 +1751,16 @@
ThreadLocalTop* tltop = thread_local_top();
if (tltop->promise_on_stack_ == NULL) return undefined;
// Find the top-most try-catch or try-finally handler.
- if (PredictExceptionCatcher() != CAUGHT_BY_JAVASCRIPT) return undefined;
+ CatchType prediction = PredictExceptionCatcher();
+ if (prediction == NOT_CAUGHT || prediction == CAUGHT_BY_EXTERNAL) {
+ return undefined;
+ }
for (JavaScriptFrameIterator it(this); !it.done(); it.Advance()) {
switch (PredictException(it.frame())) {
case HandlerTable::UNCAUGHT:
break;
case HandlerTable::CAUGHT:
+ case HandlerTable::DESUGARING:
return undefined;
case HandlerTable::PROMISE:
return tltop->promise_on_stack_->promise();
diff --git a/src/isolate.h b/src/isolate.h
index 3284512..bc01619 100644
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -748,7 +748,12 @@
// Tries to predict whether an exception will be caught. Note that this can
// only produce an estimate, because it is undecidable whether a finally
// clause will consume or re-throw an exception.
- enum CatchType { NOT_CAUGHT, CAUGHT_BY_JAVASCRIPT, CAUGHT_BY_EXTERNAL };
+ enum CatchType {
+ NOT_CAUGHT,
+ CAUGHT_BY_JAVASCRIPT,
+ CAUGHT_BY_EXTERNAL,
+ CAUGHT_BY_DESUGARING
+ };
CatchType PredictExceptionCatcher();
void ScheduleThrow(Object* exception);
diff --git a/src/objects.h b/src/objects.h
index 416bde2..bef317e 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -4466,9 +4466,13 @@
// exception or cause a re-throw to outside the code boundary. Since this is
// undecidable it is merely an approximation (e.g. useful for debugger).
enum CatchPrediction {
- UNCAUGHT, // the handler will (likely) rethrow the exception.
- CAUGHT, // the exception will be caught by the handler.
- PROMISE // the exception will be caught and cause a promise rejection.
+ UNCAUGHT, // The handler will (likely) rethrow the exception.
+ CAUGHT, // The exception will be caught by the handler.
+ PROMISE, // The exception will be caught and cause a promise rejection.
+ DESUGARING, // The exception will be caught, but both the exception and the
+ // catching are part of a desugaring and should therefore not
+ // be visible to the user (we won't notify the debugger of such
+ // exceptions).
};
// Getters for handler table based on ranges.
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index 239c4c8..545d78c 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -6531,7 +6531,7 @@
catch_scope->DeclareLocal(name, VAR, kCreatedInitialized,
Variable::NORMAL);
- try_catch = factory->NewTryCatchStatement(
+ try_catch = factory->NewTryCatchStatementForDesugaring(
try_block, catch_scope, catch_variable, catch_block, nopos);
}
diff --git a/test/mjsunit/debug-exceptions.js b/test/mjsunit/debug-exceptions.js
index 655e535..1a0e222 100644
--- a/test/mjsunit/debug-exceptions.js
+++ b/test/mjsunit/debug-exceptions.js
@@ -67,4 +67,22 @@
});
+// Check that an internal exception in our yield* desugaring is not observable.
+{
+ uncaught = null;
+
+ let iter = {
+ next() {return {value:42, done:false}},
+ throw() {return {done:true}}
+ };
+ let iterable = {[Symbol.iterator]() {return iter}};
+ function* f() { yield* iterable }
+
+ let g = f();
+ g.next();
+ assertEquals({value: undefined, done: true}, g.throw());
+ assertNull(uncaught); // No exception event was generated.
+}
+
+
assertFalse(error);
