Skip to content

[LLDB] Update DIL to pass current 'frame var' tests. #145055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cmtice merged 9 commits into from
Jul 1, 2025
Merged

[LLDB] Update DIL to pass current 'frame var' tests. #145055

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e9079e6
[LLDB] Update DIL to pass current 'frame var' tests.
cmtice Jun 20, 2025
e7ae837
Fix clang-format issues.
cmtice Jun 20, 2025
f7fbde0
Fix more clang-format issues.
cmtice Jun 20, 2025
c7c16bd
Fix clang-format issue.
cmtice Jun 20, 2025
30bf17a
Merge remote-tracking branch 'origin/main' into DIL-fix-tests
cmtice Jun 24, 2025
ca0f328
Address reviewer comments:
cmtice Jun 29, 2025
d0c585c
Merge remote-tracking branch 'origin/main' into DIL-fix-tests
cmtice Jun 29, 2025
3a408eb
Fix bug introduced in previous commit.
cmtice Jun 30, 2025
bc74c12
Address reviewer comment:
cmtice Jun 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lldb/source/Target/StackFrame.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ ValueObjectSP StackFrame::DILGetValueForVariableExpressionPath(
return ValueObjectConstResult::Create(nullptr, std::move(error));
}

var_sp = (*valobj_or_error)->GetVariable();
return *valobj_or_error;
}

Expand Down
10 changes: 5 additions & 5 deletions lldb/source/ValueObject/DILEval.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,8 @@ Interpreter::Visit(const MemberOfNode *node) {

if (!m_use_synthetic || !field_obj) {
std::string errMsg = llvm::formatv(
"no member named '{0}' in {1}", node->GetFieldName(),
base->GetCompilerType().GetFullyUnqualifiedType().TypeDescription());
"\"{0}\" is not a member of \"({1}) {2}\"", node->GetFieldName(),
base->GetTypeName().AsCString("<invalid type>"), base->GetName());
return llvm::make_error<DILDiagnosticError>(
m_expr, errMsg, node->GetLocation(), node->GetFieldName().size());
}
Expand All @@ -376,9 +376,9 @@ Interpreter::Visit(const MemberOfNode *node) {
CompilerType base_type = base->GetCompilerType();
if (node->GetIsArrow() && base->IsPointerType())
base_type = base_type.GetPointeeType();
std::string errMsg =
llvm::formatv("no member named '{0}' in {1}", node->GetFieldName(),
base_type.GetFullyUnqualifiedType().TypeDescription());
std::string errMsg = llvm::formatv(
"\"{0}\" is not a member of \"({1}) {2}\"", node->GetFieldName(),
base->GetTypeName().AsCString("<invalid type>"), base->GetName());
return llvm::make_error<DILDiagnosticError>(
m_expr, errMsg, node->GetLocation(), node->GetFieldName().size());
}
Expand Down
39 changes: 34 additions & 5 deletions lldb/source/ValueObject/DILParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,40 @@ ASTNodeUP DILParser::ParsePrimaryExpression() {
}

