Reland "[debugger] Rewrite the ScopeIterator/DebugEvaluate to use Scope rather...

Reland "[debugger] Rewrite the ScopeIterator/DebugEvaluate to use Scope rather than ScopeInfo for inner scopes."

This is a reland of 9e27d473

Original change's description:
> [debugger] Rewrite the ScopeIterator/DebugEvaluate to use Scope rather than ScopeInfo for inner scopes.
> 
> This CL also bottlenecks all current scope handling in the ScopeIterator, and cleans up frame handling in debug-frames and the deoptimizer.
> 
> Change-Id: I061922a356ce17794262f8d77d5d7c824558fc50
> Reviewed-on: https://chromium-review.googlesource.com/1095094
> Commit-Queue: Toon Verwaest <verwaest@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#53741}

Change-Id: I05262fef66d852876b9bb2869339053629c9b51d
Reviewed-on: https://chromium-review.googlesource.com/1102297Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53751}

src/ast/scopes.cc

@@ -2369,21 +2369,8 @@ void Scope::AllocateScopeInfosRecursively(Isolate* isolate,
   }
 }
 
-void Scope::AllocateDebuggerScopeInfos(Isolate* isolate,
-                                       MaybeHandle<ScopeInfo> outer_scope) {
-  if (scope_info_.is_null()) {
-    scope_info_ = ScopeInfo::Create(isolate, zone(), this, outer_scope);
-  }
-  MaybeHandle<ScopeInfo> outer = NeedsContext() ? scope_info_ : outer_scope;
-  for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
-    if (scope->is_function_scope()) continue;
-    scope->AllocateDebuggerScopeInfos(isolate, outer);
-  }
-}
-
 // static
