Skip to content

Commit 7eb3b38

Browse files
committed
Addressing PR comments.
1 parent 4964d57 commit 7eb3b38

File tree

5 files changed

+40
-56
lines changed

5 files changed

+40
-56
lines changed

lldb/tools/lldb-dap/Breakpoint.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ struct Breakpoint : public BreakpointBase {
1919
// The LLDB breakpoint associated wit this source breakpoint
2020
lldb::SBBreakpoint bp;
2121

22-
Breakpoint(DAP &d) : BreakpointBase(d) {}
2322
Breakpoint(DAP &d, const llvm::json::Object &obj) : BreakpointBase(d, obj) {}
2423
Breakpoint(DAP &d, lldb::SBBreakpoint bp) : BreakpointBase(d), bp(bp) {}
2524

lldb/tools/lldb-dap/BreakpointBase.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@ struct BreakpointBase {
2424
// ignored. The backend is expected to interpret the expression as needed
2525
std::string hitCondition;
2626

27-
BreakpointBase(DAP &d) : dap(d) {}
27+
explicit BreakpointBase(DAP &d) : dap(d) {}
2828
BreakpointBase(DAP &d, const llvm::json::Object &obj);
29-
BreakpointBase(const BreakpointBase &) = default;
3029
virtual ~BreakpointBase() = default;
3130

3231
virtual void SetCondition() = 0;

lldb/tools/lldb-dap/InstructionBreakpoint.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
#include "Breakpoint.h"
1414
#include "DAPForward.h"
15-
#include "lldb/lldb-defines.h"
1615
#include "lldb/lldb-types.h"
1716
#include <cstdint>
1817

@@ -24,9 +23,6 @@ struct InstructionBreakpoint : public Breakpoint {
2423
lldb::addr_t instructionAddressReference;
2524
int32_t offset;
2625

27-
explicit InstructionBreakpoint(DAP &d)
28-
: Breakpoint(d), instructionAddressReference(LLDB_INVALID_ADDRESS),
29-
offset(0) {}
3026
InstructionBreakpoint(DAP &d, const llvm::json::Object &obj);
3127

3228
// Set instruction breakpoint in LLDB as a new breakpoint

lldb/tools/lldb-dap/JSONUtils.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include "DAP.h"
1313
#include "ExceptionBreakpoint.h"
1414
#include "LLDBUtils.h"
15-
1615
#include "lldb/API/SBAddress.h"
1716
#include "lldb/API/SBCompileUnit.h"
1817
#include "lldb/API/SBDeclaration.h"
@@ -35,7 +34,6 @@
3534
#include "lldb/lldb-defines.h"
3635
#include "lldb/lldb-enumerations.h"
3736
#include "lldb/lldb-types.h"
38-
3937
#include "llvm/ADT/DenseMap.h"
4038
#include "llvm/ADT/StringExtras.h"
4139
#include "llvm/ADT/StringRef.h"
@@ -45,13 +43,12 @@
4543
#include "llvm/Support/Path.h"
4644
#include "llvm/Support/ScopedPrinter.h"
4745
#include "llvm/Support/raw_ostream.h"
48-
4946
#include <chrono>
47+
#include <cstddef>
5048
#include <iomanip>
5149
#include <optional>
5250
#include <sstream>
5351
#include <string>
54-
#include <string.h>
5552
#include <sys/syslimits.h>
5653
#include <utility>
5754
#include <vector>
@@ -1483,11 +1480,11 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request,
14831480
// Keep all the top level items from the statistics dump, except for the
14841481
// "modules" array. It can be huge and cause delay
14851482
// Array and dictionary value will return as <key, JSON string> pairs
1486-
void FilterAndGetValueForKey(const lldb::SBStructuredData data, const char *key,
1487-
llvm::json::Object &out) {
1488-
lldb::SBStructuredData value = data.GetValueForKey(key);
1483+
void FilterAndGetValueForKey(const lldb::SBStructuredData data,
1484+
llvm::StringRef key, llvm::json::Object &out) {
1485+
lldb::SBStructuredData value = data.GetValueForKey(key.data());
14891486
std::string key_utf8 = llvm::json::fixUTF8(key);
1490-
if (strcmp(key, "modules") == 0)
1487+
if (key == "modules")
14911488
return;
14921489
switch (value.GetType()) {
14931490
case lldb::eStructuredDataTypeFloat:

lldb/tools/lldb-dap/lldb-dap.cpp

Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,16 @@
1313
#include "OutputRedirector.h"
1414
#include "RunInTerminal.h"
1515
#include "Watchpoint.h"
16-
1716
#include "lldb/API/SBDeclaration.h"
1817
#include "lldb/API/SBInstruction.h"
1918
#include "lldb/API/SBListener.h"
2019
#include "lldb/API/SBMemoryRegionInfo.h"
2120
#include "lldb/API/SBStream.h"
2221
#include "lldb/API/SBStringList.h"
2322
#include "lldb/Host/Config.h"
24-
2523
#include "llvm/ADT/ArrayRef.h"
2624
#include "llvm/ADT/DenseMap.h"
25+
#include "llvm/ADT/DenseSet.h"
2726
#include "llvm/ADT/ScopeExit.h"
2827
#include "llvm/ADT/SetVector.h"
2928
#include "llvm/ADT/StringExtras.h"
@@ -38,16 +37,23 @@
3837
#include "llvm/Support/Path.h"
3938
#include "llvm/Support/PrettyStackTrace.h"
4039
#include "llvm/Support/raw_ostream.h"
41-
40+
#include <algorithm>
41+
#include <array>
4242
#include <cassert>
4343
#include <climits>
4444
#include <cstdarg>
4545
#include <cstdio>
4646
#include <cstdlib>
4747
#include <cstring>
48+
#include <map>
49+
#include <memory>
4850
#include <optional>
51+
#include <set>
4952
#include <sys/stat.h>
5053
#include <sys/types.h>
54+
#include <thread>
55+
#include <vector>
56+
5157
#if defined(_WIN32)
5258
// We need to #define NOMINMAX in order to skip `min()` and `max()` macro
5359
// definitions that conflict with other system headers.
@@ -68,14 +74,6 @@
6874
#include <sys/prctl.h>
6975
#endif
7076

71-
#include <algorithm>
72-
#include <array>
73-
#include <map>
74-
#include <memory>
75-
#include <set>
76-
#include <thread>
77-
#include <vector>
78-
7977
#if defined(_WIN32)
8078
#ifndef PATH_MAX
8179
#define PATH_MAX MAX_PATH
@@ -2706,16 +2704,14 @@ void request_setBreakpoints(const llvm::json::Object &request) {
27062704
if (bp_obj) {
27072705
SourceBreakpoint src_bp(g_dap, *bp_obj);
27082706
request_bps.try_emplace(src_bp.line, src_bp);
2709-
const auto [kv, inserted] =
2707+
const auto [iv, inserted] =
27102708
g_dap.source_breakpoints[path].try_emplace(src_bp.line, src_bp);
27112709
// We check if this breakpoint already exists to update it
2712-
if (inserted) {
2713-
kv->getSecond().SetBreakpoint(path.data());
2714-
} else {
2715-
kv->getSecond().UpdateBreakpoint(src_bp);
2716-
}
2717-
2718-
AppendBreakpoint(&kv->getSecond(), response_breakpoints, path,
2710+
if (inserted)
2711+
iv->getSecond().SetBreakpoint(path.data());
2712+
else
2713+
iv->getSecond().UpdateBreakpoint(src_bp);
2714+
AppendBreakpoint(&iv->getSecond(), response_breakpoints, path,
27192715
src_bp.line);
27202716
}
27212717
}
@@ -2803,14 +2799,14 @@ void request_setExceptionBreakpoints(const llvm::json::Object &request) {
28032799

28042800
for (const auto &value : *filters) {
28052801
const auto filter = GetAsString(value);
2806-
auto exc_bp = g_dap.GetExceptionBreakpoint(std::string(filter));
2802+
auto *exc_bp = g_dap.GetExceptionBreakpoint(std::string(filter));
28072803
if (exc_bp) {
28082804
exc_bp->SetBreakpoint();
28092805
unset_filters.erase(std::string(filter));
28102806
}
28112807
}
28122808
for (const auto &filter : unset_filters) {
2813-
auto exc_bp = g_dap.GetExceptionBreakpoint(filter);
2809+
auto *exc_bp = g_dap.GetExceptionBreakpoint(filter);
28142810
if (exc_bp)
28152811
exc_bp->ClearBreakpoint();
28162812
}
@@ -2907,22 +2903,21 @@ void request_setFunctionBreakpoints(const llvm::json::Object &request) {
29072903
// There is no call to remove function breakpoints other than calling this
29082904
// function with a smaller or empty "breakpoints" list.
29092905
const auto name_iter = g_dap.function_breakpoints.keys();
2910-
llvm::SetVector<llvm::StringRef> seen(name_iter.begin(), name_iter.end());
2906+
llvm::DenseSet<llvm::StringRef> seen(name_iter.begin(), name_iter.end());
29112907
for (const auto &value : *breakpoints) {
29122908
const auto *bp_obj = value.getAsObject();
29132909
if (!bp_obj)
29142910
continue;
29152911
FunctionBreakpoint fn_bp(g_dap, *bp_obj);
2916-
const auto [kv, inserted] = g_dap.function_breakpoints.try_emplace(
2912+
const auto [it, inserted] = g_dap.function_breakpoints.try_emplace(
29172913
fn_bp.functionName, g_dap, *bp_obj);
2918-
if (inserted) {
2919-
kv->second.SetBreakpoint();
2920-
} else {
2921-
kv->second.UpdateBreakpoint(fn_bp);
2922-
}
2914+
if (inserted)
2915+
it->second.SetBreakpoint();
2916+
else
2917+
it->second.UpdateBreakpoint(fn_bp);
29232918

2924-
AppendBreakpoint(&kv->second, response_breakpoints);
2925-
seen.remove(fn_bp.functionName);
2919+
AppendBreakpoint(&it->second, response_breakpoints);
2920+
seen.erase(fn_bp.functionName);
29262921
}
29272922

29282923
// Remove any breakpoints that are no longer in our list
@@ -3183,9 +3178,8 @@ void request_setDataBreakpoints(const llvm::json::Object &request) {
31833178
if (breakpoints) {
31843179
for (const auto &bp : *breakpoints) {
31853180
const auto *bp_obj = bp.getAsObject();
3186-
if (bp_obj) {
3181+
if (bp_obj)
31873182
watchpoints.emplace_back(g_dap, *bp_obj);
3188-
}
31893183
}
31903184
}
31913185
// If two watchpoints start at the same address, the latter overwrite the
@@ -4740,7 +4734,7 @@ void request_setInstructionBreakpoints(const llvm::json::Object &request) {
47404734
// Disable any instruction breakpoints that aren't in this request.
47414735
// There is no call to remove instruction breakpoints other than calling this
47424736
// function with a smaller or empty "breakpoints" list.
4743-
llvm::SetVector<lldb::addr_t> seen;
4737+
llvm::DenseSet<lldb::addr_t> seen;
47444738
for (const auto &addr : g_dap.instruction_breakpoints)
47454739
seen.insert(addr.first);
47464740

@@ -4750,15 +4744,14 @@ void request_setInstructionBreakpoints(const llvm::json::Object &request) {
47504744
continue;
47514745
// Read instruction breakpoint request.
47524746
InstructionBreakpoint inst_bp(g_dap, *bp_obj);
4753-
const auto [kv, inserted] = g_dap.instruction_breakpoints.try_emplace(
4747+
const auto [iv, inserted] = g_dap.instruction_breakpoints.try_emplace(
47544748
inst_bp.instructionAddressReference, g_dap, *bp_obj);
4755-
if (inserted) {
4756-
kv->second.SetBreakpoint();
4757-
} else {
4758-
kv->second.UpdateBreakpoint(inst_bp);
4759-
}
4760-
AppendBreakpoint(&kv->second, response_breakpoints);
4761-
seen.remove(inst_bp.instructionAddressReference);
4749+
if (inserted)
4750+
iv->second.SetBreakpoint();
4751+
else
4752+
iv->second.UpdateBreakpoint(inst_bp);
4753+
AppendBreakpoint(&iv->second, response_breakpoints);
4754+
seen.erase(inst_bp.instructionAddressReference);
47624755
}
47634756

47644757
for (const auto &addr : seen) {

0 commit comments

Comments
 (0)