if (CurToken().Is(Token::l_paren)) {
m_dil_lexer.Advance();
auto expr = ParseExpression();
Expect(Token::r_paren);
m_dil_lexer.Advance();
return expr;
// Check in case this is an anonynmous namespace
if (m_dil_lexer.LookAhead(1).Is(Token::identifier) &&
(m_dil_lexer.LookAhead(1).GetSpelling() == "anonymous") &&
m_dil_lexer.LookAhead(2).Is(Token::identifier) &&
(m_dil_lexer.LookAhead(2).GetSpelling() == "namespace") &&
m_dil_lexer.LookAhead(3).Is(Token::r_paren) &&
m_dil_lexer.LookAhead(4).Is(Token::coloncolon)) {
m_dil_lexer.Advance(4);

std::string identifier = "(anonymous namespace)";
Copy link
Collaborator

@labath labath Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of basically inlining a part of ParseIdExpression, could we find a way to delegate to it? What would happen if we changed the check on line 172 to say "if next token is :: or identifier OR next 4 tokens are '(anonymous namespace)'" ?

Expect(Token::coloncolon);
// Save the source location for the diagnostics message.
uint32_t loc = CurToken().GetLocation();
m_dil_lexer.Advance();
assert(
(CurToken().Is(Token::identifier) || CurToken().Is(Token::l_paren)) &&
"Expected an identifier or anonymous namespeace, but not found.");
Copy link
Collaborator

@labath labath Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An assert is fine for an internal invariant, but this looks like its checking the next token in the stream. That's can't be right. Or am I misunderstanding something?

Copy link
Collaborator

@labath labath Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No -- I don't think I am -- I just made lldb crash with this assertion.

Copy link
Collaborator

@labath labath Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

(lldb) frame variable 'lldb::(anonymous namespace)::a'
lldb: lldb/source/ValueObject/DILParser.cpp:235: std::string lldb_private::dil::DILP
arser::ParseNestedNameSpecifier(): Assertion `(CurToken().Is(Token::identifier) || CurToken().Is(Token::l
_paren)) && "Expected an identifier or anonymous namespace, but not found."' failed.

std::string identifier2 = ParseNestedNameSpecifier();
if (identifier2.empty()) {
// There was only an identifer, no more levels of nesting. Or there
// was an invalid expression starting with a left parenthesis.
Expect(Token::identifier);
identifier2 = CurToken().GetSpelling();
m_dil_lexer.Advance();
}
identifier = identifier + "::" + identifier2;
return std::make_unique<IdentifierNode>(loc, identifier);
} else {
Copy link
Collaborator

@labath labath Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no else after return

Copy link
Contributor Author

@cmtice cmtice Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

m_dil_lexer.Advance();
auto expr = ParseExpression();
Expect(Token::r_paren);
m_dil_lexer.Advance();
return expr;
}
}

BailOut(llvm::formatv("Unexpected token: {0}", CurToken()),
Expand Down
19 changes: 13 additions & 6 deletions lldb/test/API/commands/frame/var-dil/basics/MemberOf/TestFrameVarDILMemberOf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,19 @@ def test_frame_var(self):
self.expect_var_path("sp->x", value="1")
self.expect_var_path("sp->r", type="int &")

self.expect("frame variable 'sp->foo'", error=True,
substrs=["no member named 'foo' in 'Sx *'"])

self.expect("frame variable 'sp.x'", error=True,
substrs=["member reference type 'Sx *' is a "
"pointer; did you mean to use '->'"])
self.expect(
"frame variable 'sp->foo'",
error=True,
substrs=['"foo" is not a member of "(Sx *) sp"'],
)

self.expect(
"frame variable 'sp.x'",
error=True,
substrs=[
"member reference type 'Sx *' is a pointer; did you mean to use '->'"
],
)

# Test for record typedefs.
self.expect_var_path("sa.x", value="3")
Expand Down
35 changes: 25 additions & 10 deletions ...ds/frame/var-dil/basics/MemberOfAnonymousMember/TestFrameVarDILMemberOfAnonymousMember.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@ def test_frame_var(self):
self.expect_var_path("a.x", value="1")
self.expect_var_path("a.y", value="2")

self.expect("frame variable 'b.x'", error=True,
substrs=["no member named 'x' in 'B'"])
self.expect(
"frame variable 'b.x'",
error=True,
substrs=['"x" is not a member of "(B) b"'],
)
#self.expect_var_path("b.y", value="0")
self.expect_var_path("b.z", value="3")
self.expect_var_path("b.w", value="4")
Expand All @@ -43,19 +46,31 @@ def test_frame_var(self):
self.expect_var_path("d.z", value="9")
self.expect_var_path("d.w", value="10")

self.expect("frame variable 'e.x'", error=True,
substrs=["no member named 'x' in 'E'"])
self.expect("frame variable 'f.x'", error=True,
substrs=["no member named 'x' in 'F'"])
self.expect(
"frame variable 'e.x'",
error=True,
substrs=['"x" is not a member of "(E) e"'],
)
self.expect(
"frame variable 'f.x'",
error=True,
substrs=['"x" is not a member of "(F) f"'],
)
self.expect_var_path("f.named_field.x", value="12")

self.expect_var_path("unnamed_derived.y", value="2")
self.expect_var_path("unnamed_derived.z", value="13")

self.expect("frame variable 'derb.x'", error=True,
substrs=["no member named 'x' in 'DerivedB'"])
self.expect("frame variable 'derb.y'", error=True,
substrs=["no member named 'y' in 'DerivedB'"])
self.expect(
"frame variable 'derb.x'",
error=True,
substrs=['"x" is not a member of "(DerivedB) derb"'],
)
self.expect(
"frame variable 'derb.y'",
error=True,
substrs=['"y" is not a member of "(DerivedB) derb"'],
)
self.expect_var_path("derb.w", value="14")
self.expect_var_path("derb.k", value="15")
self.expect_var_path("derb.a.x", value="1")
Expand Down
Loading