Skip to content

Conversation

@augusto2112
Copy link
Contributor

Some months ago, the LookupInfo constructor logic was refactored to not depend on language specific logic, and use languages plugins instead. In this refactor, when the language type is unknown, a single LookupInfo object will handle multiple languages. This doesn't work well, as multiple languages might want to configure the LookupInfo object in different ways. For example, different languages might want to set the m_lookup_name differently from each other, but the previous implementation would pick the first name a language provided, and effectively ignored every other language. Other fields of the LookupInfo object are also configured in incompatible ways.

This approach doesn't seem to be a problem upstream, since only the C++/Objective-C language plugins are available, but it broke downstream on the Swift fork, as adding Swift to the list of default languages when the language type is unknown breaks C++ tests.

This patch makes it so instead of building a single LookupInfo object for multiple languages, one LookupInfo object is built per language instead.

rdar://159531216

@llvmbot
Copy link
Member

llvmbot commented Nov 20, 2025

@llvm/pr-subscribers-lldb

Author: Augusto Noronha (augusto2112)

Changes

Some months ago, the LookupInfo constructor logic was refactored to not depend on language specific logic, and use languages plugins instead. In this refactor, when the language type is unknown, a single LookupInfo object will handle multiple languages. This doesn't work well, as multiple languages might want to configure the LookupInfo object in different ways. For example, different languages might want to set the m_lookup_name differently from each other, but the previous implementation would pick the first name a language provided, and effectively ignored every other language. Other fields of the LookupInfo object are also configured in incompatible ways.

This approach doesn't seem to be a problem upstream, since only the C++/Objective-C language plugins are available, but it broke downstream on the Swift fork, as adding Swift to the list of default languages when the language type is unknown breaks C++ tests.

This patch makes it so instead of building a single LookupInfo object for multiple languages, one LookupInfo object is built per language instead.

rdar://159531216


Patch is 24.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/168797.diff

13 Files Affected:

  • (modified) lldb/include/lldb/Core/Module.h (+43-2)
  • (modified) lldb/include/lldb/Symbol/SymbolContext.h (+14)
  • (modified) lldb/include/lldb/Symbol/SymbolFile.h (+4)
  • (modified) lldb/source/Breakpoint/BreakpointResolverName.cpp (+21-11)
  • (modified) lldb/source/Core/Module.cpp (+59-32)
  • (modified) lldb/source/Core/ModuleList.cpp (+26-22)
  • (modified) lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp (+1-1)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp (+8)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h (+5)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+5-5)
  • (modified) lldb/source/Symbol/SymbolContext.cpp (+28-7)
  • (modified) lldb/source/Symbol/SymbolFile.cpp (+8)
  • (modified) lldb/tools/lldb-test/lldb-test.cpp (+4-3)
diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 8513e147ee523..de4e416218f73 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -315,6 +315,22 @@ class Module : public std::enable_shared_from_this<Module>,
                      const ModuleFunctionSearchOptions &options,
                      SymbolContextList &sc_list);
 
+  /// Find functions by a vector of lookup infos.
+  ///
+  /// If the function is an inlined function, it will have a block,
+  /// representing the inlined function, and the function will be the
+  /// containing function.  If it is not inlined, then the block will be NULL.
+  ///
+  /// \param[in] lookup_infos
+  ///     The vector of lookup infos of the function we are looking for.
+  ///
+  /// \param[out] sc_list
+  ///     A symbol context list that gets filled in with all of the
+  ///     matches.
+  void FindFunctions(const std::vector<LookupInfo> &lookup_infos,
+                     const CompilerDeclContext &parent_decl_ctx,
+                     const ModuleFunctionSearchOptions &options,
+                     SymbolContextList &sc_list);
   /// Find functions by name.
   ///
   /// If the function is an inlined function, it will have a block,
@@ -917,8 +933,29 @@ class Module : public std::enable_shared_from_this<Module>,
   public:
     LookupInfo() = default;
 