-void DeclarationScope::AllocateScopeInfos(ParseInfo* info, Isolate* isolate,
-                                          AnalyzeMode mode) {
+void DeclarationScope::AllocateScopeInfos(ParseInfo* info, Isolate* isolate) {
   DeclarationScope* scope = info->literal()->scope();
   if (!scope->scope_info_.is_null()) return;  // Allocated by outer function.
 
@@ -2393,9 +2380,6 @@ void DeclarationScope::AllocateScopeInfos(ParseInfo* info, Isolate* isolate,
   }
 
   scope->AllocateScopeInfosRecursively(isolate, outer_scope);
-  if (mode == AnalyzeMode::kDebugger) {
-    scope->AllocateDebuggerScopeInfos(isolate, outer_scope);
-  }
 
   // The debugger expects all shared function infos to contain a scope info.
   // Since the top-most scope will end up in a shared function info, make sure

src/ast/scopes.h

@@ -78,8 +78,6 @@ class SloppyBlockFunctionMap : public ZoneHashMap {
   int count_;
 };
 
-enum class AnalyzeMode { kRegular, kDebugger };
-
 // Global invariants after AST construction: Each reference (i.e. identifier)
 // to a JavaScript variable (including global properties) is represented by a
 // VariableProxy node. Immediately after AST construction and before variable
@@ -367,7 +365,8 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // Whether this needs to be represented by a runtime context.
   bool NeedsContext() const {
     // Catch scopes always have heap slots.
-    DCHECK(!is_catch_scope() || num_heap_slots() > 0);
+    DCHECK_IMPLIES(is_catch_scope(), num_heap_slots() > 0);
+    DCHECK_IMPLIES(is_with_scope(), num_heap_slots() > 0);
     return num_heap_slots() > 0;
   }
 
@@ -813,7 +812,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   // The local variable 'arguments' if we need to allocate it; nullptr
   // otherwise.
   Variable* arguments() const {
-    DCHECK(!is_arrow_scope() || arguments_ == nullptr);
+    DCHECK_IMPLIES(is_arrow_scope(), arguments_ == nullptr);
     return arguments_;
   }
 
@@ -868,8 +867,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
 
   // Allocate ScopeInfos for top scope and any inner scopes that need them.
   // Does nothing if ScopeInfo is already allocated.
-  static void AllocateScopeInfos(ParseInfo* info, Isolate* isolate,
-                                 AnalyzeMode mode);
+  static void AllocateScopeInfos(ParseInfo* info, Isolate* isolate);
 
   Handle<StringSet> CollectNonLocals(ParseInfo* info,
                                      Handle<StringSet> non_locals);

src/compiler-dispatcher/unoptimized-compile-job.cc

@@ -288,8 +288,7 @@ void UnoptimizedCompileJob::FinalizeOnMainThread(Isolate* isolate) {
     // Internalize ast values onto the heap.
     parse_info_->ast_value_factory()->Internalize(isolate);
     // Allocate scope infos for the literal.
-    DeclarationScope::AllocateScopeInfos(parse_info_.get(), isolate,
-                                         AnalyzeMode::kRegular);
+    DeclarationScope::AllocateScopeInfos(parse_info_.get(), isolate);
     if (compilation_job_->state() == CompilationJob::State::kFailed ||
         !Compiler::FinalizeCompilationJob(compilation_job_.release(), shared_,
                                           isolate)) {

src/compiler.cc

@@ -509,8 +509,7 @@ bool FinalizeUnoptimizedCode(
   DCHECK(AllowCompilation::IsAllowed(isolate));
 
   // Allocate scope infos for the literal.
-  DeclarationScope::AllocateScopeInfos(parse_info, isolate,
-                                       AnalyzeMode::kRegular);
+  DeclarationScope::AllocateScopeInfos(parse_info, isolate);
 
   // Finalize the outer-most function's compilation job.
   if (FinalizeUnoptimizedCompilationJob(outer_function_job, shared_info,

src/debug/debug-evaluate.cc

@@ -159,20 +159,23 @@ MaybeHandle<Object> DebugEvaluate::Evaluate(
   return result;
 }
 
+Handle<SharedFunctionInfo> DebugEvaluate::ContextBuilder::outer_info() const {
+  return handle(frame_inspector_.GetFunction()->shared(), isolate_);
+}
 
 DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
                                               JavaScriptFrame* frame,
                                               int inlined_jsframe_index)
     : isolate_(isolate),
-      frame_(frame),
-      inlined_jsframe_index_(inlined_jsframe_index) {
-  FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
-  Handle<JSFunction> local_function = frame_inspector.GetFunction();
-  Handle<Context> outer_context(local_function->context());
+      frame_inspector_(frame, inlined_jsframe_index, isolate),
+      scope_iterator_(isolate, &frame_inspector_,
+                      ScopeIterator::COLLECT_NON_LOCALS) {
+  Handle<Context> outer_context(frame_inspector_.GetFunction()->context());
   evaluation_context_ = outer_context;
-  outer_info_ = handle(local_function->shared(), isolate);
   Factory* factory = isolate->factory();
 
+  if (scope_iterator_.Done()) return;
+
   // To evaluate as if we were running eval at the point of the debug break,
   // we reconstruct the context chain as follows:
   //  - To make stack-allocated variables visible, we materialize them and
@@ -189,64 +192,32 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
   //  - Look up in the materialized stack variables.
   //  - Look up in the original context.
   //  - Check the whitelist to find out whether to skip contexts during lookup.
-  const ScopeIterator::Option option = ScopeIterator::COLLECT_NON_LOCALS;
-  for (ScopeIterator it(isolate, &frame_inspector, option); !it.Done();
-       it.Next()) {
-    ScopeIterator::ScopeType scope_type = it.Type();
+  for (; scope_iterator_.InInnerScope(); scope_iterator_.Next()) {
+    ScopeIterator::ScopeType scope_type = scope_iterator_.Type();
+    if (scope_type == ScopeIterator::ScopeTypeScript) break;
+    ContextChainElement context_chain_element;
+    if (scope_type == ScopeIterator::ScopeTypeLocal ||
+        scope_iterator_.DeclaresLocals(ScopeIterator::Mode::STACK)) {
+      context_chain_element.materialized_object =
+          scope_iterator_.ScopeObject(ScopeIterator::Mode::STACK);
+    }
+    if (scope_iterator_.HasContext()) {
+      context_chain_element.wrapped_context = scope_iterator_.CurrentContext();
+    }
     if (scope_type == ScopeIterator::ScopeTypeLocal) {
-      DCHECK_EQ(FUNCTION_SCOPE, it.CurrentScopeInfo()->scope_type());
-      Handle<JSObject> materialized = factory->NewJSObjectWithNullProto();
-      Handle<Context> local_context =
-          it.HasContext() ? it.CurrentContext() : outer_context;
-      Handle<StringSet> non_locals = it.GetNonLocals();
-      MaterializeReceiver(materialized, local_context, local_function,
-                          non_locals);
-      MaterializeStackLocals(materialized, local_function, &frame_inspector);
-      ContextChainElement context_chain_element;
-      context_chain_element.scope_info = it.CurrentScopeInfo();
-      context_chain_element.materialized_object = materialized;
-      // Non-locals that are already being referenced by the current function
-      // are guaranteed to be correctly resolved.
-      context_chain_element.whitelist = non_locals;
-      if (it.HasContext()) {
-        context_chain_element.wrapped_context = it.CurrentContext();
-      }
-      context_chain_.push_back(context_chain_element);
-      evaluation_context_ = outer_context;
-      break;
-    } else if (scope_type == ScopeIterator::ScopeTypeCatch ||
-               scope_type == ScopeIterator::ScopeTypeWith ||
-               scope_type == ScopeIterator::ScopeTypeModule) {
-      ContextChainElement context_chain_element;
-      Handle<Context> current_context = it.CurrentContext();
-      if (!current_context->IsDebugEvaluateContext()) {
-        context_chain_element.wrapped_context = current_context;
-      }
-      context_chain_.push_back(context_chain_element);
-    } else if (scope_type == ScopeIterator::ScopeTypeBlock ||
-               scope_type == ScopeIterator::ScopeTypeEval) {
-      Handle<JSObject> materialized = factory->NewJSObjectWithNullProto();
-      frame_inspector.MaterializeStackLocals(materialized,
-                                             it.CurrentScopeInfo());
-      ContextChainElement context_chain_element;
-      context_chain_element.scope_info = it.CurrentScopeInfo();
-      context_chain_element.materialized_object = materialized;
-      if (it.HasContext()) {
-        context_chain_element.wrapped_context = it.CurrentContext();
-      }
-      context_chain_.push_back(context_chain_element);
-    } else {
-      break;
+      context_chain_element.whitelist = scope_iterator_.GetNonLocals();
     }
+    context_chain_.push_back(context_chain_element);
   }
 
+  Handle<ScopeInfo> scope_info =
+      evaluation_context_->IsNativeContext()
+          ? Handle<ScopeInfo>::null()
+          : handle(evaluation_context_->scope_info(), isolate);
   for (auto rit = context_chain_.rbegin(); rit != context_chain_.rend();
        rit++) {
     ContextChainElement element = *rit;
-    Handle<ScopeInfo> scope_info(ScopeInfo::CreateForWithScope(
-        isolate, evaluation_context_->IsNativeContext()
-                     ? Handle<ScopeInfo>::null()
-                     : Handle<ScopeInfo>(evaluation_context_->scope_info())));
+    scope_info = ScopeInfo::CreateForWithScope(isolate, scope_info);
     scope_info->SetIsDebugEvaluateScope();
     evaluation_context_ = factory->NewDebugEvaluateContext(
         evaluation_context_, scope_info, element.materialized_object,
@@ -256,62 +227,24 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
 
 
 void DebugEvaluate::ContextBuilder::UpdateValues() {
+  scope_iterator_.Restart();
   for (ContextChainElement& element : context_chain_) {
     if (!element.materialized_object.is_null()) {
-      // Write back potential changes to materialized stack locals to the stack.
-      FrameInspector(frame_, inlined_jsframe_index_, isolate_)
-          .UpdateStackLocalsFromMaterializedObject(element.materialized_object,
-                                                   element.scope_info);
+      Handle<FixedArray> keys =
+          KeyAccumulator::GetKeys(element.materialized_object,
+                                  KeyCollectionMode::kOwnOnly,
+                                  ENUMERABLE_STRINGS)
+              .ToHandleChecked();
+
+      for (int i = 0; i < keys->length(); i++) {
+        DCHECK(keys->get(i)->IsString());
+        Handle<String> key(String::cast(keys->get(i)), isolate_);
+        Handle<Object> value =
+            JSReceiver::GetDataProperty(element.materialized_object, key);
+        scope_iterator_.SetVariableValue(key, value);
+      }
     }
-  }
-}
-
-
-void DebugEvaluate::ContextBuilder::MaterializeReceiver(
-    Handle<JSObject> target, Handle<Context> local_context,
-    Handle<JSFunction> local_function, Handle<StringSet> non_locals) {
-  Handle<Object> recv = isolate_->factory()->undefined_value();
-  Handle<String> name = isolate_->factory()->this_string();
-  if (non_locals->Has(name)) {
-    // 'this' is allocated in an outer context and is is already being
-    // referenced by the current function, so it can be correctly resolved.
-    return;
-  } else if (local_function->shared()->scope_info()->HasReceiver() &&
-             !frame_->receiver()->IsTheHole(isolate_)) {
-    recv = handle(frame_->receiver(), isolate_);
-  }
-  JSObject::SetOwnPropertyIgnoreAttributes(target, name, recv, NONE).Check();
-}
-
-void DebugEvaluate::ContextBuilder::MaterializeStackLocals(
-    Handle<JSObject> target, Handle<JSFunction> function,
-    FrameInspector* frame_inspector) {
-  bool materialize_arguments_object = true;
-
-  // Do not materialize the arguments object for eval or top-level code.
-  if (function->shared()->is_toplevel()) materialize_arguments_object = false;
-
-  // First materialize stack locals (modulo arguments object).
-  Handle<SharedFunctionInfo> shared(function->shared());
-  Handle<ScopeInfo> scope_info(shared->scope_info());
-  frame_inspector->MaterializeStackLocals(target, scope_info,
-                                          materialize_arguments_object);
-
-  // Then materialize the arguments object.
-  if (materialize_arguments_object) {
-    // Skip if "arguments" is already taken and wasn't optimized out (which
-    // causes {MaterializeStackLocals} above to skip the local variable).
-    Handle<String> arguments_str = isolate_->factory()->arguments_string();
-    Maybe<bool> maybe = JSReceiver::HasOwnProperty(target, arguments_str);
-    DCHECK(maybe.IsJust());
-    if (maybe.FromJust()) return;
-
-    // FunctionGetArguments can't throw an exception.
-    Handle<JSObject> arguments =
-        Accessors::FunctionGetArguments(frame_, inlined_jsframe_index_);
-    JSObject::SetOwnPropertyIgnoreAttributes(target, arguments_str, arguments,
-                                             NONE)
-        .Check();
+    scope_iterator_.Next();
   }
 }
 

src/debug/debug-evaluate.h

@@ -7,7 +7,8 @@
 
 #include <vector>
 
-#include "src/frames.h"
+#include "src/debug/debug-frames.h"
+#include "src/debug/debug-scopes.h"
 #include "src/objects.h"
 #include "src/objects/shared-function-info.h"
 #include "src/objects/string-table.h"
@@ -70,31 +71,20 @@ class DebugEvaluate : public AllStatic {
     void UpdateValues();
 
     Handle<Context> evaluation_context() const { return evaluation_context_; }
-    Handle<SharedFunctionInfo> outer_info() const { return outer_info_; }
+    Handle<SharedFunctionInfo> outer_info() const;
 
    private:
     struct ContextChainElement {
-      Handle<ScopeInfo> scope_info;
       Handle<Context> wrapped_context;
       Handle<JSObject> materialized_object;
       Handle<StringSet> whitelist;
     };
 
-    void MaterializeReceiver(Handle<JSObject> target,
-                             Handle<Context> local_context,
-                             Handle<JSFunction> local_function,
-                             Handle<StringSet> non_locals);
-
-    void MaterializeStackLocals(Handle<JSObject> target,
-                                Handle<JSFunction> function,
-                                FrameInspector* frame_inspector);
-
-    Handle<SharedFunctionInfo> outer_info_;
     Handle<Context> evaluation_context_;
     std::vector<ContextChainElement> context_chain_;
     Isolate* isolate_;
-    JavaScriptFrame* frame_;
-    int inlined_jsframe_index_;
+    FrameInspector frame_inspector_;
+    ScopeIterator scope_iterator_;
   };
 
   static MaybeHandle<Object> Evaluate(Isolate* isolate,

src/debug/debug-frames.cc

@@ -15,6 +15,7 @@ namespace internal {
 FrameInspector::FrameInspector(StandardFrame* frame, int inlined_frame_index,
                                Isolate* isolate)
     : frame_(frame),
+      inlined_frame_index_(inlined_frame_index),
       isolate_(isolate) {
   // Extract the relevant information from the frame summary and discard it.
   FrameSummary summary = FrameSummary::Get(frame, inlined_frame_index);
@@ -82,97 +83,6 @@ bool FrameInspector::IsWasm() { return frame_->is_wasm(); }
 
 bool FrameInspector::IsJavaScript() { return frame_->is_java_script(); }
 
-// To inspect all the provided arguments the frame might need to be
-// replaced with the arguments frame.
-void FrameInspector::SetArgumentsFrame(StandardFrame* frame) {
-  DCHECK(has_adapted_arguments_);
-  DCHECK(frame->is_arguments_adaptor());
-  frame_ = frame;
-  is_optimized_ = frame_->is_optimized();
-  is_interpreted_ = frame_->is_interpreted();
-  DCHECK(!is_optimized_);
-}
-
-
-// Create a plain JSObject which materializes the local scope for the specified
-// frame.
-void FrameInspector::MaterializeStackLocals(Handle<JSObject> target,
-                                            Handle<ScopeInfo> scope_info,
-                                            bool materialize_arguments_object) {
-  HandleScope scope(isolate_);
-  // First fill all parameters.
-  for (int i = 0; i < scope_info->ParameterCount(); ++i) {
-    // Do not materialize the parameter if it is shadowed by a context local.
-    // TODO(yangguo): check whether this is necessary, now that we materialize
-    //                context locals as well.
-    Handle<String> name(scope_info->ParameterName(i));
-    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
-    if (ParameterIsShadowedByContextLocal(scope_info, name)) continue;
-
-    Handle<Object> value =
-        i < GetParametersCount()
-            ? GetParameter(i)
-            : Handle<Object>::cast(isolate_->factory()->undefined_value());
-    DCHECK(!value->IsTheHole(isolate_));
-
-    JSObject::SetOwnPropertyIgnoreAttributes(target, name, value, NONE).Check();
-  }
-
-  // Second fill all stack locals.
-  for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
-    Handle<String> name(scope_info->StackLocalName(i));
-    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
-    Handle<Object> value = GetExpression(scope_info->StackLocalIndex(i));
-    // TODO(yangguo): We convert optimized out values to {undefined} when they
-    // are passed to the debugger. Eventually we should handle them somehow.
-    if (value->IsTheHole(isolate_)) {
-      value = isolate_->factory()->undefined_value();
-    }
-    if (value->IsOptimizedOut(isolate_)) {
-      if (materialize_arguments_object) {
-        Handle<String> arguments_str = isolate_->factory()->arguments_string();
-        if (String::Equals(name, arguments_str)) continue;
-      }
-      value = isolate_->factory()->undefined_value();
-    }
-    JSObject::SetOwnPropertyIgnoreAttributes(target, name, value, NONE).Check();
-  }
-}
-
-
-void FrameInspector::UpdateStackLocalsFromMaterializedObject(
-    Handle<JSObject> target, Handle<ScopeInfo> scope_info) {
-  // Optimized frames and wasm frames are not supported. Simply give up.
-  if (is_optimized_ || frame_->is_wasm()) return;
-
-  HandleScope scope(isolate_);
-
-  // Parameters.
-  for (int i = 0; i < scope_info->ParameterCount(); ++i) {
-    // Shadowed parameters were not materialized.
-    Handle<String> name(scope_info->ParameterName(i));
-    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
-    if (ParameterIsShadowedByContextLocal(scope_info, name)) continue;
-
-    DCHECK(!javascript_frame()->GetParameter(i)->IsTheHole(isolate_));
-    Handle<Object> value =
-        Object::GetPropertyOrElement(target, name).ToHandleChecked();
-    javascript_frame()->SetParameterValue(i, *value);
-  }
-
-  // Stack locals.
-  for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
-    Handle<String> name(scope_info->StackLocalName(i));
-    if (ScopeInfo::VariableIsSynthetic(*name)) continue;
-    int index = scope_info->StackLocalIndex(i);
-    if (frame_->GetExpression(index)->IsTheHole(isolate_)) continue;
-    Handle<Object> value =
-        Object::GetPropertyOrElement(target, name).ToHandleChecked();
-    frame_->SetExpression(index, *value);
-  }
-}
-
-
 bool FrameInspector::ParameterIsShadowedByContextLocal(
     Handle<ScopeInfo> info, Handle<String> parameter_name) {
   VariableMode mode;

src/debug/debug-frames.h

@@ -22,7 +22,7 @@ class FrameInspector {
   ~FrameInspector();
 
   int GetParametersCount();
-  Handle<JSFunction> GetFunction() { return function_; }
+  Handle<JSFunction> GetFunction() const { return function_; }
   Handle<Script> GetScript() { return script_; }
   Handle<Object> GetParameter(int index);
   Handle<Object> GetExpression(int index);
@@ -41,21 +41,14 @@ class FrameInspector {
                                           : JavaScriptFrame::cast(frame_);
   }
 
-  JavaScriptFrame* GetArgumentsFrame() { return javascript_frame(); }
-  void SetArgumentsFrame(StandardFrame* frame);
-
-  void MaterializeStackLocals(Handle<JSObject> target,
-                              Handle<ScopeInfo> scope_info,
-                              bool materialize_arguments_object = false);
-
-  void UpdateStackLocalsFromMaterializedObject(Handle<JSObject> object,
-                                               Handle<ScopeInfo> scope_info);
+  int inlined_frame_index() const { return inlined_frame_index_; }
 
  private:
   bool ParameterIsShadowedByContextLocal(Handle<ScopeInfo> info,
                                          Handle<String> parameter_name);
 
   StandardFrame* frame_;
+  int inlined_frame_index_;
   std::unique_ptr<DeoptimizedFrameInfo> deoptimized_frame_;
   wasm::WasmInterpreter::FramePtr wasm_interpreted_frame_;
   Isolate* isolate_;

src/debug/debug-scope-iterator.cc

@@ -75,26 +75,8 @@ void DebugScopeIterator::Advance() {
 }
 
 bool DebugScopeIterator::ShouldIgnore() {
-  // Almost always Script scope will be empty, so just filter out that noise.
-  // Also drop empty Block, Eval and Script scopes, should we get any.
-  DCHECK(!Done());
-  debug::ScopeIterator::ScopeType type = GetType();
-  if (type != debug::ScopeIterator::ScopeTypeBlock &&
-      type != debug::ScopeIterator::ScopeTypeScript &&
-      type != debug::ScopeIterator::ScopeTypeEval &&
-      type != debug::ScopeIterator::ScopeTypeModule) {
-    return false;
-  }
-
-  // TODO(kozyatinskiy): make this function faster.
-  Handle<JSObject> value;
-  if (!iterator_.ScopeObject().ToHandle(&value)) return false;
-  Handle<FixedArray> keys =
-      KeyAccumulator::GetKeys(value, KeyCollectionMode::kOwnOnly,
-                              ENUMERABLE_STRINGS,
-                              GetKeysConversion::kConvertToString)
-          .ToHandleChecked();
-  return keys->length() == 0;
+  if (GetType() == debug::ScopeIterator::ScopeTypeLocal) return false;
+  return !iterator_.DeclaresLocals(i::ScopeIterator::Mode::ALL);
 }
 
 v8::debug::ScopeIterator::ScopeType DebugScopeIterator::GetType() {
@@ -104,11 +86,8 @@ v8::debug::ScopeIterator::ScopeType DebugScopeIterator::GetType() {
 
 v8::Local<v8::Object> DebugScopeIterator::GetObject() {
   DCHECK(!Done());
-  Handle<JSObject> value;
-  if (iterator_.ScopeObject().ToHandle(&value)) {
-    return Utils::ToLocal(value);
-  }
-  return v8::Local<v8::Object>();
+  Handle<JSObject> value = iterator_.ScopeObject(i::ScopeIterator::Mode::ALL);
+  return Utils::ToLocal(value);
 }
 
 v8::Local<v8::Function> DebugScopeIterator::GetFunction() {

src/debug/debug-scopes.h

@@ -48,32 +48,35 @@ class ScopeIterator {
 
   ScopeIterator(Isolate* isolate, Handle<JSFunction> function);
   ScopeIterator(Isolate* isolate, Handle<JSGeneratorObject> generator);
+  ~ScopeIterator();
 
-  V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> MaterializeScopeDetails();
+  Handle<JSObject> MaterializeScopeDetails();
 
   // More scopes?
-  bool Done() { return context_.is_null(); }
+  bool Done() const { return context_.is_null(); }
 
   // Move to the next scope.
   void Next();
 
+  // Restart to the first scope and context.
+  void Restart();
+
   // Return the type of the current scope.
-  ScopeType Type();
+  ScopeType Type() const;
+
+  // Indicates which variables should be visited. Either only variables from the
+  // scope that are available on the stack, or all variables.
+  enum class Mode { STACK, ALL };
 
   // Return the JavaScript object with the content of the current scope.
-  MaybeHandle<JSObject> ScopeObject();
+  Handle<JSObject> ScopeObject(Mode mode);
 
-  bool HasContext();
+  // Returns whether the current scope declares any variables.
+  bool DeclaresLocals(Mode mode) const;
 
   // Set variable value and return true on success.
   bool SetVariableValue(Handle<String> variable_name, Handle<Object> new_value);
 
-  Handle<ScopeInfo> CurrentScopeInfo();
-
-  // Return the context for this scope. For the local context there might not
-  // be an actual context.
-  Handle<Context> CurrentContext();
-
   // Populate the set with collected non-local variable names.
   Handle<StringSet> GetNonLocals();
 
@@ -94,96 +97,63 @@ class ScopeIterator {
   void DebugPrint();
 #endif
 
- private:
-  struct ExtendedScopeInfo {
-    ExtendedScopeInfo(Handle<ScopeInfo> info, int start, int end)
-        : scope_info(info), start_position(start), end_position(end) {}
-    explicit ExtendedScopeInfo(Handle<ScopeInfo> info)
-        : scope_info(info), start_position(-1), end_position(-1) {}
-    Handle<ScopeInfo> scope_info;
-    int start_position;
-    int end_position;
-    bool is_hidden() { return start_position == -1 && end_position == -1; }
-  };
+  bool InInnerScope() const { return !function_.is_null(); }
+  bool HasContext() const;
+  Handle<Context> CurrentContext() const {
+    DCHECK(HasContext());
+    return context_;
+  }
 
+ private:
   Isolate* isolate_;
+  ParseInfo* info_ = nullptr;
   FrameInspector* const frame_inspector_ = nullptr;
   Handle<JSGeneratorObject> generator_;
   Handle<JSFunction> function_;
-  Handle<ScopeInfo> function_scope_info_;
   Handle<Context> context_;
   Handle<Script> script_;
-  std::vector<ExtendedScopeInfo> nested_scope_chain_;
   Handle<StringSet> non_locals_;
-  bool seen_script_scope_;
+  DeclarationScope* closure_scope_ = nullptr;
+  Scope* start_scope_ = nullptr;
+  Scope* current_scope_ = nullptr;
+  bool seen_script_scope_ = false;
 
-  inline JavaScriptFrame* GetFrame() {
-    return frame_inspector_->GetArgumentsFrame();
+  inline JavaScriptFrame* GetFrame() const {
+    return frame_inspector_->javascript_frame();
   }
 
-  Handle<Context> GetContext();
   int GetSourcePosition();
 
-  void MaterializeStackLocals(Handle<JSObject> local_scope,
-                              Handle<ScopeInfo> scope_info);
-
   void TryParseAndRetrieveScopes(ScopeIterator::Option option);
 
   void RetrieveScopeChain(DeclarationScope* scope);
 
-  void CollectNonLocals(ParseInfo* info, DeclarationScope* scope);
-
   void UnwrapEvaluationContext();
 
-  V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> MaterializeScriptScope();
-  V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> MaterializeLocalScope();
-  V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> MaterializeModuleScope();
-  Handle<JSObject> MaterializeClosure();
-  Handle<JSObject> MaterializeInnerScope();
+  typedef std::function<bool(Handle<String> name, Handle<Object> value)>
+      Visitor;
+
   Handle<JSObject> WithContextExtension();
 
   bool SetLocalVariableValue(Handle<String> variable_name,
                              Handle<Object> new_value);
-  bool SetInnerScopeVariableValue(Handle<String> variable_name,
-                                  Handle<Object> new_value);
-  bool SetClosureVariableValue(Handle<String> variable_name,
+  bool SetContextVariableValue(Handle<String> variable_name,
                                Handle<Object> new_value);
+  bool SetContextExtensionValue(Handle<String> variable_name,
+                                Handle<Object> new_value);
   bool SetScriptVariableValue(Handle<String> variable_name,
                               Handle<Object> new_value);
   bool SetModuleVariableValue(Handle<String> variable_name,
                               Handle<Object> new_value);
 
   // Helper functions.
-  bool SetParameterValue(Handle<ScopeInfo> scope_info,
-                         Handle<String> parameter_name,
-                         Handle<Object> new_value);
-  bool SetStackVariableValue(Handle<ScopeInfo> scope_info,
-                             Handle<String> variable_name,
-                             Handle<Object> new_value);
-  bool SetContextVariableValue(Handle<ScopeInfo> scope_info,
-                               Handle<Context> context,
-                               Handle<String> variable_name,
-                               Handle<Object> new_value);
-
-  void CopyContextLocalsToScopeObject(Handle<ScopeInfo> scope_info,
-                                      Handle<Context> context,
-                                      Handle<JSObject> scope_object);
-  void CopyModuleVarsToScopeObject(Handle<ScopeInfo> scope_info,
-                                   Handle<Context> context,
-                                   Handle<JSObject> scope_object);
-  void CopyContextExtensionToScopeObject(Handle<Context> context,
-                                         Handle<JSObject> scope_object,
-                                         KeyCollectionMode mode);
-
-  // Get the chain of nested scopes within this scope for the source statement
-  // position. The scopes will be added to the list from the outermost scope to
-  // the innermost scope. Only nested block, catch or with scopes are tracked
-  // and will be returned, but no inner function scopes.
-  void GetNestedScopeChain(Isolate* isolate, Scope* scope,
-                           int statement_position);
-
-  bool HasNestedScopeChain() const;
-  ExtendedScopeInfo& LastNestedScopeChain();
+  void VisitScope(const Visitor& visitor, Mode mode) const;
+  void VisitLocalScope(const Visitor& visitor, Mode mode) const;
+  void VisitScriptScope(const Visitor& visitor) const;
+  void VisitModuleScope(const Visitor& visitor) const;
+  bool VisitLocals(const Visitor& visitor, Mode mode) const;
+  bool VisitContextLocals(const Visitor& visitor, Handle<ScopeInfo> scope_info,
+                          Handle<Context> context) const;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(ScopeIterator);
 };

src/deoptimizer.cc

@@ -2198,52 +2198,28 @@ Handle<Object> GetValueForDebugger(TranslatedFrame::iterator it,
 DeoptimizedFrameInfo::DeoptimizedFrameInfo(TranslatedState* state,
                                            TranslatedState::iterator frame_it,
                                            Isolate* isolate) {
-  // If the previous frame is an adaptor frame, we will take the parameters
-  // from there.
-  TranslatedState::iterator parameter_frame = frame_it;
-  if (parameter_frame != state->begin()) {
-    parameter_frame--;
-  }
-  int parameter_count;
-  if (parameter_frame->kind() == TranslatedFrame::kArgumentsAdaptor) {
-    parameter_count = parameter_frame->height() - 1;  // Ignore the receiver.
-  } else {
-    parameter_frame = frame_it;
-    parameter_count =
-        frame_it->shared_info()->internal_formal_parameter_count();
-  }
-  TranslatedFrame::iterator parameter_it = parameter_frame->begin();
-  parameter_it++;  // Skip the function.
-  parameter_it++;  // Skip the receiver.
+  int parameter_count =
+      frame_it->shared_info()->internal_formal_parameter_count();
+  TranslatedFrame::iterator stack_it = frame_it->begin();
 
-  // Figure out whether there is a construct stub frame on top of
-  // the parameter frame.
-  has_construct_stub_ =
-      parameter_frame != state->begin() &&
-      (parameter_frame - 1)->kind() == TranslatedFrame::kConstructStub;
+  // Get the function. Note that this might materialize the function.
+  // In case the debugger mutates this value, we should deoptimize
+  // the function and remember the value in the materialized value store.
+  function_ = Handle<JSFunction>::cast(stack_it->GetValue());
+  stack_it++;  // Skip the function.
+  stack_it++;  // Skip the receiver.
 
   DCHECK_EQ(TranslatedFrame::kInterpretedFunction, frame_it->kind());
   source_position_ = Deoptimizer::ComputeSourcePositionFromBytecodeArray(
       *frame_it->shared_info(), frame_it->node_id());
 
-  TranslatedFrame::iterator value_it = frame_it->begin();
-  // Get the function. Note that this might materialize the function.
-  // In case the debugger mutates this value, we should deoptimize
-  // the function and remember the value in the materialized value store.
-  function_ = Handle<JSFunction>::cast(value_it->GetValue());
+  DCHECK_EQ(parameter_count,
+            function_->shared()->internal_formal_parameter_count());
 
   parameters_.resize(static_cast<size_t>(parameter_count));
   for (int i = 0; i < parameter_count; i++) {
-    Handle<Object> parameter = GetValueForDebugger(parameter_it, isolate);
+    Handle<Object> parameter = GetValueForDebugger(stack_it, isolate);
     SetParameter(i, parameter);
-    parameter_it++;
-  }
-
-  // Skip the function, the receiver and the arguments.
-  int skip_count =
-      frame_it->shared_info()->internal_formal_parameter_count() + 2;
-  TranslatedFrame::iterator stack_it = frame_it->begin();
-  for (int i = 0; i < skip_count; i++) {
     stack_it++;
   }
 

src/deoptimizer.h

@@ -1025,12 +1025,6 @@ class DeoptimizedFrameInfo : public Malloced {
   // Get the frame context.
   Handle<Object> GetContext() { return context_; }
 
-  // Check if this frame is preceded by construct stub frame.  The bottom-most
-  // inlined frame might still be called by an uninlined construct stub.
-  bool HasConstructStub() {
-    return has_construct_stub_;
-  }
-
   // Get an incoming argument.
   Handle<Object> GetParameter(int index) {
     DCHECK(0 <= index && index < parameters_count());
@@ -1062,7 +1056,6 @@ class DeoptimizedFrameInfo : public Malloced {
 
   Handle<JSFunction> function_;
   Handle<Object> context_;
-  bool has_construct_stub_;
   std::vector<Handle<Object> > parameters_;
   std::vector<Handle<Object> > expression_stack_;
   int source_position_;

src/objects/scope-info.cc

@@ -740,6 +740,7 @@ int ScopeInfo::StackSlotIndex(String* name) const {
 int ScopeInfo::ModuleIndex(Handle<String> name, VariableMode* mode,
                            InitializationFlag* init_flag,
                            MaybeAssignedFlag* maybe_assigned_flag) {
+  DCHECK(name->IsInternalizedString());
   DCHECK_EQ(scope_type(), MODULE_SCOPE);
   DCHECK_NOT_NULL(mode);
   DCHECK_NOT_NULL(init_flag);

src/runtime/runtime-debug.cc

@@ -344,7 +344,7 @@ RUNTIME_FUNCTION(Runtime_GetGeneratorScopeDetails) {
     return isolate->heap()->undefined_value();
   }
 
-  RETURN_RESULT_OR_FAILURE(isolate, it.MaterializeScopeDetails());
+  return *it.MaterializeScopeDetails();
 }
 
 static bool SetScopeVariableValue(ScopeIterator* it, int index,

test/cctest/test-parsing.cc

@@ -648,8 +648,7 @@ TEST(ScopeUsesArgumentsSuperThis) {
       CHECK(i::Rewriter::Rewrite(&info));
       info.ast_value_factory()->Internalize(isolate);
       CHECK(i::DeclarationScope::Analyze(&info));
-      i::DeclarationScope::AllocateScopeInfos(&info, isolate,
-                                              i::AnalyzeMode::kRegular);
+      i::DeclarationScope::AllocateScopeInfos(&info, isolate);
       CHECK_NOT_NULL(info.literal());
 
       i::DeclarationScope* script_scope = info.literal()->scope();
@@ -10126,8 +10125,7 @@ TEST(LexicalLoopVariable) {
     CHECK(i::parsing::ParseProgram(&info, isolate));
     CHECK(i::Rewriter::Rewrite(&info));
     CHECK(i::DeclarationScope::Analyze(&info));
-    i::DeclarationScope::AllocateScopeInfos(&info, isolate,
-                                            i::AnalyzeMode::kRegular);
+    i::DeclarationScope::AllocateScopeInfos(&info, isolate);
     CHECK_NOT_NULL(info.literal());
 
     i::DeclarationScope* script_scope = info.literal()->scope();

test/debugger/debug/harmony/modules-debug-scopes2.js

@@ -142,7 +142,7 @@ listener_delegate = function(exec_state) {
       {exported_var: undefined, imported_var: undefined},
       0, exec_state);
   CheckScopeDoesNotHave(
-      ["local_var", "doesntexist", "local_let", "exported_let", "imported_let"],
+      ["doesntexist", "exported_let", "imported_let"],
       0, exec_state);
 };
 debugger;
@@ -163,7 +163,7 @@ listener_delegate = function(exec_state) {
   CheckScopeContent(
       {exported_let: 3, exported_var: 4,
        imported_let: 3, imported_var: 4}, 0, exec_state);
-  CheckScopeDoesNotHave(["local_var", "local_let"], 0, exec_state);
+  CheckScopeDoesNotHave([], 0, exec_state);
 };
 debugger;
 EndTest();
@@ -181,7 +181,7 @@ listener_delegate = function(exec_state) {
   CheckScopeContent(
       {exported_let: 13, exported_var: 14,
        imported_let: 13, imported_var: 14}, 0, exec_state);
-  CheckScopeDoesNotHave(["local_var", "local_let"], 0, exec_state);
+  CheckScopeDoesNotHave([], 0, exec_state);
 };
 debugger;
 EndTest();

test/inspector/debugger/evaluate-on-call-frame-in-module-expected.txt

@@ -4,7 +4,7 @@ Running test: testTotal
 foo1 (module1:7:2)
 foo2 (module2:6:9)
 (anonymous) (module3:4:0)
-local:foo1
+local
 [
     [0] : c1 = 12
     [1] : g1 = 2
@@ -94,7 +94,7 @@ foo1 =
     objectId : <objectId>
     type : function
 }
-local:foo2
+local
 [
     [0] : c2 = 22
 ]
@@ -180,7 +180,8 @@ foo2 =
 module
 [
     [0] : foo2 = function foo2() {   let c2 = 22;   return foo1() + a2 + b2 + c2; }
-    [1] : b3 = 31
+    [1] : a3 = 30
+    [2] : b3 = 31
 ]
 global
 [
@@ -205,6 +206,20 @@ foo2 =
     objectId : <objectId>
     type : function
 }
+a3 = 
+{
+    description : 30
+    type : number
+    value : 30
+}
+Evaluating: ++a3
+updated a3 = 
+{
+    description : 31
+    type : number
+    value : 31
+}
+Evaluating: --a3
 b3 = 
 {
     description : 31
@@ -224,7 +239,7 @@ Running test: testAnother
 (anonymous) (module4:5:13)
 bar (module4:5:24)
 (anonymous) (module4:7:0)
-local
+local:bar
 [
 ]
 closure:bar
@@ -295,6 +310,12 @@ updated a =
     value : 1
 }
 Evaluating: --a
+module
+[
+    [0] : a = 1
+    [1] : b = 2
+    [2] : bar = function bar() {   let a = 0;   (() => {a; debugger;})(); }
+]
 global
 [
   ...
@@ -310,6 +331,41 @@ Array =
     objectId : <objectId>
     type : function
 }
+a = 
+{
+    description : 1
+    type : number
+    value : 1
+}
+Evaluating: ++a
+updated a = 
+{
+    description : 2
+    type : number
+    value : 2
+}
+Evaluating: --a
+b = 
+{
+    description : 2
+    type : number
+    value : 2
+}
+Evaluating: ++b
+updated b = 
+{
+    description : 3
+    type : number
+    value : 3
+}
+Evaluating: --b
+bar = 
+{
+    className : Function
+    description : function bar() {   let a = 0;   (() => {a; debugger;})(); }
+    objectId : <objectId>
+    type : function
+}
 
 Running test: testDifferentModuleVariables
 (anonymous) (module5:5:0)
@@ -485,6 +541,10 @@ Evaluating: --x
 
 Running test: testLocalVariableToplevel
 (anonymous) (module7:2:0)
+module
+[
+    [0] : x = 5
+]
 global
 [
   ...
@@ -501,3 +561,17 @@ Array =
     objectId : <objectId>
     type : function
 }
+x = 
+{
+    description : 5
+    type : number
+    value : 5
+}
+Evaluating: ++x
+updated x = 
+{
+    description : 6
+    type : number
+    value : 6
+}
+Evaluating: --x