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

[felipepiovezan]

This moves the functionally of finding a DIE based on a fully qualified name from SymbolFileDWARF into DWARFIndex itself, so that specializations of DWARFIndex can implement faster versions of this query.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

This moves the functionally of finding a DIE based on a fully qualified name from SymbolFileDWARF into DWARFIndex itself, so that specializations of DWARFIndex can implement faster versions of this query.

---

Full diff: https://github.com/llvm/llvm-project/pull/76977.diff

3 Files Affected:

• (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp (+14)
• (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h (+8)
• (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+2-7)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index b1c323b101cef3..01b47551b25c30 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -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"
@@ -112,3 +114,15 @@ 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) {
+    DWARFDeclContext dwarf_decl_ctx =
+        die.GetDIE()->GetDWARFDeclContext(die.GetCU());
+    if (dwarf_decl_ctx == context && callback(die))
+      return false;
+    return true;
+  });
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index 9aadeddbb21753..4fd10a634fc57f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -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;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 505ea29ca4d4f5..e7dc9115cd80e7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -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++.
@@ -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::"
@@ -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;

[clayborg]

LGTM

[felipepiovezan]

Thanks for the review @clayborg
Just to let you know, I decided to make a helper function so that derived classes can share part of this implementation (you can see the diff by looking only at the fixup commit)

[adrian-prantl]

What does the return value signal?

[felipepiovezan]

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!

[clayborg]

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
}