-    LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask,
-               lldb::LanguageType language);
+    /// Creates a vector of lookup infos for function name resolution.
+    ///
+    /// \param[in] name
+    ///     The function name to search for. This can be a simple name like
+    ///     "foo" or a qualified name like "Class::method".
+    ///
+    /// \param[in] name_type_mask
+    ///     A bitmask specifying what types of names to search for
+    ///     (e.g., eFunctionNameTypeFull, eFunctionNameTypeBase,
+    ///     eFunctionNameTypeMethod, eFunctionNameTypeAuto). Multiple types
+    ///     can be combined with bitwise OR.
+    ///
+    /// \param[in] requested_lang_type
+    ///     The language to create lookups for. If eLanguageTypeUnknown is
+    ///     passed, creates one LookupInfo for each language plugin currently
+    ///     available in LLDB. If a specific language is provided, creates only
+    //      a single LookupInfo for that language.
+    ///
+    /// \return
+    ///     A vector of LookupInfo objects, one per relevant language.
+    static std::vector<LookupInfo>
+    MakeLookupInfos(ConstString name, lldb::FunctionNameType name_type_mask,
+                    lldb::LanguageType requested_lang_type);
 
     ConstString GetName() const { return m_name; }
 
@@ -959,6 +996,10 @@ class Module : public std::enable_shared_from_this<Module>,
     /// If \b true, then demangled names that match will need to contain
     /// "m_name" in order to be considered a match
     bool m_match_name_after_lookup = false;
