Skip to content
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

Jump to bottom

[lldb][DWARFIndex][nfc] Factor out fully qualified name query #76977

Merged
merged 2 commits into from
Jan 8, 2024
Merged

[lldb][DWARFIndex][nfc] Factor out fully qualified name query #76977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b828fb2
[lldb][DWARFIndex][nfc] Factor out fully qualified name query
felipepiovezan Jan 4, 2024
6f6c50b
fixup! [lldb][DWARFIndex][nfc] Factor out fully qualified name query
felipepiovezan Jan 5, 2024
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
20 changes: 20 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//

#include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
#include "DWARFDebugInfoEntry.h"
#include "DWARFDeclContext.h"
#include "Plugins/Language/ObjC/ObjCLanguage.h"
#include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
Expand Down Expand Up @@ -112,3 +114,21 @@ void DWARFIndex::ReportInvalidDIERef(DIERef ref, llvm::StringRef name) const {
"bad die {0:x16} for '{1}')\n",
ref.die_offset(), name.str().c_str());
}

void DWARFIndex::GetFullyQualifiedType(
const DWARFDeclContext &context,
llvm::function_ref<bool(DWARFDIE die)> callback) {
GetTypes(context, [&](DWARFDIE die) {
return GetFullyQualifiedTypeImpl(context, die, callback);
});
}

bool DWARFIndex::GetFullyQualifiedTypeImpl(
const DWARFDeclContext &context, DWARFDIE die,
llvm::function_ref<bool(DWARFDIE die)> callback) {
DWARFDeclContext dwarf_decl_ctx =
die.GetDIE()->GetDWARFDeclContext(die.GetCU());
if (dwarf_decl_ctx == context)
return callback(die);
return true;
}
14 changes: 14 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ class DWARFIndex {
llvm::function_ref<bool(DWARFDIE die)> callback) = 0;
virtual void GetTypes(const DWARFDeclContext &context,
llvm::function_ref<bool(DWARFDIE die)> callback) = 0;

/// Finds all DIEs whose fully qualified name matches `context`. A base
/// implementation is provided, and it uses the entire CU to check the DIE
/// parent hierarchy. Specializations should override this if they are able
/// to provide a faster implementation.
virtual void
GetFullyQualifiedType(const DWARFDeclContext &context,
llvm::function_ref<bool(DWARFDIE die)> callback);
virtual void
GetNamespaces(ConstString name,
llvm::function_ref<bool(DWARFDIE die)> callback) = 0;
Expand Down Expand Up @@ -102,6 +110,12 @@ class DWARFIndex {
}

void ReportInvalidDIERef(DIERef ref, llvm::StringRef name) const;

/// Implementation of `GetFullyQualifiedType` to check a single entry,
/// shareable with derived classes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does the return value signal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm glad you asked, because this API has been driving me crazy 😅
We have to return "true" if we want the "search to keep going" and false if we want the "search to stop", i.e. we found a match. This is what the callbacks are required to do as well.

If I can get support for that, I'd like to rewrite these APIs to return some kind of enumeration with two names: "KeepGoing", "Stop". Future patch though!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm glad you asked, because this API has been driving me crazy 😅 We have to return "true" if we want the "search to keep going" and false if we want the "search to stop", i.e. we found a match. This is what the callbacks are required to do as well.

If I can get support for that, I'd like to rewrite these APIs to return some kind of enumeration with two names: "KeepGoing", "Stop". Future patch though!

Yes, it would be much clearer in the code if we switched to returning an enum. Something like:

enum class IteratorCallbackResult {
  Continue,
  Done
}

bool
GetFullyQualifiedTypeImpl(const DWARFDeclContext &context, DWARFDIE die,
llvm::function_ref<bool(DWARFDIE die)> callback);
};
} // namespace dwarf
} // namespace lldb_private::plugin
Expand Down
9 changes: 2 additions & 7 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3095,7 +3095,7 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
}

const DWARFDeclContext die_dwarf_decl_ctx = GetDWARFDeclContext(die);
m_index->GetTypes(die_dwarf_decl_ctx, [&](DWARFDIE type_die) {
m_index->GetFullyQualifiedType(die_dwarf_decl_ctx, [&](DWARFDIE type_die) {
// Make sure type_die's language matches the type system we are
// looking for. We don't want to find a "Foo" type from Java if we
// are looking for a "Foo" type for C, C++, ObjC, or ObjC++.
Expand All @@ -3122,9 +3122,8 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
return true;
}

DWARFDeclContext type_dwarf_decl_ctx = GetDWARFDeclContext(type_die);

if (log) {
DWARFDeclContext type_dwarf_decl_ctx = GetDWARFDeclContext(type_die);
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::"
Expand All @@ -3134,10 +3133,6 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
type_dwarf_decl_ctx.GetQualifiedName());
}

// Make sure the decl contexts match all the way up
if (die_dwarf_decl_ctx != type_dwarf_decl_ctx)
return true;

Type *resolved_type = ResolveType(type_die, false);
if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
return true;
Expand Down