Skip to content

Commit 7f7b3f6

Browse files
committed
[lldb][lldb-dap] add review changes
1 parent 1815d6b commit 7f7b3f6

File tree

4 files changed

+100
-20
lines changed

4 files changed

+100
-20
lines changed

lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ static Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
4141
// as the locals scope will not be expanded. It becomes more annoying when
4242
// the scope has arguments, return_value and locals.
4343
if (variablesReference == VARREF_LOCALS)
44-
scope.presentationHint = Scope::ePresentationHintLocals;
44+
scope.presentationHint = Scope::eScopePresentationHintLocals;
4545
else if (variablesReference == VARREF_REGS)
46-
scope.presentationHint = Scope::ePresentationHintRegisters;
46+
scope.presentationHint = Scope::eScopePresentationHintRegisters;
4747

4848
scope.variablesReference = variablesReference;
4949
scope.namedVariables = namedVariables;

lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ bool fromJSON(const json::Value &Params, Source::PresentationHint &PH,
2525
}
2626
std::optional<Source::PresentationHint> hint =
2727
StringSwitch<std::optional<Source::PresentationHint>>(*rawHint)
28-
.Case("normal", Source::ePresentationHintNormal)
29-
.Case("emphasize", Source::ePresentationHintEmphasize)
30-
.Case("deemphasize", Source::ePresentationHintDeemphasize)
28+
.Case("normal", Source::eSourcePresentationHintNormal)
29+
.Case("emphasize", Source::eSourcePresentationHintEmphasize)
30+
.Case("deemphasize", Source::eSourcePresentationHintDeemphasize)
3131
.Default(std::nullopt);
3232
if (!hint) {
3333
P.report("unexpected value");
@@ -43,13 +43,14 @@ bool fromJSON(const json::Value &Params, Source &S, json::Path P) {
4343
O.map("presentationHint", S.presentationHint) &&
4444
O.map("sourceReference", S.sourceReference);
4545
}
46+
4647
llvm::json::Value toJSON(Source::PresentationHint hint) {
4748
switch (hint) {
48-
case Source::ePresentationHintNormal:
49+
case Source::eSourcePresentationHintNormal:
4950
return "normal";
50-
case Source::ePresentationHintEmphasize:
51+
case Source::eSourcePresentationHintEmphasize:
5152
return "emphasize";
52-
case Source::ePresentationHintDeemphasize:
53+
case Source::eSourcePresentationHintDeemphasize:
5354
return "deemphasize";
5455
}
5556
llvm_unreachable("unhandled presentation hint.");
@@ -435,6 +436,41 @@ json::Value toJSON(const Capabilities &C) {
435436
return result;
436437
}
437438

439+
bool fromJSON(const json::Value &Params, Scope::PresentationHint &PH,
440+
json::Path P) {
441+
auto rawHint = Params.getAsString();
442+
if (!rawHint) {
443+
P.report("expected a string");
444+
return false;
445+
}
446+
const std::optional<Scope::PresentationHint> hint =
447+
StringSwitch<std::optional<Scope::PresentationHint>>(*rawHint)
448+
.Case("arguments", Scope::eScopePresentationHintArguments)
449+
.Case("locals", Scope::eScopePresentationHintLocals)
450+
.Case("registers", Scope::eScopePresentationHintRegisters)
451+
.Case("returnValue", Scope::eScopePresentationHintReturnValue)
452+
.Default(std::nullopt);
453+
if (!hint) {
454+
P.report("unexpected value");
455+
return false;
456+
}
457+
PH = *hint;
458+
return true;
459+
}
460+
461+
bool fromJSON(const json::Value &Params, Scope &S, json::Path P) {
462+
json::ObjectMapper O(Params, P);
463+
return O && O.map("name", S.name) &&
464+
O.mapOptional("presentationHint", S.presentationHint) &&
465+
O.map("variablesReference", S.variablesReference) &&
466+
O.mapOptional("namedVariables", S.namedVariables) &&
467+
O.map("indexedVariables", S.indexedVariables) &&
468+
O.mapOptional("source", S.source) && O.map("expensive", S.expensive) &&
469+
O.mapOptional("line", S.line) && O.mapOptional("column", S.column) &&
470+
O.mapOptional("endLine", S.endLine) &&
471+
O.mapOptional("endColumn", S.endColumn);
472+
}
473+
438474
llvm::json::Value toJSON(const Scope &SC) {
439475
llvm::json::Object result{{"name", SC.name},
440476
{"variablesReference", SC.variablesReference},
@@ -443,16 +479,16 @@ llvm::json::Value toJSON(const Scope &SC) {
443479
if (SC.presentationHint.has_value()) {
444480
llvm::StringRef presentationHint;
445481
switch (*SC.presentationHint) {
446-
case Scope::ePresentationHintArguments:
482+
case Scope::eScopePresentationHintArguments:
447483
presentationHint = "arguments";
448484
break;
449-
case Scope::ePresentationHintLocals:
485+
case Scope::eScopePresentationHintLocals:
450486
presentationHint = "locals";
451487
break;
452-
case Scope::ePresentationHintRegisters:
488+
case Scope::eScopePresentationHintRegisters:
453489
presentationHint = "registers";
454490
break;
455-
case Scope::ePresentationHintReturnValue:
491+
case Scope::eScopePresentationHintReturnValue:
456492
presentationHint = "returnValue";
457493
break;
458494
}

lldb/tools/lldb-dap/Protocol/ProtocolTypes.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,9 @@ llvm::json::Value toJSON(const Capabilities &);
290290
/// breakpoints.
291291
struct Source {
292292
enum PresentationHint : unsigned {
293-
ePresentationHintNormal,
294-
ePresentationHintEmphasize,
295-
ePresentationHintDeemphasize,
293+
eSourcePresentationHintNormal,
294+
eSourcePresentationHintEmphasize,
295+
eSourcePresentationHintDeemphasize,
296296
};
297297

298298
/// The short name of the source. Every source returned from the debug adapter
@@ -326,10 +326,10 @@ llvm::json::Value toJSON(const Source &);
326326
/// a source or a range within a source.
327327
struct Scope {
328328
enum PresentationHint : unsigned {
329-
ePresentationHintArguments,
330-
ePresentationHintLocals,
331-
ePresentationHintRegisters,
332-
ePresentationHintReturnValue
329+
eScopePresentationHintArguments,
330+
eScopePresentationHintLocals,
331+
eScopePresentationHintRegisters,
332+
eScopePresentationHintReturnValue
333333
};
334334
/// Name of the scope such as 'Arguments', 'Locals', or 'Registers'. This
335335
/// string is shown in the UI as is and can be translated.
@@ -387,6 +387,9 @@ struct Scope {
387387
/// it is 0- or 1-based.
388388
std::optional<uint64_t> endColumn;
389389
};
390+
bool fromJSON(const llvm::json::Value &Params, Scope::PresentationHint &PH,
391+
llvm::json::Path);
392+
bool fromJSON(const llvm::json::Value &, Scope &, llvm::json::Path);
390393
llvm::json::Value toJSON(const Scope &);
391394

392395
/// The granularity of one `step` in the stepping requests `next`, `stepIn`,

lldb/unittests/DAP/ProtocolTypesTest.cpp

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ TEST(ProtocolTypesTest, Source) {
5050
source.name = "testName";
5151
source.path = "/path/to/source";
5252
source.sourceReference = 12345;
53-
source.presentationHint = ePresentationHintEmphasize;
53+
source.presentationHint = Source::eSourcePresentationHintEmphasize;
5454

5555
llvm::Expected<Source> deserialized_source = roundtrip(source);
5656
ASSERT_THAT_EXPECTED(deserialized_source, llvm::Succeeded());
@@ -292,6 +292,47 @@ TEST(ProtocolTypesTest, Capabilities) {
292292
deserialized_capabilities->lldbExtVersion);
293293
}
294294

295+
TEST(ProtocolTypesTest, Scope) {
296+
Scope scope;
297+
scope.name = "Locals";
298+
scope.presentationHint = Scope::eScopePresentationHintLocals;
299+
scope.variablesReference = 1;
300+
scope.namedVariables = 2;
301+
scope.indexedVariables = std::nullopt;
302+
scope.expensive = false;
303+
scope.line = 2;
304+
scope.column = 3;
305+
scope.endLine = 10;
306+
scope.endColumn = 20;
307+
308+
scope.source =
309+
Source{.name = "testName",
310+
.path = "/path/to/source",
311+
.sourceReference = 12345,
312+
.presentationHint = Source::eSourcePresentationHintNormal};
313+
314+
llvm::Expected<Scope> deserialized_scope = roundtrip(scope);
315+
ASSERT_THAT_EXPECTED(deserialized_scope, llvm::Succeeded());
316+
EXPECT_EQ(scope.name, deserialized_scope->name);
317+
EXPECT_EQ(scope.presentationHint, deserialized_scope->presentationHint);
318+
EXPECT_EQ(scope.variablesReference, deserialized_scope->variablesReference);
319+
EXPECT_EQ(scope.namedVariables, deserialized_scope->namedVariables);
320+
EXPECT_EQ(scope.indexedVariables, deserialized_scope->indexedVariables);
321+
EXPECT_EQ(scope.expensive, deserialized_scope->expensive);
322+
EXPECT_EQ(scope.line, deserialized_scope->line);
323+
EXPECT_EQ(scope.column, deserialized_scope->column);
324+
EXPECT_EQ(scope.endLine, deserialized_scope->endLine);
325+
EXPECT_EQ(scope.endColumn, deserialized_scope->endColumn);
326+
327+
EXPECT_THAT(deserialized_scope->source.has_value(), true);
328+
const Source &source = scope.source.value();
329+
const Source &deserialized_source = deserialized_scope->source.value();
330+
331+
EXPECT_EQ(source.path, deserialized_source.path);
332+
EXPECT_EQ(source.sourceReference, deserialized_source.sourceReference);
333+
EXPECT_EQ(source.presentationHint, deserialized_source.presentationHint);
334+
}
335+
295336
TEST(ProtocolTypesTest, PresentationHint) {
296337
// Test all PresentationHint values.
297338
std::vector<std::pair<PresentationHint, llvm::StringRef>> test_cases = {

0 commit comments

Comments
 (0)