[LLDB] Fix GetIndexOfChildMemberWithName to handle anonymous struct in base classes
#158256
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s in base classes Fixes llvm#158131 Similar to llvm#138487 but also search for child indexes in the base classes
…rds, and fix `type_lookup_anon_struct` test
|
@llvm/pr-subscribers-lldb Author: Kiva (imkiva) ChangesFixes #158131 Similar to #138487 but also search for child indices in the base classes. Full diff: https://github.com/llvm/llvm-project/pull/158256.diff 5 Files Affected:
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 39aacdb58e694..43d956a2e7088 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6707,6 +6707,52 @@ uint32_t TypeSystemClang::GetIndexForRecordChild(
return UINT32_MAX;
}
+bool TypeSystemClang::FindInAnonRecordFields(const clang::RecordDecl *rd,
+ std::vector<uint32_t> &path,
+ llvm::StringRef name,
+ bool omit_empty_base_classes) {
+ uint32_t local_idx = 0;
+
+ // We need the visible base count to compute the child index offset
+ const clang::CXXRecordDecl *crd =
+ llvm::dyn_cast<clang::CXXRecordDecl>(rd);
+ const uint32_t bases =
+ TypeSystemClang::GetNumBaseClasses(crd, omit_empty_base_classes);
+
+ // We only treat anonymous record fields as transparent containers for further lookup.
+ for (auto it = rd->field_begin(), ie = rd->field_end();
+ it != ie; ++it, ++local_idx) {
+ llvm::StringRef fname = it->getName();
+ const bool is_anon = it->isAnonymousStructOrUnion() || fname.empty();
+
+ // named field, check for a match
+ if (!is_anon) {
+ if (fname == name) {
+ path.push_back(bases + local_idx);
+ return true;
+ }
+ continue;
+ }
+
+ // anonymous field, look inside only if it is a record type
+ if (!it->getType()->isRecordType())
+ continue;
+
+ const auto *inner_rt = it->getType()->castAs<clang::RecordType>();
+ const clang::RecordDecl *inner_rd = inner_rt->getOriginalDecl()->getDefinitionOrSelf();
+ if (!inner_rd)
+ continue;
+
+ // only descend into the "fields" of the anonymous record
+ // (do not traverse its bases here)
+ path.push_back(bases + local_idx);
+ if (FindInAnonRecordFields(inner_rd, path, name, omit_empty_base_classes))
+ return true;
+ path.pop_back();
+ }
+ return false;
+}
+
// Look for a child member (doesn't include base classes, but it does include
// their members) in the type hierarchy. Returns an index path into
// "clang_type" on how to reach the appropriate member.
@@ -6766,16 +6812,21 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
field_end = record_decl->field_end();
field != field_end; ++field, ++child_idx) {
llvm::StringRef field_name = field->getName();
- if (field_name.empty()) {
- CompilerType field_type = GetType(field->getType());
- std::vector<uint32_t> save_indices = child_indexes;
- child_indexes.push_back(
- child_idx + TypeSystemClang::GetNumBaseClasses(
- cxx_record_decl, omit_empty_base_classes));
- if (field_type.GetIndexOfChildMemberWithName(
- name, omit_empty_base_classes, child_indexes))
- return child_indexes.size();
- child_indexes = std::move(save_indices);
+ const bool is_anon =
+ field->isAnonymousStructOrUnion() || field_name.empty();
+ if (is_anon) {
+ if (field->getType()->isRecordType()) {
+ const uint32_t this_slot =
+ child_idx + TypeSystemClang::GetNumBaseClasses(
+ cxx_record_decl, omit_empty_base_classes);
+ std::vector<uint32_t> save_indices = child_indexes;
+ child_indexes.push_back(this_slot);
+ const auto *rt = field->getType()->castAs<clang::RecordType>();
+ const clang::RecordDecl *rd = rt->getOriginalDecl()->getDefinitionOrSelf();
+ if (rd && FindInAnonRecordFields(rd, child_indexes, name, omit_empty_base_classes))
+ return child_indexes.size();
+ child_indexes = std::move(save_indices);
+ }
} else if (field_name == name) {
// We have to add on the number of base classes to this index!
child_indexes.push_back(
@@ -6786,6 +6837,23 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
}
if (cxx_record_decl) {
+ for (const clang::CXXBaseSpecifier &base_spec : cxx_record_decl->bases()) {
+ uint32_t base_slot =
+ GetIndexForRecordBase(record_decl, &base_spec, omit_empty_base_classes);
+ if (base_slot == UINT32_MAX)
+ continue;
+
+ std::vector<uint32_t> save = child_indexes;
+ child_indexes.push_back(base_slot);
+ CompilerType base_type = GetType(base_spec.getType());
+ if (GetIndexOfChildMemberWithName(base_type.GetOpaqueQualType(),
+ name, omit_empty_base_classes,
+ child_indexes)) {
+ return child_indexes.size();
+ }
+ child_indexes = std::move(save);
+ }
+
const clang::RecordDecl *parent_record_decl = cxx_record_decl;
// Didn't find things easily, lets let clang do its thang...
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 709f89590ba3b..62e6d831440b2 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -320,6 +320,11 @@ class TypeSystemClang : public TypeSystem {
const clang::CXXBaseSpecifier *base_spec,
bool omit_empty_base_classes);
+ bool FindInAnonRecordFields(const clang::RecordDecl *rd,
+ std::vector<uint32_t> &path,
+ llvm::StringRef name,
+ bool omit_empty_base_classes);
+
/// Synthesize a clang::Module and return its ID or a default-constructed ID.
OptionalClangModuleID GetOrCreateClangModule(llvm::StringRef name,
OptionalClangModuleID parent,
diff --git a/lldb/test/API/lang/cpp/type_lookup_anon_base_member/Makefile b/lldb/test/API/lang/cpp/type_lookup_anon_base_member/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/lang/cpp/type_lookup_anon_base_member/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/type_lookup_anon_base_member/TestCppTypeLookupAnonBaseMember.py b/lldb/test/API/lang/cpp/type_lookup_anon_base_member/TestCppTypeLookupAnonBaseMember.py
new file mode 100644
index 0000000000000..8f38403acfc34
--- /dev/null
+++ b/lldb/test/API/lang/cpp/type_lookup_anon_base_member/TestCppTypeLookupAnonBaseMember.py
@@ -0,0 +1,37 @@
+"""
+Test that we properly print anonymous members in a base class.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import decorators
+
+
+class TestTypeLookupAnonBaseMember(TestBase):
+ def test_lookup_anon_base_member(self):
+ self.build()
+ (target, process, thread, bp1) = lldbutil.run_to_source_breakpoint(
+ self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp")
+ )
+
+ thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+ frame = thread.GetFrameAtIndex(0)
+
+ d = frame.FindVariable("d")
+ self.assertTrue(d.IsValid())
+
+ # b from Base
+ b = d.GetChildMemberWithName("b")
+ self.assertTrue(b.IsValid())
+ self.assertEqual(b.GetValueAsSigned(), 1)
+
+ # x from anonymous struct (inside Base)
+ x = d.GetChildMemberWithName("x")
+ self.assertTrue(x.IsValid())
+ self.assertEqual(x.GetValueAsSigned(), 2)
+
+ # d from Derived
+ a = d.GetChildMemberWithName("a")
+ self.assertTrue(a.IsValid())
+ self.assertEqual(a.GetValueAsSigned(), 3)
diff --git a/lldb/test/API/lang/cpp/type_lookup_anon_base_member/main.cpp b/lldb/test/API/lang/cpp/type_lookup_anon_base_member/main.cpp
new file mode 100644
index 0000000000000..f27cf4ce163f9
--- /dev/null
+++ b/lldb/test/API/lang/cpp/type_lookup_anon_base_member/main.cpp
@@ -0,0 +1,18 @@
+struct Base {
+ int b;
+ struct {
+ int x;
+ };
+};
+
+struct Derived : public Base {
+ int a;
+};
+
+int main() {
+ Derived d;
+ d.b = 1;
+ d.x = 2;
+ d.a = 3;
+ return 0; // Set breakpoint here
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
imkiva
changed the title
GetIndexOfChildMemberWithName to handle anonymous struct in base classes
Michael137
reviewed
Sep 12, 2025
|
Hello, sorry to bother, is there a way I can get the pr reviewed? |
Michael137
reviewed
Oct 10, 2025
There was a problem hiding this comment.
I have the suspicion that this could be done much simpler.
CXXRecordDecl::lookupInBases is already perfectly capable of finding fields in anonymous structures in bases. That's why the expression evaluator is able to find them just fine.
I think the actual problem lies in our logic that iterates the CXXBasePaths after the lookupInBases lookup.
The field of the anonymous structure is found by lookupInBases. It is an IndirectFieldDecl. But we fail to extract it in:
for (clang::DeclContext::lookup_iterator I = path->Decls, E;
I != E; ++I) {
child_idx = GetIndexForRecordChild(
parent_record_decl, *I, omit_empty_base_classes);
if (child_idx == UINT32_MAX) {
child_indexes.clear();
return 0;
Here GetIndexForRecordChild doesn't correctly get the index of the IndirectFieldDecl:
6702 clang::RecordDecl::field_iterator field, field_end;
6703 for (field = record_decl->field_begin(), field_end = record_decl->field_end();
6704 field != field_end; ++field, ++child_idx) {
-> 6705 if (field->getCanonicalDecl() == canonical_decl)
6706 return child_idx;
6707 }
6708
6709 return UINT32_MAX;
6710 }
6711
6712 // Look for a child member (doesn't include base classes, but it does include
6713 // their members) in the type hierarchy. Returns an index path into
6714 // "clang_type" on how to reach the appropriate member.
6715 //
Target 0: (lldb) stopped.
(lldb) p field->getCanonicalDecl()->dump()
FieldDecl 0x7e9f01418 <<invalid sloc>> <invalid sloc> implicit 'Base::(anonymous struct)'
(lldb) p canonical_decl->dump()
IndirectFieldDecl 0x7e9f014c8 <<invalid sloc>> <invalid sloc> implicit x 'int'
|-Field 0x7e9f01418 field_index 0 'Base::(anonymous struct)'
`-Field 0x7e9f013c0 'x' 'int'
I haven't fully thought this through, but could we adjust GetIndexForRecordChild to unwrap the anonymous union?
We can fish out the underlying FieldDecls in both cases:
(lldb) p ((clang::IndirectFieldDecl*)canonical_decl)->getAnonField()
(clang::FieldDecl *) 0x0000000c563293c0
(lldb) p field->getType()->getAsCXXRecordDecl()->field_begin()
(clang::RecordDecl::field_iterator) {
Current = {
Current = 0x0000000c563293c0
}
}
(lldb) p field->getType()->getAsCXXRecordDecl()->field_begin()->dump()
FieldDecl 0xc563293c0 <<invalid sloc>> <invalid sloc> x 'int'
So in pseudo-code, maybe the function could look something like:
uint32_t TypeSystemClang::GetIndexForRecordChild(
const clang::RecordDecl *record_decl, clang::NamedDecl *canonical_decl,
bool omit_empty_base_classes) {
uint32_t child_idx = TypeSystemClang::GetNumBaseClasses(
llvm::dyn_cast<clang::CXXRecordDecl>(record_decl),
omit_empty_base_classes);
clang::RecordDecl::field_iterator field, field_end;
for (field = record_decl->field_begin(), field_end = record_decl->field_end();
field != field_end; ++field, ++child_idx) {
IndirectFieldDecl * indirect_decl = llvm::dyn_cast<IndirectFieldDecl>(canonical_decl);
if (field is unnamed && indirect_decl) {
for (anon_field : fields of anonymous structure)
if (anon_field == canonical_decl->getAnonField())
return child_idx;
} else if (field == canonical_decl)
return child_idx;
}
return UINT32_MAX;
}
Might need to adjust the child_idx calculation here. But hopefully that conveys the overall idea.
Let me know if you have any questions or if you think this doesn't work.
We also need to make sure GetChildAtIndex still works, using the index found with GetIndexOfChildMemberWithName.
All that being said, i also worry this might break scripts that relied on the index not accounting for such anonymous structures. We might need to put this behind a boolean option.
|
@Michael137 Thanks for the explanation, you’re right. uint32_t TypeSystemClang::GetIndexForRecordChild(
const clang::RecordDecl *record_decl, clang::NamedDecl *canonical_decl,
bool omit_empty_base_classes) {
uint32_t child_idx = TypeSystemClang::GetNumBaseClasses(
llvm::dyn_cast<clang::CXXRecordDecl>(record_decl),
omit_empty_base_classes);
clang::RecordDecl::field_iterator field, field_end;
for (field = record_decl->field_begin(), field_end = record_decl->field_end();
field != field_end; ++field, ++child_idx) {
if (auto *indirect_decl = llvm::dyn_cast<clang::IndirectFieldDecl>(canonical_decl)) {
// Only meaningful when iterating over the anonymous aggregate slot.
if (field->isAnonymousStructOrUnion()) {
const clang::FieldDecl *anon_aggregate = nullptr;
for (clang::NamedDecl *ND : indirect_decl->chain()) {
if (auto *FD = llvm::dyn_cast<clang::FieldDecl>(ND)) {
if (FD->isAnonymousStructOrUnion() && FD->getDeclContext() == record_decl) {
anon_aggregate = FD;
break;
}
}
}
if (anon_aggregate && field->getCanonicalDecl() == anon_aggregate->getCanonicalDecl())
return child_idx;
}
} else if (field->getCanonicalDecl() == canonical_decl)
return child_idx;
}
return UINT32_MAX;
}However, I later realized the returned child_idx points to the anonymous aggregate in the base, not to the member within that aggregate. That’s why: To address this, I added an overload of
Regarding the concern about breaking existing scripts: the original |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #158131
Similar to #138487 but also search for child indices in the base classes.