+
+  private:
+    LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask,
+               lldb::LanguageType lang_type);
   };
 
   /// Get a unique hash for this module.
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h
index af2f694e554de..0834825cdbd25 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -231,6 +231,20 @@ class SymbolContext {
 
   lldb::LanguageType GetLanguage() const;
 
+  /// Compares the two symbol contexts, considering that the symbol may or may
+  /// not be present. If both symbols are present, compare them, if one of the
+  /// symbols is not present, consider the symbol contexts as equal as long as
+  /// the other fields are equal.
+  ///
+  /// This function exists because SymbolContexts are often created without the
+  /// symbol, which is filled in later on, after its creation.
+  static bool CompareConsideringPossiblyNullSymbol(const SymbolContext &lhs,
+                                                   const SymbolContext &rhs);
+
+  /// Compares the two symbol contexts, except for the symbol field.
+  static bool CompareWithoutSymbol(const SymbolContext &lhs,
+                                   const SymbolContext &rhs);
+
   /// Find a block that defines the function represented by this symbol
   /// context.
   ///
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h
index 3b4d7bc01d132..305eb0f201b37 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -309,6 +309,10 @@ class SymbolFile : public PluginInterface {
   virtual void FindFunctions(const Module::LookupInfo &lookup_info,
                              const CompilerDeclContext &parent_decl_ctx,
                              bool include_inlines, SymbolContextList &sc_list);
+  virtual void
+  FindFunctions(const std::vector<Module::LookupInfo> &lookup_infos,
+                const CompilerDeclContext &parent_decl_ctx,
+                bool include_inlines, SymbolContextList &sc_list);
   virtual void FindFunctions(const RegularExpression &regex,
                              bool include_inlines, SymbolContextList &sc_list);
 
diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp b/lldb/source/Breakpoint/BreakpointResolverName.cpp
index 4f252f91cccdc..3a7f12a238da4 100644
--- a/lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -218,19 +218,22 @@ StructuredData::ObjectSP BreakpointResolverName::SerializeToStructuredData() {
 
 void BreakpointResolverName::AddNameLookup(ConstString name,
                                            FunctionNameType name_type_mask) {
-
-  Module::LookupInfo lookup(name, name_type_mask, m_language);
-  m_lookups.emplace_back(lookup);
+  std::vector<Module::LookupInfo> infos =
+      Module::LookupInfo::MakeLookupInfos(name, name_type_mask, m_language);
+  llvm::append_range(m_lookups, infos);
 
   auto add_variant_funcs = [&](Language *lang) {
     for (Language::MethodNameVariant variant :
          lang->GetMethodNameVariants(name)) {
       // FIXME: Should we be adding variants that aren't of type Full?
       if (variant.GetType() & lldb::eFunctionNameTypeFull) {
-        Module::LookupInfo variant_lookup(name, variant.GetType(),
-                                          lang->GetLanguageType());
-        variant_lookup.SetLookupName(variant.GetName());
-        m_lookups.emplace_back(variant_lookup);
+        std::vector<Module::LookupInfo> variant_lookups =
+            Module::LookupInfo::MakeLookupInfos(name, variant.GetType(),
+                                               lang->GetLanguageType());
+        llvm::for_each(variant_lookups, [&](auto &variant_lookup) {
+          variant_lookup.SetLookupName(variant.GetName());
+        });
+        llvm::append_range(m_lookups, variant_lookups);
       }
     }
     return IterationAction::Continue;
@@ -401,14 +404,21 @@ void BreakpointResolverName::GetDescription(Stream *s) {
   if (m_match_type == Breakpoint::Regexp)
     s->Printf("regex = '%s'", m_regex.GetText().str().c_str());
   else {
-    size_t num_names = m_lookups.size();
-    if (num_names == 1)
-      s->Printf("name = '%s'", m_lookups[0].GetName().GetCString());
+    // Since there may be many lookups objects for the same name breakpoint (one per language available),
+    // unique them by name, and operate on those unique names.
+    std::vector<ConstString> unique_lookups;
+    for (auto &lookup : m_lookups) {
+      if (!llvm::is_contained(unique_lookups, lookup.GetName()))
+        unique_lookups.push_back(lookup.GetName());
+    }
+    if (unique_lookups.size() == 1)
+      s->Printf("name = '%s'", unique_lookups[0].GetCString());
     else {
+      size_t num_names = unique_lookups.size();
       s->Printf("names = {");
       for (size_t i = 0; i < num_names; i++) {
         s->Printf("%s'%s'", (i == 0 ? "" : ", "),
-                  m_lookups[i].GetName().GetCString());
+                  unique_lookups[i].GetCString());
       }
       s->Printf("}");
     }
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 815cc9dada2c1..1595918cf762a 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -643,26 +643,13 @@ void Module::FindCompileUnits(const FileSpec &path,
 
 Module::LookupInfo::LookupInfo(ConstString name,
                                FunctionNameType name_type_mask,
-                               LanguageType language)
-    : m_name(name), m_lookup_name(name), m_language(language) {
+                               LanguageType lang_type)
+    : m_name(name), m_lookup_name(name), m_language(lang_type) {
   std::optional<ConstString> basename;
-
-  std::vector<Language *> languages;
-  {
-    std::vector<LanguageType> lang_types;
-    if (language != eLanguageTypeUnknown)
-      lang_types.push_back(language);
-    else
-      lang_types = {eLanguageTypeObjC, eLanguageTypeC_plus_plus};
-
-    for (LanguageType lang_type : lang_types) {
-      if (Language *lang = Language::FindPlugin(lang_type))
-        languages.push_back(lang);
-    }
-  }
+  Language *lang = Language::FindPlugin(lang_type);
 
   if (name_type_mask & eFunctionNameTypeAuto) {
-    for (Language *lang : languages) {
+    if (lang) {
       auto info = lang->GetFunctionNameInfo(name);
       if (info.first != eFunctionNameTypeNone) {
         m_name_type_mask |= info.first;
@@ -679,7 +666,7 @@ Module::LookupInfo::LookupInfo(ConstString name,
 
   } else {
     m_name_type_mask = name_type_mask;
-    for (Language *lang : languages) {
+    if (lang) {
       auto info = lang->GetFunctionNameInfo(name);
       if (info.first & m_name_type_mask) {
         // If the user asked for FunctionNameTypes that aren't possible,
@@ -688,14 +675,12 @@ Module::LookupInfo::LookupInfo(ConstString name,
         // ObjC)
         m_name_type_mask &= info.first;
         basename = info.second;
-        break;
-      }
-      // Still try and get a basename in case someone specifies a name type mask
-      // of eFunctionNameTypeFull and a name like "A::func"
-      if (name_type_mask & eFunctionNameTypeFull &&
-          info.first != eFunctionNameTypeNone && !basename && info.second) {
+      } else if (name_type_mask & eFunctionNameTypeFull &&
+                 info.first != eFunctionNameTypeNone && !basename &&
+                 info.second) {
+        // Still try and get a basename in case someone specifies a name type
+        // mask of eFunctionNameTypeFull and a name like "A::func"
         basename = info.second;
-        break;
       }
     }
   }
@@ -711,6 +696,37 @@ Module::LookupInfo::LookupInfo(ConstString name,
   }
 }
 
+std::vector<Module::LookupInfo>
+Module::LookupInfo::MakeLookupInfos(ConstString name,
+                                   lldb::FunctionNameType name_type_mask,
+                                     lldb::LanguageType requested_lang_type) {
+  std::vector<LanguageType> lang_types;
+  if (requested_lang_type != eLanguageTypeUnknown) {
+    lang_types.push_back(requested_lang_type);
+  } else {
+    // If the language type was not specified, look up in every language
+    // available.
+    Language::ForEach([&](Language *lang) {
+      auto lang_type = lang->GetLanguageType();
+      if (!llvm::is_contained(lang_types, lang_type))
+        lang_types.push_back(lang_type);
+      return IterationAction::Continue;
+    });
+
+    if (lang_types.empty())
+      lang_types = {eLanguageTypeObjC, eLanguageTypeC_plus_plus};
+  }
+
+  std::vector<Module::LookupInfo> infos;
+  infos.reserve(lang_types.size());
+  for (LanguageType lang_type : lang_types) {
+    Module::LookupInfo info(name, name_type_mask, lang_type);
+    infos.push_back(info);
+  }
+  return infos;
+}
+
+
 bool Module::LookupInfo::NameMatchesLookupInfo(
     ConstString function_name, LanguageType language_type) const {
   // We always keep unnamed symbols
@@ -824,18 +840,29 @@ void Module::FindFunctions(const Module::LookupInfo &lookup_info,
   }
 }
 
+void Module::FindFunctions(const std::vector<Module::LookupInfo> &lookup_infos,
+                           const CompilerDeclContext &parent_decl_ctx,
+                           const ModuleFunctionSearchOptions &options,
+                           SymbolContextList &sc_list) {
+  for (auto &lookup_info : lookup_infos)
+    FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
+}
+
 void Module::FindFunctions(ConstString name,
                            const CompilerDeclContext &parent_decl_ctx,
                            FunctionNameType name_type_mask,
                            const ModuleFunctionSearchOptions &options,
                            SymbolContextList &sc_list) {
-  const size_t old_size = sc_list.GetSize();
-  LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
-  FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
-  if (name_type_mask & eFunctionNameTypeAuto) {
-    const size_t new_size = sc_list.GetSize();
-    if (old_size < new_size)
-      lookup_info.Prune(sc_list, old_size);
+  std::vector<LookupInfo> lookup_infos =
+      LookupInfo::MakeLookupInfos(name, name_type_mask, eLanguageTypeUnknown);
+  for (auto &lookup_info : lookup_infos) {
+    const size_t old_size = sc_list.GetSize();
+    FindFunctions(lookup_info, parent_decl_ctx, options, sc_list);
+    if (name_type_mask & eFunctionNameTypeAuto) {
+      const size_t new_size = sc_list.GetSize();
+      if (old_size < new_size)
+        lookup_info.Prune(sc_list, old_size);
+    }
   }
 }
 
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 5444c04e74522..2e937e3f2d3f7 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -453,21 +453,22 @@ void ModuleList::FindFunctions(ConstString name,
                                FunctionNameType name_type_mask,
                                const ModuleFunctionSearchOptions &options,
                                SymbolContextList &sc_list) const {
-  const size_t old_size = sc_list.GetSize();
-
   if (name_type_mask & eFunctionNameTypeAuto) {
-    Module::LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
-
+    std::vector<Module::LookupInfo> lookup_infos =
+        Module::LookupInfo::MakeLookupInfos(name, name_type_mask,
+                                           eLanguageTypeUnknown);
     std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
-    for (const ModuleSP &module_sp : m_modules) {
-      module_sp->FindFunctions(lookup_info, CompilerDeclContext(), options,
-                               sc_list);
-    }
-
-    const size_t new_size = sc_list.GetSize();
+    for (const auto &lookup_info : lookup_infos) {
+      const size_t old_size = sc_list.GetSize();
+      for (const ModuleSP &module_sp : m_modules) {
+        module_sp->FindFunctions(lookup_info, CompilerDeclContext(), options,
+                                 sc_list);
+      }
 
-    if (old_size < new_size)
-      lookup_info.Prune(sc_list, old_size);
+      const size_t new_size = sc_list.GetSize();
+      if (old_size < new_size)
+        lookup_info.Prune(sc_list, old_size);
+    }
   } else {
     std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
     for (const ModuleSP &module_sp : m_modules) {
@@ -480,21 +481,24 @@ void ModuleList::FindFunctions(ConstString name,
 void ModuleList::FindFunctionSymbols(ConstString name,
                                      lldb::FunctionNameType name_type_mask,
                                      SymbolContextList &sc_list) {
-  const size_t old_size = sc_list.GetSize();
-
   if (name_type_mask & eFunctionNameTypeAuto) {
-    Module::LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown);
+    std::vector<Module::LookupInfo> lookup_infos =
+        Module::LookupInfo::MakeLookupInfos(name, name_type_mask,
+                                           eLanguageTypeUnknown);
 
     std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
-    for (const ModuleSP &module_sp : m_modules) {
-      module_sp->FindFunctionSymbols(lookup_info.GetLookupName(),
-                                     lookup_info.GetNameTypeMask(), sc_list);
-    }
+    for (const auto &lookup_info : lookup_infos) {
+      const size_t old_size = sc_list.GetSize();
+      for (const ModuleSP &module_sp : m_modules) {
+        module_sp->FindFunctionSymbols(lookup_info.GetLookupName(),
+                                       lookup_info.GetNameTypeMask(), sc_list);
+      }
 
-    const size_t new_size = sc_list.GetSize();
+      const size_t new_size = sc_list.GetSize();
 
-    if (old_size < new_size)
-      lookup_info.Prune(sc_list, old_size);
+      if (old_size < new_size)
+        lookup_info.Prune(sc_list, old_size);
+    }
   } else {
     std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
     for (const ModuleSP &module_sp : m_modules) {
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index ce2ba69be2e96..14932e957d081 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -437,7 +437,7 @@ void SymbolFileBreakpad::FindFunctions(
       sc.comp_unit = cu_sp.get();
       sc.function = func_sp.get();
       sc.module_sp = func_sp->CalculateSymbolContextModule();
-      sc_list.Append(sc);
+      sc_list.AppendIfUnique(sc, /*merge_symbol_into_function=*/true);
     }
   }
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index 64a8005308454..c4efc06ab7873 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -173,6 +173,14 @@ void DWARFIndex::GetNamespacesWithParents(
   });
 }
 
+void DWARFIndex::GetFunctions(
+    const std::vector<Module::LookupInfo> &lookup_infos, SymbolFileDWARF &dwarf,
+    const CompilerDeclContext &parent_decl_ctx,
+    llvm::function_ref<IterationAction(DWARFDIE die)> callback) {
+  for (auto &lookup_info : lookup_infos)
+    GetFunctions(lookup_info, dwarf, parent_decl_ctx, callback);
+}
+
 IterationAction DWARFIndex::ProcessNamespaceDieMatchParents(
     const CompilerDeclContext &parent_decl_ctx, DWARFDIE die,
     llvm::function_ref<IterationAction(DWARFDIE die)> callback) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index be73255aaf141..eaf1da123602e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -86,6 +86,11 @@ class DWARFIndex {
                const CompilerDeclContext &parent_decl_ctx,
                llvm::function_ref<IterationAction(DWARFDIE die)> callback) = 0;
   virtual void
+  GetFunctions(const std::vector<Module::LookupInfo> &lookup_infos,
+               SymbolFileDWARF &dwarf,
+               const CompilerDeclContext &parent_decl_ctx,
+               llvm::function_ref<IterationAction(DWARFDIE die)> callback);
+  virtual void
   GetFunctions(const RegularExpression &regex,
                llvm::function_ref<IterationAction(DWARFDIE die)> callback) = 0;
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index bcb3ad854c388..66b2b5c91bf18 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2482,7 +2482,7 @@ bool SymbolFileDWARF::ResolveFunction(const DWARFDIE &orig_die,
         sc.block = function_block.FindBlockByID(inlined_die.GetOffset());
     }
 
-    sc_list.Append(sc);
+    sc_list.AppendIfUnique(sc, /*merge_symbol_into_function=*/true);
     return true;
   }
 
@@ -2551,11 +2551,11 @@ SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label,
                                         const DWARFDIE &declaration) {
   auto do_lookup = [this](llvm::StringRef lookup_name) -> DWARFDIE {
     DWARFDIE found;
-    Module::LookupInfo info(ConstString(lookup_name),
-                            lldb::eFunctionNameTypeFull,
-                            lldb::eLanguageTypeUnknown);
+    auto lookup_infos = Module::LookupInfo::M...
[truncated]

@github-actions
Copy link

github-actions bot commented Nov 20, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Some months ago, the LookupInfo constructor logic was refactored to
not depend on language specific logic, and use languages plugins
instead. In this refactor, when the language type is unknown, a
single LookupInfo object will handle multiple languages. This doesn't
work well, as multiple languages might want to configure the LookupInfo
object in different ways. For example, different languages might want
to set the m_lookup_name differently from each other, but the previous
implementation would pick the first name a language provided, and
effectively ignored every other language. Other fields of the LookupInfo
object are also configured in incompatible ways.

This approach doesn't seem to be a problem upstream, since only
the C++/Objective-C language plugins are available, but it broke
downstream on the Swift fork, as adding Swift to the list of default
languages when the language type is unknown breaks C++ tests.

This patch makes it so instead of building a single LookupInfo
object for multiple languages, one LookupInfo object is built per
language instead.

rdar://159531216
@github-actions
Copy link

🐧 Linux x64 Test Results

  • 33167 tests passed
  • 490 tests skipped

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

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

Looks reasonable. Based on your description, the issue doesn't manifest itself with the upstream languages, which complicates a test. Any chance we can unit test this with some mocking? If not, can we at least add a few unit tests for the helpers?

Comment on lines +323 to +329
///
/// \param[in] lookup_infos
/// The vector of lookup infos of the function we are looking for.
///
/// \param[out] sc_list
/// A symbol context list that gets filled in with all of the
/// matches.
Copy link
Member

Choose a reason for hiding this comment

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

The Doxygen comment is missing arguments. I personally think it adds little value compared to the cost of keeping it in sync as the function will eventually change, so I would just omit it.

/// A vector of LookupInfo objects, one per relevant language.
static std::vector<LookupInfo>
MakeLookupInfos(ConstString name, lldb::FunctionNameType name_type_mask,
lldb::LanguageType requested_lang_type);
Copy link
Member

Choose a reason for hiding this comment

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

Why not just lang_type?

Suggested change
lldb::LanguageType requested_lang_type);
lldb::LanguageType lang_type);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants