[debugger] make strict eval-scope visible to debugging.

src/debug/debug-scopes.cc

Index: src/debug/debug-scopes.cc
diff --git a/src/debug/debug-scopes.cc b/src/debug/debug-scopes.cc
index 7c6f944335baa6341fe3809b461fd4fab70fad31..b8546b1409d3a5b801e506c339aa5e26056061e7 100644
--- a/src/debug/debug-scopes.cc
+++ b/src/debug/debug-scopes.cc
@@ -5,6 +5,7 @@
 #include "src/debug/debug-scopes.h"
 
 #include "src/ast/scopes.h"
+#include "src/compiler.h"
 #include "src/debug/debug.h"
 #include "src/frames-inl.h"
 #include "src/globals.h"
@@ -80,34 +81,29 @@ ScopeIterator::ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector,
   }
 
   // Reparse the code and analyze the scopes.
-  Scope* scope = NULL;
   // Check whether we are in global, eval or function code.
   Zone zone(isolate->allocator());
+  base::SmartPointer<ParseInfo> info;
   if (scope_info->scope_type() != FUNCTION_SCOPE) {
     // Global or eval code.
     Handle<Script> script(Script::cast(shared_info->script()));
-    ParseInfo info(&zone, script);
+    info.Reset(new ParseInfo(&zone, script));
+    info->set_toplevel();
     if (scope_info->scope_type() == SCRIPT_SCOPE) {
-      info.set_global();
+      info->set_global();
     } else {
       DCHECK(scope_info->scope_type() == EVAL_SCOPE);
-      info.set_eval();
-      info.set_context(Handle<Context>(function->context()));
-    }
-    if (Parser::ParseStatic(&info) && Scope::Analyze(&info)) {
-      scope = info.literal()->scope();
+      info->set_eval();
+      info->set_context(Handle<Context>(function->context()));
     }
-    if (!ignore_nested_scopes) RetrieveScopeChain(scope);
-    if (collect_non_locals) CollectNonLocals(scope);
   } else {
-    // Function code
-    ParseInfo info(&zone, function);
-    if (Parser::ParseStatic(&info) && Scope::Analyze(&info)) {
-      scope = info.literal()->scope();
-    }
-    if (!ignore_nested_scopes) RetrieveScopeChain(scope);
-    if (collect_non_locals) CollectNonLocals(scope);
+    // Inner function.
+    info.Reset(new ParseInfo(&zone, function));
   }
+  Scope* scope = NULL;
+  if (Compiler::ParseAndAnalyze(info.get())) scope = info->literal()->scope();
+  if (!ignore_nested_scopes) RetrieveScopeChain(scope);
+  if (collect_non_locals) CollectNonLocals(scope);
   UnwrapEvaluationContext();
 }
 
