Skip to content

Commit f5fb2da

Browse files
backesCommit bot
backes
authored and
Commit bot
committed
[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}
1 parent 82061d6 commit f5fb2da

15 files changed

Lines changed: 152 additions & 104 deletions

‎BUILD.gn‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,6 +1298,7 @@ v8_source_set("v8_base") {
12981298
"src/debug/debug-scopes.h",
12991299
"src/debug/debug.cc",
13001300
"src/debug/debug.h",
1301+
"src/debug/interface-types.h",
13011302
"src/debug/liveedit.cc",
13021303
"src/debug/liveedit.h",
13031304
"src/deoptimize-reason.cc",

‎src/api.cc‎

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9003,8 +9003,8 @@ int GetSmiValue(i::Handle<i::FixedArray> array, int index) {
90039003
} // namespace
90049004

90059005
bool DebugInterface::Script::GetPossibleBreakpoints(
9006-
const Location& start, const Location& end,
9007-
std::vector<Location>* locations) const {
9006+
const debug::Location& start, const debug::Location& end,
9007+
std::vector<debug::Location>* locations) const {
90089008
CHECK(!start.IsEmpty());
90099009
i::Handle<i::Script> script = Utils::OpenHandle(this);
90109010
if (script->type() == i::Script::TYPE_WASM) {
@@ -9046,15 +9046,16 @@ bool DebugInterface::Script::GetPossibleBreakpoints(
90469046
if (current_line_end_index > 0) {
90479047
line_offset = GetSmiValue(line_ends, current_line_end_index - 1) + 1;
90489048
}
9049-
locations->push_back(Location(
9049+
locations->push_back(debug::Location(
90509050
current_line_end_index + script->line_offset(),
90519051
offset - line_offset +
90529052
(current_line_end_index == 0 ? script->column_offset() : 0)));
90539053
}
90549054
return true;
90559055
}
90569056

9057-
int DebugInterface::Script::GetSourcePosition(const Location& location) const {
9057+
int DebugInterface::Script::GetSourcePosition(
9058+
const debug::Location& location) const {
90589059
i::Handle<i::Script> script = Utils::OpenHandle(this);
90599060
if (script->type() == i::Script::TYPE_WASM) {
90609061
// TODO(clemensh): Return the proper thing for wasm.
@@ -9101,26 +9102,26 @@ MaybeLocal<DebugInterface::Script> DebugInterface::Script::Wrap(
91019102
handle_scope.CloseAndEscape(script_obj));
91029103
}
91039104

9104-
DebugInterface::Location::Location(int lineNumber, int columnNumber)
9105-
: lineNumber_(lineNumber), columnNumber_(columnNumber) {
9106-
CHECK(lineNumber >= 0);
9107-
CHECK(columnNumber >= 0);
9105+
debug::Location::Location(int line_number, int column_number)
9106+
: line_number_(line_number), column_number_(column_number) {
9107+
CHECK(line_number >= 0);
9108+
CHECK(column_number >= 0);
91089109
}
91099110

9110-
DebugInterface::Location::Location() : lineNumber_(-1), columnNumber_(-1) {}
9111+
debug::Location::Location() : line_number_(-1), column_number_(-1) {}
91119112

9112-
int DebugInterface::Location::GetLineNumber() const {
9113-
CHECK(lineNumber_ >= 0);
9114-
return lineNumber_;
9113+
int debug::Location::GetLineNumber() const {
9114+
CHECK(line_number_ >= 0);
9115+
return line_number_;
91159116
}
91169117

9117-
int DebugInterface::Location::GetColumnNumber() const {
9118-
CHECK(columnNumber_ >= 0);
9119-
return columnNumber_;
9118+
int debug::Location::GetColumnNumber() const {
9119+
CHECK(column_number_ >= 0);
9120+
return column_number_;
91209121
}
91219122

9122-
bool DebugInterface::Location::IsEmpty() const {
9123-
return lineNumber_ == -1 && columnNumber_ == -1;
9123+
bool debug::Location::IsEmpty() const {
9124+
return line_number_ == -1 && column_number_ == -1;
91249125
}
91259126

91269127
void DebugInterface::GetLoadedScripts(
@@ -9146,10 +9147,8 @@ void DebugInterface::GetLoadedScripts(
91469147
}
91479148
}
91489149

9149-
std::pair<std::string, std::vector<std::tuple<uint32_t, int, int>>>
9150-
DebugInterface::DisassembleWasmFunction(Isolate* v8_isolate,
9151-
Local<Object> v8_script,
9152-
int function_index) {
9150+
debug::WasmDisassembly DebugInterface::DisassembleWasmFunction(
9151+
Isolate* v8_isolate, Local<Object> v8_script, int function_index) {
91539152
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
91549153
if (v8_script.IsEmpty()) return {};
91559154
i::Handle<i::Object> script_wrapper = Utils::OpenHandle(*v8_script);

‎src/debug/debug-interface.h‎

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include "include/v8-util.h"
1010
#include "include/v8.h"
1111

12+
#include "src/debug/interface-types.h"
13+
1214
namespace v8 {
1315

1416
class DebugInterface {
@@ -140,27 +142,6 @@ class DebugInterface {
140142
static void PrepareStep(Isolate* isolate, StepAction action);
141143
static void ClearStepping(Isolate* isolate);
142144

143-
/**
144-
* Defines location inside script.
145-
* Lines and columns are 0-based.
146-
*/
147-
class Location {
148-
public:
149-
Location(int lineNumber, int columnNumber);
150-
/**
151-
* Create empty location.
152-
*/
153-
Location();
154-
155-
int GetLineNumber() const;
156-
int GetColumnNumber() const;
157-
bool IsEmpty() const;
158-
159-
private:
160-
int lineNumber_;
161-
int columnNumber_;
162-
};
163-
164145
/**
165146
* Native wrapper around v8::internal::Script object.
166147
*/
@@ -180,8 +161,9 @@ class DebugInterface {
180161
MaybeLocal<String> ContextData() const;
181162
MaybeLocal<String> Source() const;
182163
bool IsWasm() const;
183-
bool GetPossibleBreakpoints(const Location& start, const Location& end,
184-
std::vector<Location>* locations) const;
164+
bool GetPossibleBreakpoints(const debug::Location& start,
165+
const debug::Location& end,
166+
std::vector<debug::Location>* locations) const;
185167

186168
/**
187169
* script parameter is a wrapper v8::internal::JSObject for
@@ -195,21 +177,18 @@ class DebugInterface {
195177
v8::Local<v8::Object> script);
196178

197179
private:
198-
int GetSourcePosition(const Location& location) const;
180+
int GetSourcePosition(const debug::Location& location) const;
199181
};
200182

201183
static void GetLoadedScripts(Isolate* isolate,
202184
PersistentValueVector<Script>& scripts);
203185

204186
/**
205187
* Compute the disassembly of a wasm function.
206-
* Returns the disassembly string and a list of <byte_offset, line, column>
207-
* entries, mapping wasm byte offsets to line and column in the disassembly.
208-
* The list is guaranteed to be ordered by the byte_offset.
209188
*/
210-
static std::pair<std::string, std::vector<std::tuple<uint32_t, int, int>>>
211-
DisassembleWasmFunction(Isolate* isolate, v8::Local<v8::Object> script,
212-
int function_index);
189+
static debug::WasmDisassembly DisassembleWasmFunction(
190+
Isolate* isolate, v8::Local<v8::Object> script, int function_index);
191+
213192
static MaybeLocal<UnboundScript> CompileInspectorScript(Isolate* isolate,
214193
Local<String> source);
215194
};

‎src/debug/interface-types.h‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright 2016 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef V8_DEBUG_INTERFACE_TYPES_H_
6+
#define V8_DEBUG_INTERFACE_TYPES_H_
7+
8+
#include <cstdint>
9+
#include <string>
10+
#include <vector>
11+
12+
namespace v8 {
13+
namespace debug {
14+
15+
/**
16+
* Defines location inside script.
17+
* Lines and columns are 0-based.
18+
*/
19+
class Location {
20+
public:
21+
Location(int line_number, int column_number);
22+
/**
23+
* Create empty location.
24+
*/
25+
Location();
26+
27+
int GetLineNumber() const;
28+
int GetColumnNumber() const;
29+
bool IsEmpty() const;
30+
31+
private:
32+
int line_number_;
33+
int column_number_;
34+
};
35+
36+
/**
37+
* The result of disassembling a wasm function.
38+
* Consists of the disassembly string and an offset table mapping wasm byte
39+
* offsets to line and column in the disassembly.
40+
* The offset table entries are ordered by the byte_offset.
41+
* All numbers are 0-based.
42+
*/
43+
struct WasmDisassemblyOffsetTableEntry {
44+
WasmDisassemblyOffsetTableEntry(uint32_t byte_offset, int line, int column)
45+
: byte_offset(byte_offset), line(line), column(column) {}
46+
47+
uint32_t byte_offset;
48+
int line;
49+
int column;
50+
};
51+
struct WasmDisassembly {
52+
using OffsetTable = std::vector<WasmDisassemblyOffsetTableEntry>;
53+
WasmDisassembly() {}
54+
WasmDisassembly(std::string disassembly, OffsetTable offset_table)
55+
: disassembly(std::move(disassembly)),
56+
offset_table(std::move(offset_table)) {}
57+
58+
std::string disassembly;
59+
OffsetTable offset_table;
60+
};
61+
62+
} // namespace debug
63+
} // namespace v8
64+
65+
#endif // V8_DEBUG_INTERFACE_TYPES_H_

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,9 @@ Response V8DebuggerAgentImpl::getPossibleBreakpoints(
385385
return Response::Error(
386386
"start.lineNumber and start.columnNumber should be >= 0");
387387

388-
v8::DebugInterface::Location v8Start(start->getLineNumber(),
389-
start->getColumnNumber(0));
390-
v8::DebugInterface::Location v8End;
388+
v8::debug::Location v8Start(start->getLineNumber(),
389+
start->getColumnNumber(0));
390+
v8::debug::Location v8End;
391391
if (end.isJust()) {
392392
if (end.fromJust()->getScriptId() != scriptId)
393393
return Response::Error("Locations should contain the same scriptId");
@@ -396,12 +396,12 @@ Response V8DebuggerAgentImpl::getPossibleBreakpoints(
396396
if (line < 0 || column < 0)
397397
return Response::Error(
398398
"end.lineNumber and end.columnNumber should be >= 0");
399-
v8End = v8::DebugInterface::Location(line, column);
399+
v8End = v8::debug::Location(line, column);
400400
}
401401
auto it = m_scripts.find(scriptId);
402402
if (it == m_scripts.end()) return Response::Error("Script not found");
403403

404-
std::vector<v8::DebugInterface::Location> v8Locations;
404+
std::vector<v8::debug::Location> v8Locations;
405405
if (!it->second->getPossibleBreakpoints(v8Start, v8End, &v8Locations))
406406
return Response::InternalError();
407407

‎src/inspector/v8-debugger-script.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,8 @@ void V8DebuggerScript::setSource(v8::Local<v8::String> source) {
173173
}
174174

175175
bool V8DebuggerScript::getPossibleBreakpoints(
176-
const v8::DebugInterface::Location& start,
177-
const v8::DebugInterface::Location& end,
178-
std::vector<v8::DebugInterface::Location>* locations) {
176+
const v8::debug::Location& start, const v8::debug::Location& end,
177+
std::vector<v8::debug::Location>* locations) {
179178
v8::HandleScope scope(m_isolate);
180179
v8::Local<v8::DebugInterface::Script> script = m_script.Get(m_isolate);
181180
return script->GetPossibleBreakpoints(start, end, locations);

‎src/inspector/v8-debugger-script.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,9 @@ class V8DebuggerScript {
6767
void setSourceMappingURL(const String16&);
6868
void setSource(v8::Local<v8::String>);
6969

70-
bool getPossibleBreakpoints(
71-
const v8::DebugInterface::Location& start,
72-
const v8::DebugInterface::Location& end,
73-
std::vector<v8::DebugInterface::Location>* locations);
70+
bool getPossibleBreakpoints(const v8::debug::Location& start,
71+
const v8::debug::Location& end,
72+
std::vector<v8::debug::Location>* locations);
7473

7574
private:
7675
String16 m_id;

‎src/inspector/wasm-translation.cc‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class WasmTranslation::TranslatorImpl::RawTranslator
7272

7373
class WasmTranslation::TranslatorImpl::DisassemblingTranslator
7474
: public WasmTranslation::TranslatorImpl {
75-
using OffsetTable = std::vector<std::tuple<uint32_t, int, int>>;
75+
using OffsetTable = debug::WasmDisassembly::OffsetTable;
7676

7777
public:
7878
DisassemblingTranslator(Isolate *isolate, Local<Object> script)
@@ -88,17 +88,17 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
8888
unsigned right = static_cast<unsigned>(offset_table.size()); // exclusive
8989
while (right - left > 1) {
9090
unsigned mid = (left + right) / 2;
91-
if (std::get<0>(offset_table[mid]) <= byte_offset) {
91+
if (offset_table[mid].byte_offset <= byte_offset) {
9292
left = mid;
9393
} else {
9494
right = mid;
9595
}
9696
}
9797

9898
loc->script_id = GetFakeScriptId(loc);
99-
if (std::get<0>(offset_table[left]) == byte_offset) {
100-
loc->line = std::get<1>(offset_table[left]);
101-
loc->column = std::get<2>(offset_table[left]);
99+
if (offset_table[left].byte_offset == byte_offset) {
100+
loc->line = offset_table[left].line;
101+
loc->column = offset_table[left].column;
102102
} else {
103103
loc->line = 0;
104104
loc->column = 0;
@@ -117,9 +117,8 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
117117
while (right - left > 1) {
118118
unsigned mid = (left + right) / 2;
119119
auto &entry = (*reverse_table)[mid];
120-
if (std::get<1>(entry) < loc->line ||
121-
(std::get<1>(entry) == loc->line &&
122-
std::get<2>(entry) <= loc->column)) {
120+
if (entry.line < loc->line ||
121+
(entry.line == loc->line && entry.column <= loc->column)) {
123122
left = mid;
124123
} else {
125124
right = mid;
@@ -129,12 +128,12 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
129128
int found_byte_offset = 0;
130129
// If we found an exact match, use it. Otherwise check whether the next
131130
// bigger entry is still in the same line. Report that one then.
132-
if (std::get<1>((*reverse_table)[left]) == loc->line &&
133-
std::get<2>((*reverse_table)[left]) == loc->column) {
134-
found_byte_offset = std::get<0>((*reverse_table)[left]);
131+
if ((*reverse_table)[left].line == loc->line &&
132+
(*reverse_table)[left].column == loc->column) {
133+
found_byte_offset = (*reverse_table)[left].byte_offset;
135134
} else if (left + 1 < reverse_table->size() &&
136-
std::get<1>((*reverse_table)[left + 1]) == loc->line) {
137-
found_byte_offset = std::get<0>((*reverse_table)[left + 1]);
135+
(*reverse_table)[left + 1].line == loc->line) {
136+
found_byte_offset = (*reverse_table)[left + 1].byte_offset;
138137
}
139138

140139
v8::Isolate *isolate = loc->translation->isolate_;
@@ -172,17 +171,19 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
172171
if (it != offset_tables_.end()) return it->second;
173172

174173
v8::Isolate *isolate = loc->translation->isolate_;
175-
std::pair<std::string, OffsetTable> disassembly =
174+
debug::WasmDisassembly disassembly_result =
176175
DebugInterface::DisassembleWasmFunction(isolate, script_.Get(isolate),
177176
func_index);
178177

179178
it = offset_tables_
180-
.insert(std::make_pair(func_index, std::move(disassembly.second)))
179+
.insert(std::make_pair(func_index,
180+
std::move(disassembly_result.offset_table)))
181181
.first;
182182

183183
String16 fake_script_id = GetFakeScriptId(loc);
184184
String16 fake_script_url = GetFakeScriptUrl(loc);
185-
String16 source(disassembly.first.data(), disassembly.first.length());
185+
String16 source(disassembly_result.disassembly.data(),
186+
disassembly_result.disassembly.length());
186187
std::unique_ptr<V8DebuggerScript> fake_script(new V8DebuggerScript(
187188
fake_script_id, std::move(fake_script_url), source));
188189

@@ -202,13 +203,10 @@ class WasmTranslation::TranslatorImpl::DisassemblingTranslator
202203

203204
OffsetTable reverse_table = it->second;
204205
// Order by line, column, then byte offset.
205-
auto cmp = [](std::tuple<uint32_t, int, int> el1,
206-
std::tuple<uint32_t, int, int> el2) {
207-
if (std::get<1>(el1) != std::get<1>(el2))
208-
return std::get<1>(el1) < std::get<1>(el2);
209-
if (std::get<2>(el1) != std::get<2>(el2))
210-
return std::get<2>(el1) < std::get<2>(el2);
211-
return std::get<0>(el1) < std::get<0>(el2);
206+
auto cmp = [](OffsetTable::value_type el1, OffsetTable::value_type el2) {
207+
if (el1.line != el2.line) return el1.line < el2.line;
208+
if (el1.column != el2.column) return el1.column < el2.column;
209+
return el1.byte_offset < el2.byte_offset;
212210
};
213211
std::sort(reverse_table.begin(), reverse_table.end(), cmp);
214212

‎src/v8.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,7 @@
836836
'debug/debug-scopes.h',
837837
'debug/debug.cc',
838838
'debug/debug.h',
839+
'debug/interface-types.h',
839840
'debug/liveedit.cc',
840841
'debug/liveedit.h',
841842
'deoptimize-reason.cc',

0 commit comments

Comments
 (0)