[inspector] Split off interface-types.h

This CL adds a new header src/debug/interface-types.h, moves the
definition of Location from the debug-interface.h to this new header,
and adds a new definition for the WasmDisassembly types.
This allows to use the types in other implementation files or headers
without having to include the entire debug-interface.h, reducing build
dependencies and compile time (especially for incremental builds).

The WasmDisassembly type replaces the old
std::pair<std::string, std::vector<std::tuple<...>>>, which was a bit
hard to unravel.

R=yangguo@chromium.org, kozyatinskiy@chromium.org, titzer@chromium.org

Review-Url: https://codereview.chromium.org/2529383002
Cr-Commit-Position: refs/heads/master@{#41488}

BUILD.gn

@@ -1298,6 +1298,7 @@ v8_source_set("v8_base") {
     "src/debug/debug-scopes.h",
     "src/debug/debug.cc",
     "src/debug/debug.h",
+    "src/debug/interface-types.h",
     "src/debug/liveedit.cc",
     "src/debug/liveedit.h",
     "src/deoptimize-reason.cc",

src/api.cc

@@ -9003,8 +9003,8 @@ int GetSmiValue(i::Handle<i::FixedArray> array, int index) {
 }  // namespace
 
 bool DebugInterface::Script::GetPossibleBreakpoints(
-    const Location& start, const Location& end,
-    std::vector<Location>* locations) const {
+    const debug::Location& start, const debug::Location& end,
+    std::vector<debug::Location>* locations) const {
   CHECK(!start.IsEmpty());
   i::Handle<i::Script> script = Utils::OpenHandle(this);
   if (script->type() == i::Script::TYPE_WASM) {
@@ -9046,7 +9046,7 @@ bool DebugInterface::Script::GetPossibleBreakpoints(
     if (current_line_end_index > 0) {
       line_offset = GetSmiValue(line_ends, current_line_end_index - 1) + 1;
     }
-    locations->push_back(Location(
+    locations->push_back(debug::Location(
         current_line_end_index + script->line_offset(),
         offset - line_offset +
             (current_line_end_index == 0 ? script->column_offset() : 0)));
@@ -9054,7 +9054,8 @@ bool DebugInterface::Script::GetPossibleBreakpoints(
   return true;
 }
 
-int DebugInterface::Script::GetSourcePosition(const Location& location) const {
+int DebugInterface::Script::GetSourcePosition(
+    const debug::Location& location) const {
   i::Handle<i::Script> script = Utils::OpenHandle(this);
   if (script->type() == i::Script::TYPE_WASM) {
     // TODO(clemensh): Return the proper thing for wasm.
@@ -9101,26 +9102,26 @@ MaybeLocal<DebugInterface::Script> DebugInterface::Script::Wrap(
       handle_scope.CloseAndEscape(script_obj));
 }
 
-DebugInterface::Location::Location(int lineNumber, int columnNumber)
-    : lineNumber_(lineNumber), columnNumber_(columnNumber) {
-  CHECK(lineNumber >= 0);
-  CHECK(columnNumber >= 0);
+debug::Location::Location(int line_number, int column_number)
+    : line_number_(line_number), column_number_(column_number) {
+  CHECK(line_number >= 0);
+  CHECK(column_number >= 0);
 }
 
-DebugInterface::Location::Location() : lineNumber_(-1), columnNumber_(-1) {}
+debug::Location::Location() : line_number_(-1), column_number_(-1) {}
 
-int DebugInterface::Location::GetLineNumber() const {
-  CHECK(lineNumber_ >= 0);
-  return lineNumber_;
+int debug::Location::GetLineNumber() const {
+  CHECK(line_number_ >= 0);
+  return line_number_;
 }
 
-int DebugInterface::Location::GetColumnNumber() const {
-  CHECK(columnNumber_ >= 0);
-  return columnNumber_;
+int debug::Location::GetColumnNumber() const {
+  CHECK(column_number_ >= 0);
+  return column_number_;
 }
 
-bool DebugInterface::Location::IsEmpty() const {
-  return lineNumber_ == -1 && columnNumber_ == -1;
+bool debug::Location::IsEmpty() const {
+  return line_number_ == -1 && column_number_ == -1;
 }
 
 void DebugInterface::GetLoadedScripts(
@@ -9146,10 +9147,8 @@ void DebugInterface::GetLoadedScripts(
   }
 }
 
-std::pair<std::string, std::vector<std::tuple<uint32_t, int, int>>>
-DebugInterface::DisassembleWasmFunction(Isolate* v8_isolate,
-                                        Local<Object> v8_script,
-                                        int function_index) {
+debug::WasmDisassembly DebugInterface::DisassembleWasmFunction(
+    Isolate* v8_isolate, Local<Object> v8_script, int function_index) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
   if (v8_script.IsEmpty()) return {};
   i::Handle<i::Object> script_wrapper = Utils::OpenHandle(*v8_script);

src/debug/debug-interface.h

@@ -9,6 +9,8 @@
 #include "include/v8-util.h"
 #include "include/v8.h"
 
+#include "src/debug/interface-types.h"
+
 namespace v8 {
 
 class DebugInterface {
@@ -140,27 +142,6 @@ class DebugInterface {
   static void PrepareStep(Isolate* isolate, StepAction action);
   static void ClearStepping(Isolate* isolate);
 
-  /**
-   * Defines location inside script.
-   * Lines and columns are 0-based.
-   */
-  class Location {
-   public:
-    Location(int lineNumber, int columnNumber);
-    /**
-     * Create empty location.
-     */
-    Location();
-
-    int GetLineNumber() const;
-    int GetColumnNumber() const;
-    bool IsEmpty() const;
-
-   private:
-    int lineNumber_;
-    int columnNumber_;
-  };
-
   /**
    * Native wrapper around v8::internal::Script object.
    */
@@ -180,8 +161,9 @@ class DebugInterface {
     MaybeLocal<String> ContextData() const;
     MaybeLocal<String> Source() const;
     bool IsWasm() const;
-    bool GetPossibleBreakpoints(const Location& start, const Location& end,
-                                std::vector<Location>* locations) const;
+    bool GetPossibleBreakpoints(const debug::Location& start,
+                                const debug::Location& end,
+                                std::vector<debug::Location>* locations) const;
 
     /**
      * script parameter is a wrapper v8::internal::JSObject for
@@ -195,7 +177,7 @@ class DebugInterface {
                                    v8::Local<v8::Object> script);
 
    private:
-    int GetSourcePosition(const Location& location) const;
+    int GetSourcePosition(const debug::Location& location) const;
   };
 
   static void GetLoadedScripts(Isolate* isolate,
@@ -203,13 +185,10 @@ class DebugInterface {
 
   /**
    * Compute the disassembly of a wasm function.
-   * Returns the disassembly string and a list of <byte_offset, line, column>
-   * entries, mapping wasm byte offsets to line and column in the disassembly.
-   * The list is guaranteed to be ordered by the byte_offset.
    */
-  static std::pair<std::string, std::vector<std::tuple<uint32_t, int, int>>>
-  DisassembleWasmFunction(Isolate* isolate, v8::Local<v8::Object> script,
-                          int function_index);
+  static debug::WasmDisassembly DisassembleWasmFunction(
+      Isolate* isolate, v8::Local<v8::Object> script, int function_index);
+
   static MaybeLocal<UnboundScript> CompileInspectorScript(Isolate* isolate,
                                                           Local<String> source);
 };

src/debug/interface-types.h

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_DEBUG_INTERFACE_TYPES_H_
+#define V8_DEBUG_INTERFACE_TYPES_H_
+
+#include <cstdint>
+#include <string>
+#include <vector>
+
+namespace v8 {
+namespace debug {
+
+/**
+ * Defines location inside script.
+ * Lines and columns are 0-based.
+ */
+class Location {
+ public:
+  Location(int line_number, int column_number);
+  /**
+   * Create empty location.
+   */
+  Location();
+
+  int GetLineNumber() const;
+  int GetColumnNumber() const;
+  bool IsEmpty() const;
+
+ private:
+  int line_number_;
+  int column_number_;
+};
+
+/**
+ * The result of disassembling a wasm function.
+ * Consists of the disassembly string and an offset table mapping wasm byte
+ * offsets to line and column in the disassembly.
+ * The offset table entries are ordered by the byte_offset.
+ * All numbers are 0-based.
+ */
+struct WasmDisassemblyOffsetTableEntry {
+  WasmDisassemblyOffsetTableEntry(uint32_t byte_offset, int line, int column)
+      : byte_offset(byte_offset), line(line), column(column) {}
+
+  uint32_t byte_offset;
+  int line;
+  int column;
+};
+struct WasmDisassembly {
+  using OffsetTable = std::vector<WasmDisassemblyOffsetTableEntry>;
+  WasmDisassembly() {}
+  WasmDisassembly(std::string disassembly, OffsetTable offset_table)
+      : disassembly(std::move(disassembly)),
+        offset_table(std::move(offset_table)) {}
+
+  std::string disassembly;
+  OffsetTable offset_table;
+};
+
+}  // namespace debug
+}  // namespace v8
+
+#endif  // V8_DEBUG_INTERFACE_TYPES_H_

src/inspector/v8-debugger-agent-impl.cc

@@ -385,9 +385,9 @@ Response V8DebuggerAgentImpl::getPossibleBreakpoints(
     return Response::Error(
         "start.lineNumber and start.columnNumber should be >= 0");
 
-  v8::DebugInterface::Location v8Start(start->getLineNumber(),
-                                       start->getColumnNumber(0));
-  v8::DebugInterface::Location v8End;
+  v8::debug::Location v8Start(start->getLineNumber(),
+                              start->getColumnNumber(0));
+  v8::debug::Location v8End;
   if (end.isJust()) {
     if (end.fromJust()->getScriptId() != scriptId)
       return Response::Error("Locations should contain the same scriptId");
@@ -396,12 +396,12 @@ Response V8DebuggerAgentImpl::getPossibleBreakpoints(
     if (line < 0 || column < 0)
       return Response::Error(
           "end.lineNumber and end.columnNumber should be >= 0");
-    v8End = v8::DebugInterface::Location(line, column);
+    v8End = v8::debug::Location(line, column);
   }
   auto it = m_scripts.find(scriptId);
   if (it == m_scripts.end()) return Response::Error("Script not found");
 
-  std::vector<v8::DebugInterface::Location> v8Locations;
+  std::vector<v8::debug::Location> v8Locations;
   if (!it->second->getPossibleBreakpoints(v8Start, v8End, &v8Locations))
     return Response::InternalError();
 

src/inspector/v8-debugger-script.cc

@@ -173,9 +173,8 @@ void V8DebuggerScript::setSource(v8::Local<v8::String> source) {
 }
 
 bool V8DebuggerScript::getPossibleBreakpoints(
-    const v8::DebugInterface::Location& start,
-    const v8::DebugInterface::Location& end,
-    std::vector<v8::DebugInterface::Location>* locations) {
+    const v8::debug::Location& start, const v8::debug::Location& end,
+    std::vector<v8::debug::Location>* locations) {
   v8::HandleScope scope(m_isolate);
   v8::Local<v8::DebugInterface::Script> script = m_script.Get(m_isolate);
   return script->GetPossibleBreakpoints(start, end, locations);

src/inspector/v8-debugger-script.h

@@ -67,10 +67,9 @@ class V8DebuggerScript {
   void setSourceMappingURL(const String16&);
   void setSource(v8::Local<v8::String>);
 
-  bool getPossibleBreakpoints(
-      const v8::DebugInterface::Location& start,
-      const v8::DebugInterface::Location& end,
-      std::vector<v8::DebugInterface::Location>* locations);
+  bool getPossibleBreakpoints(const v8::debug::Location& start,
+                              const v8::debug::Location& end,
+                              std::vector<v8::debug::Location>* locations);
 
  private:
   String16 m_id;

src/inspector/wasm-translation.cc

@@ -72,7 +72,7 @@ class WasmTranslation::TranslatorImpl::RawTranslator
 
 class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     : public WasmTranslation::TranslatorImpl {
-  using OffsetTable = std::vector<std::tuple<uint32_t, int, int>>;
+  using OffsetTable = debug::WasmDisassembly::OffsetTable;
 
  public:
   DisassemblingTranslator(Isolate *isolate, Local<Object> script)
@@ -88,7 +88,7 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     unsigned right = static_cast<unsigned>(offset_table.size());  // exclusive
     while (right - left > 1) {
       unsigned mid = (left + right) / 2;
-      if (std::get<0>(offset_table[mid]) <= byte_offset) {
+      if (offset_table[mid].byte_offset <= byte_offset) {
         left = mid;
       } else {
         right = mid;
@@ -96,9 +96,9 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     }
 
     loc->script_id = GetFakeScriptId(loc);
-    if (std::get<0>(offset_table[left]) == byte_offset) {
-      loc->line = std::get<1>(offset_table[left]);
-      loc->column = std::get<2>(offset_table[left]);
+    if (offset_table[left].byte_offset == byte_offset) {
+      loc->line = offset_table[left].line;
+      loc->column = offset_table[left].column;
     } else {
       loc->line = 0;
       loc->column = 0;
@@ -117,9 +117,8 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     while (right - left > 1) {
       unsigned mid = (left + right) / 2;
       auto &entry = (*reverse_table)[mid];
-      if (std::get<1>(entry) < loc->line ||
-          (std::get<1>(entry) == loc->line &&
-           std::get<2>(entry) <= loc->column)) {
+      if (entry.line < loc->line ||
+          (entry.line == loc->line && entry.column <= loc->column)) {
         left = mid;
       } else {
         right = mid;
@@ -129,12 +128,12 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     int found_byte_offset = 0;
     // If we found an exact match, use it. Otherwise check whether the next
     // bigger entry is still in the same line. Report that one then.
-    if (std::get<1>((*reverse_table)[left]) == loc->line &&
-        std::get<2>((*reverse_table)[left]) == loc->column) {
-      found_byte_offset = std::get<0>((*reverse_table)[left]);
+    if ((*reverse_table)[left].line == loc->line &&
+        (*reverse_table)[left].column == loc->column) {
+      found_byte_offset = (*reverse_table)[left].byte_offset;
     } else if (left + 1 < reverse_table->size() &&
-               std::get<1>((*reverse_table)[left + 1]) == loc->line) {
-      found_byte_offset = std::get<0>((*reverse_table)[left + 1]);
+               (*reverse_table)[left + 1].line == loc->line) {
+      found_byte_offset = (*reverse_table)[left + 1].byte_offset;
     }
 
     v8::Isolate *isolate = loc->translation->isolate_;
@@ -172,17 +171,19 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
     if (it != offset_tables_.end()) return it->second;
 
     v8::Isolate *isolate = loc->translation->isolate_;
-    std::pair<std::string, OffsetTable> disassembly =
+    debug::WasmDisassembly disassembly_result =
         DebugInterface::DisassembleWasmFunction(isolate, script_.Get(isolate),
                                                 func_index);
 
     it = offset_tables_
-             .insert(std::make_pair(func_index, std::move(disassembly.second)))
+             .insert(std::make_pair(func_index,
+                                    std::move(disassembly_result.offset_table)))
              .first;
 
     String16 fake_script_id = GetFakeScriptId(loc);
     String16 fake_script_url = GetFakeScriptUrl(loc);
-    String16 source(disassembly.first.data(), disassembly.first.length());
+    String16 source(disassembly_result.disassembly.data(),
+                    disassembly_result.disassembly.length());
     std::unique_ptr<V8DebuggerScript> fake_script(new V8DebuggerScript(
         fake_script_id, std::move(fake_script_url), source));
 
@@ -202,13 +203,10 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
 
     OffsetTable reverse_table = it->second;
     // Order by line, column, then byte offset.
-    auto cmp = [](std::tuple<uint32_t, int, int> el1,
-                  std::tuple<uint32_t, int, int> el2) {
-      if (std::get<1>(el1) != std::get<1>(el2))
-        return std::get<1>(el1) < std::get<1>(el2);
-      if (std::get<2>(el1) != std::get<2>(el2))
-        return std::get<2>(el1) < std::get<2>(el2);
-      return std::get<0>(el1) < std::get<0>(el2);
+    auto cmp = [](OffsetTable::value_type el1, OffsetTable::value_type el2) {
+      if (el1.line != el2.line) return el1.line < el2.line;
+      if (el1.column != el2.column) return el1.column < el2.column;
+      return el1.byte_offset < el2.byte_offset;
     };
     std::sort(reverse_table.begin(), reverse_table.end(), cmp);
 

src/v8.gyp

@@ -836,6 +836,7 @@
         'debug/debug-scopes.h',
         'debug/debug.cc',
         'debug/debug.h',
+        'debug/interface-types.h',
         'debug/liveedit.cc',
         'debug/liveedit.h',
         'deoptimize-reason.cc',

src/wasm/wasm-module.cc

@@ -6,7 +6,8 @@
 
 #include "src/base/atomic-utils.h"
 #include "src/code-stubs.h"
-
+#include "src/compiler/wasm-compiler.h"
+#include "src/debug/interface-types.h"
 #include "src/macro-assembler.h"
 #include "src/objects.h"
 #include "src/property-descriptor.h"
@@ -22,8 +23,6 @@
 #include "src/wasm/wasm-objects.h"
 #include "src/wasm/wasm-result.h"
 
-#include "src/compiler/wasm-compiler.h"
-
 using namespace v8::internal;
 using namespace v8::internal::wasm;
 namespace base = v8::base;

src/wasm/wasm-module.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "src/api.h"
+#include "src/debug/debug-interface.h"
 #include "src/globals.h"
 #include "src/handles.h"
 #include "src/parsing/preparse-data.h"

src/wasm/wasm-objects.cc

@@ -540,8 +540,8 @@ int WasmCompiledModule::GetAsmJsSourcePosition(
   return offset_table->get_int(2 * left + 1);
 }
 
-std::pair<std::string, std::vector<std::tuple<uint32_t, int, int>>>
-WasmCompiledModule::DisassembleFunction(int func_index) {
+v8::debug::WasmDisassembly WasmCompiledModule::DisassembleFunction(
+    int func_index) {
   DisallowHeapAllocation no_gc;
 
   if (func_index < 0 ||
@@ -553,7 +553,7 @@ WasmCompiledModule::DisassembleFunction(int func_index) {
                                   module_bytes_str->length());
 
   std::ostringstream disassembly_os;
-  std::vector<std::tuple<uint32_t, int, int>> offset_table;
+  v8::debug::WasmDisassembly::OffsetTable offset_table;
 
   PrintWasmText(module(), module_bytes, static_cast<uint32_t>(func_index),
                 disassembly_os, &offset_table);

src/wasm/wasm-objects.h

@@ -5,6 +5,7 @@
 #ifndef V8_WASM_OBJECTS_H_
 #define V8_WASM_OBJECTS_H_
 
+#include "src/debug/interface-types.h"
 #include "src/objects-inl.h"
 #include "src/trap-handler/trap-handler.h"
 #include "src/wasm/managed.h"
@@ -302,8 +303,7 @@ class WasmCompiledModule : public FixedArray {
   // entries, mapping wasm byte offsets to line and column in the disassembly.
   // The list is guaranteed to be ordered by the byte_offset.
   // Returns an empty string and empty vector if the function index is invalid.
-  std::pair<std::string, std::vector<std::tuple<uint32_t, int, int>>>
-  DisassembleFunction(int func_index);
+  debug::WasmDisassembly DisassembleFunction(int func_index);
 
  private:
   void InitId();

src/wasm/wasm-text.cc

@@ -4,6 +4,7 @@
 
 #include "src/wasm/wasm-text.h"
 
+#include "src/debug/interface-types.h"
 #include "src/ostreams.h"
 #include "src/vector.h"
 #include "src/wasm/ast-decoder.h"
@@ -11,6 +12,7 @@
 #include "src/wasm/wasm-opcodes.h"
 #include "src/zone/zone.h"
 
+using namespace v8;
 using namespace v8::internal;
 using namespace v8::internal::wasm;
 
@@ -128,10 +130,10 @@ bool IsValidFunctionName(const Vector<const char> &name) {
 
 }  // namespace
 
-void wasm::PrintWasmText(
-    const WasmModule *module, const ModuleWireBytes &wire_bytes,
-    uint32_t func_index, std::ostream &os,
-    std::vector<std::tuple<uint32_t, int, int>> *offset_table) {
+void wasm::PrintWasmText(const WasmModule *module,
+                         const ModuleWireBytes &wire_bytes, uint32_t func_index,
+                         std::ostream &os,
+                         debug::WasmDisassembly::OffsetTable *offset_table) {
   DCHECK_NOT_NULL(module);
   DCHECK_GT(module->functions.size(), func_index);
   const WasmFunction *fun = &module->functions[func_index];
@@ -189,8 +191,8 @@ void wasm::PrintWasmText(
     const int kMaxIndentation = 64;
     int indentation = std::min(kMaxIndentation, 2 * control_depth);
     if (offset_table) {
-      offset_table->push_back(
-          std::make_tuple(i.pc_offset(), line_nr, indentation));
+      offset_table->push_back(debug::WasmDisassemblyOffsetTableEntry(
+          i.pc_offset(), line_nr, indentation));
     }
 
     // 64 whitespaces

src/wasm/wasm-text.h

@@ -11,8 +11,12 @@
 #include <vector>
 
 namespace v8 {
-namespace internal {
 
+namespace debug {
+struct WasmDisassemblyOffsetTableEntry;
+}  // namespace debug
+
+namespace internal {
 namespace wasm {
 
 // Forward declaration.
@@ -22,9 +26,10 @@ struct ModuleWireBytes;
 // Generate disassembly according to official text format.
 // Output disassembly to the given output stream, and optionally return an
 // offset table of <byte offset, line, column> via the given pointer.
-void PrintWasmText(const WasmModule *module, const ModuleWireBytes &wire_bytes,
-                   uint32_t func_index, std::ostream &os,
-                   std::vector<std::tuple<uint32_t, int, int>> *offset_table);
+void PrintWasmText(
+    const WasmModule *module, const ModuleWireBytes &wire_bytes,
+    uint32_t func_index, std::ostream &os,
+    std::vector<debug::WasmDisassemblyOffsetTableEntry> *offset_table);
 
 }  // namespace wasm
 }  // namespace internal