@@ -236,7 +232,8 @@ ScopeIterator::ScopeType ScopeIterator::Type() {
         DCHECK(!scope_info->HasContext() || context_->IsBlockContext());
         return ScopeTypeBlock;
       case EVAL_SCOPE:
-        UNREACHABLE();
+        DCHECK(!scope_info->HasContext() || context_->IsFunctionContext());
+        return ScopeTypeEval;

[jgruber1, 2016/05/11 08:38:06]

Should we add a default clause here or alternatively add UNREACHABLE() after the
switch?

[Yang, 2016/05/11 11:37:23]

The compiler would fail if the switch does not list all the enum cases and does
not have a default. I added an UNREACHABLE though.

     }
   }
   if (context_->IsNativeContext()) {
@@ -284,7 +281,8 @@ MaybeHandle<JSObject> ScopeIterator::ScopeObject() {
       // Materialize the content of the closure scope into a JSObject.
       return MaterializeClosure();
     case ScopeIterator::ScopeTypeBlock:
-      return MaterializeBlockScope();
+    case ScopeIterator::ScopeTypeEval:
+      return MaterializeInnerScope();
     case ScopeIterator::ScopeTypeModule:
       return MaterializeModuleScope();
   }
@@ -295,7 +293,8 @@ MaybeHandle<JSObject> ScopeIterator::ScopeObject() {
 
 bool ScopeIterator::HasContext() {
   ScopeType type = Type();
-  if (type == ScopeTypeBlock || type == ScopeTypeLocal) {
+  if (type == ScopeTypeBlock || type == ScopeTypeLocal ||
+      type == ScopeTypeEval) {
     if (!nested_scope_chain_.is_empty()) {
       return nested_scope_chain_.last().scope_info->HasContext();
     }
@@ -321,7 +320,8 @@ bool ScopeIterator::SetVariableValue(Handle<String> variable_name,
     case ScopeIterator::ScopeTypeScript:
       return SetScriptVariableValue(variable_name, new_value);
     case ScopeIterator::ScopeTypeBlock:
-      return SetBlockVariableValue(variable_name, new_value);
+    case ScopeIterator::ScopeTypeEval:
+      return SetInnerScopeVariableValue(variable_name, new_value);
     case ScopeIterator::ScopeTypeModule:
       // TODO(2399): should we implement it?
       break;
@@ -482,19 +482,16 @@ MaybeHandle<JSObject> ScopeIterator::MaterializeLocalScope() {
 
   if (!scope_info->HasContext()) return local_scope;
 
-  // Third fill all context locals.
+  // Fill all context locals.
   Handle<Context> function_context(frame_context->closure_context());
   CopyContextLocalsToScopeObject(scope_info, function_context, local_scope);
 
   // Finally copy any properties from the function context extension.
   // These will be variables introduced by eval.
   if (function_context->closure() == *function &&
-      function_context->has_extension() &&
       !function_context->IsNativeContext()) {
-    bool success = CopyContextExtensionToScopeObject(
-        handle(function_context->extension_object(), isolate_), local_scope,
-        INCLUDE_PROTOS);
-    if (!success) return MaybeHandle<JSObject>();
+    CopyContextExtensionToScopeObject(function_context, local_scope,
+                                      INCLUDE_PROTOS);
   }
 
   return local_scope;
@@ -520,12 +517,7 @@ Handle<JSObject> ScopeIterator::MaterializeClosure() {
 
   // Finally copy any properties from the function context extension. This will
   // be variables introduced by eval.
-  if (context->has_extension()) {
-    bool success = CopyContextExtensionToScopeObject(
-        handle(context->extension_object(), isolate_), closure_scope, OWN_ONLY);
-    DCHECK(success);
-    USE(success);
-  }
+  CopyContextExtensionToScopeObject(context, closure_scope, OWN_ONLY);
 
   return closure_scope;
 }
@@ -560,14 +552,14 @@ Handle<JSObject> ScopeIterator::WithContextExtension() {
 
 // Create a plain JSObject which materializes the block scope for the specified
 // block context.
-Handle<JSObject> ScopeIterator::MaterializeBlockScope() {
-  Handle<JSObject> block_scope =
+Handle<JSObject> ScopeIterator::MaterializeInnerScope() {
+  Handle<JSObject> inner_scope =
       isolate_->factory()->NewJSObjectWithNullProto();
 
   Handle<Context> context = Handle<Context>::null();
   if (!nested_scope_chain_.is_empty()) {
     Handle<ScopeInfo> scope_info = nested_scope_chain_.last().scope_info;
-    frame_inspector_->MaterializeStackLocals(block_scope, scope_info);
+    frame_inspector_->MaterializeStackLocals(inner_scope, scope_info);
     if (scope_info->HasContext()) context = CurrentContext();
   } else {
     context = CurrentContext();
@@ -575,17 +567,10 @@ Handle<JSObject> ScopeIterator::MaterializeBlockScope() {
 
   if (!context.is_null()) {
     // Fill all context locals.
-    CopyContextLocalsToScopeObject(handle(context->scope_info()),
-                                   context, block_scope);
-    // Fill all extension variables.
-    if (context->extension_object() != nullptr) {
-      bool success = CopyContextExtensionToScopeObject(
-          handle(context->extension_object()), block_scope, OWN_ONLY);
-      DCHECK(success);
-      USE(success);
-    }
+    CopyContextLocalsToScopeObject(CurrentScopeInfo(), context, inner_scope);
+    CopyContextExtensionToScopeObject(context, inner_scope, OWN_ONLY);
   }
-  return block_scope;
+  return inner_scope;
 }
 
 
@@ -695,9 +680,11 @@ bool ScopeIterator::SetLocalVariableValue(Handle<String> variable_name,
   return result;
 }
 
-bool ScopeIterator::SetBlockVariableValue(Handle<String> variable_name,
-                                          Handle<Object> new_value) {
+bool ScopeIterator::SetInnerScopeVariableValue(Handle<String> variable_name,
+                                               Handle<Object> new_value) {
   Handle<ScopeInfo> scope_info = CurrentScopeInfo();
+  DCHECK(scope_info->scope_type() == BLOCK_SCOPE ||
+         scope_info->scope_type() == EVAL_SCOPE);
   JavaScriptFrame* frame = GetFrame();
 
   // Setting stack locals of optimized frames is not supported.
@@ -772,31 +759,29 @@ void ScopeIterator::CopyContextLocalsToScopeObject(
   }
 }
 
-bool ScopeIterator::CopyContextExtensionToScopeObject(
-    Handle<JSObject> extension, Handle<JSObject> scope_object,
+void ScopeIterator::CopyContextExtensionToScopeObject(
+    Handle<Context> context, Handle<JSObject> scope_object,
     KeyCollectionType type) {
-  Handle<FixedArray> keys;
-  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
-      isolate_, keys, JSReceiver::GetKeys(extension, type, ENUMERABLE_STRINGS),
-      false);
+  if (!context->has_extension()) return;
+  Handle<JSObject> extension(context->extension_object());
+  Handle<FixedArray> keys =
+      JSReceiver::GetKeys(extension, type, ENUMERABLE_STRINGS)
+          .ToHandleChecked();
 
   for (int i = 0; i < keys->length(); i++) {
     // Names of variables introduced by eval are strings.
     DCHECK(keys->get(i)->IsString());
     Handle<String> key(String::cast(keys->get(i)));
-    Handle<Object> value;
-    ASSIGN_RETURN_ON_EXCEPTION_VALUE(
-        isolate_, value, Object::GetPropertyOrElement(extension, key), false);
-    RETURN_ON_EXCEPTION_VALUE(
-        isolate_, JSObject::SetOwnPropertyIgnoreAttributes(
-            scope_object, key, value, NONE), false);
+    Handle<Object> value =
+        Object::GetPropertyOrElement(extension, key).ToHandleChecked();
+    JSObject::SetOwnPropertyIgnoreAttributes(scope_object, key, value, NONE)
+        .Check();
   }
-  return true;
 }
 
 void ScopeIterator::GetNestedScopeChain(Isolate* isolate, Scope* scope,
                                         int position) {
-  if (!scope->is_eval_scope() && !scope->is_hidden()) {
+  if (!scope->is_hidden()) {
     nested_scope_chain_.Add(ExtendedScopeInfo(scope->GetScopeInfo(isolate),
                                               scope->start_position(),
                                               scope->end_position()));