[lldb] Make Module::LookupInfo::Prune language-agnostic

[bulbazord]

Summary

Module::LookupInfo::Prune tries to prune results from lookups that don't
match the user-provided lookup information. If we're trying to match a
name with type eFunctionNameTypeFull, we want to match what the user
provided exactly. Currently, we try to see if what the user provided is
matches exactly assuming that a C++ name was provided. However, this is
specific to the C++ debugging path. LLDB supports more languages than
C++, so it is important that this codepath be language-agnostic.

[labath]

How hot is this code? Parsing a demangled name isn't entirely cheap, and the fact that the objc++ class calls back into the c++ version means that the c++ name will be parsed twice.

Could we move this pruning elsewhere? These values come from the symbol file plugins anyway, and they can do a better job at determining which language does a particular name belong to.

(OK, they can an also come from the symtab, but there I guess we could infer something from the mangling scheme).

lldb/include/lldb/Target/Language.h

320

I think we're passing ConstStrings by value these days...

[bulbazord]

In D128366#3603943, @labath wrote:

How hot is this code? Parsing a demangled name isn't entirely cheap, and the fact that the objc++ class calls back into the c++ version means that the c++ name will be parsed twice.

Not sure, I'll measure this. Also, at least on my machine, the list of language plugins includes at least 3 C++ language plugins, so it happens *at least* twice...

Could we move this pruning elsewhere? These values come from the symbol file plugins anyway, and they can do a better job at determining which language does a particular name belong to.
(OK, they can an also come from the symtab, but there I guess we could infer something from the mangling scheme).

We're specifically pruning the results from a name lookup, so I'm not sure where would be a better place to move it. That being said, based on your observation, we could try to do a better job of inferring the language a name belongs to by looking at the mangling scheme before we look at every single language plugin.

lldb/include/lldb/Target/Language.h

320

I see. Is there a reason we do that instead of passing by reference?

[labath]

In D128366#3605484, @bulbazord wrote:

In D128366#3603943, @labath wrote:

Could we move this pruning elsewhere? These values come from the symbol file plugins anyway, and they can do a better job at determining which language does a particular name belong to.
(OK, they can an also come from the symtab, but there I guess we could infer something from the mangling scheme).

We're specifically pruning the results from a name lookup, so I'm not sure where would be a better place to move it.

Well. I guess ideally I would pass whatever information is needed into the lookup functions themselves, so that there is no need for the additional filtering. I don't know why it's done this way but this setup seems inefficient, as we have to generate all these SymbolContext only for them to be (potentially) thrown away...

lldb/include/lldb/Target/Language.h

320

ConstString is just a fancy wrapper around a const char *, so adding a layer of indirection does not really save us anything (in fact, it's the opposite).

[bulbazord]

In D128366#3608051, @labath wrote:

In D128366#3605484, @bulbazord wrote:

In D128366#3603943, @labath wrote:

Could we move this pruning elsewhere? These values come from the symbol file plugins anyway, and they can do a better job at determining which language does a particular name belong to.
(OK, they can an also come from the symtab, but there I guess we could infer something from the mangling scheme).

We're specifically pruning the results from a name lookup, so I'm not sure where would be a better place to move it.

Well. I guess ideally I would pass whatever information is needed into the lookup functions themselves, so that there is no need for the additional filtering. I don't know why it's done this way but this setup seems inefficient, as we have to generate all these SymbolContext only for them to be (potentially) thrown away...

I see what you mean now. I'll look at this instead, maybe we can get rid of LookupInfo::Prune altogether. :)

lldb/include/lldb/Target/Language.h

320

Ah, this makes sense. I should have looked at ConstString more closely. Thank you! :)

lldb/include/lldb/Target/Language.h

public:
   }
 
   virtual ConstString
   FindBestAlternateFunctionMangledName(const Mangled mangled,
                                        const SymbolContext &sym_ctx) const {
     return ConstString();
   }
 
+  virtual bool NamesAreEquivalentWithContext(
+      const ConstString &user_provided_name,
+      const ConstString &full_name_with_context) const {
+    return user_provided_name == full_name_with_context;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
   Language();
 
 private:
   Language(const Language &) = delete;
   const Language &operator=(const Language &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_LANGUAGE_H

lldb/source/Core/Module.cpp

if (m_match_name_after_lookup && m_name) {
     }
   }
 
   // If we have only full name matches we might have tried to set breakpoint on
   // "func" and specified eFunctionNameTypeFull, but we might have found
   // "a::func()", "a::b::func()", "c::func()", "func()" and "func". Only
   // "func()" and "func" should end up matching.
   if (m_name_type_mask == eFunctionNameTypeFull) {
+    std::vector<Language *> languages;
+    if (m_language != eLanguageTypeUnknown) {
+      if (auto *lang = Language::FindPlugin(m_language))
+        languages.push_back(lang);
+    } else {
+      Language::ForEach([&languages](Language *l) {
+        languages.push_back(l);
+        return true;
+      });
+    }
+
     SymbolContext sc;
     size_t i = start_idx;
     while (i < sc_list.GetSize()) {
       if (!sc_list.GetContextAtIndex(i, sc))
         break;
       // Make sure the mangled and demangled names don't match before we try to
       // pull anything out
       ConstString mangled_name(sc.GetFunctionName(Mangled::ePreferMangled));
       ConstString full_name(sc.GetFunctionName());
       if (mangled_name != m_name && full_name != m_name) {
-        CPlusPlusLanguage::MethodName cpp_method(full_name);
-        if (cpp_method.IsValid()) {
-          if (cpp_method.GetContext().empty()) {
-            if (cpp_method.GetBasename().compare(m_name.GetStringRef()) != 0) {
-              sc_list.RemoveContextAtIndex(i);
-              continue;
-            }
-          } else {
-            std::string qualified_name;
-            llvm::StringRef anon_prefix("(anonymous namespace)");
-            if (cpp_method.GetContext() == anon_prefix)
-              qualified_name = cpp_method.GetBasename().str();
-            else
-              qualified_name = cpp_method.GetScopeQualifiedName();
-            if (qualified_name != m_name.GetCString()) {
-              sc_list.RemoveContextAtIndex(i);
-              continue;
-            }
+        bool removed = false;
+        for (const auto *lang : languages) {
+          if (!lang->NamesAreEquivalentWithContext(m_name, full_name)) {
+            removed = true;
+            break;
           }
         }
+        if (removed)
+          continue;
       }
       ++i;
     }
   }
 }
 
 void Module::FindFunctions(ConstString name,
                            const CompilerDeclContext &parent_decl_ctx,

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h

static bool ExtractContextAndIdentifier(const char *name,
                                           llvm::StringRef &identifier);
 
   std::vector<ConstString>
   GenerateAlternateFunctionManglings(const ConstString mangled) const override;
 
   ConstString FindBestAlternateFunctionMangledName(
       const Mangled mangled, const SymbolContext &sym_ctx) const override;
 
+  bool NamesAreEquivalentWithContext(
+      const ConstString &user_provided_name,
+      const ConstString &full_name_with_context) const override;
+
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_CPLUSPLUSLANGUAGE_H

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

for (auto suffix : suffixes) {
     if (file_path.endswith_insensitive(suffix))
       return true;
   }
 
   // Check if we're in a STL path (where the files usually have no extension
   // that we could check for.
   return file_path.contains("/usr/include/c++/");
 }
+
+bool CPlusPlusLanguage::NamesAreEquivalentWithContext(
+    const ConstString &user_provided_name,
+    const ConstString &full_name_with_context) const {
+  MethodName method(full_name_with_context);
+  if (!method.IsValid())
+    return true;
+
+  if (method.GetContext().empty()) {
+    if (method.GetBasename().compare(user_provided_name.GetStringRef()) != 0)
+      return false;
+  } else {
+    std::string qualified_name;
+    llvm::StringRef anon_prefix("(anonymous namespace)");
+    if (method.GetContext() == anon_prefix)
+      qualified_name = method.GetBasename().str();
+    else
+      qualified_name = method.GetScopeQualifiedName();
+    if (qualified_name != user_provided_name.GetCString()) {
+      return false;
+    }
+  }
+
+  return true;
+}

lldb/source/Plugins/Language/ObjC/ObjCLanguage.h

static bool IsPossibleObjCSelector(const char *name) {
     if (strchr(name, ':') == nullptr)
       return true;
     else if (name[strlen(name) - 1] == ':')
       return true;
     else
       return false;
   }
 
+  bool NamesAreEquivalentWithContext(
+      const ConstString &user_provided_name,
+      const ConstString &full_name_with_context) const override;
+
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_LANGUAGE_OBJC_OBJCLANGUAGE_H

lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

bool isObjCpointer =
       (((valobj.GetCompilerType().GetTypeInfo(nullptr)) & mask) == mask);
   if (!isObjCpointer)
     return false;
   bool canReadValue = true;
   bool isZero = valobj.GetValueAsUnsigned(0, &canReadValue) == 0;
   return canReadValue && isZero;
 }
 
+bool ObjCLanguage::NamesAreEquivalentWithContext(
+    const ConstString &user_provided_name,
+    const ConstString &full_name_with_context) const {
+  ObjCLanguage::MethodName user_method(user_provided_name.GetCString(), false);
+  ObjCLanguage::MethodName full_method(full_name_with_context.GetCString(),
+                                       false);
+
+  if (!user_method.IsValid() || !full_method.IsValid())
+    return true;
+  return user_method.GetClassName() == full_method.GetClassName() &&
+         user_method.GetSelector() == full_method.GetSelector();
+}
+
 bool ObjCLanguage::IsSourceFile(llvm::StringRef file_path) const {
   const auto suffixes = {".h", ".m", ".M"};
   for (auto suffix : suffixes) {
     if (file_path.endswith_insensitive(suffix))
       return true;
   }
   return false;
 }

lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h

public:
   static void Initialize();
 
   static void Terminate();
 
   static lldb_private::Language *CreateInstance(lldb::LanguageType language);
 
   static llvm::StringRef GetPluginNameStatic() { return "objcplusplus"; }
 
+  bool NamesAreEquivalentWithContext(
+      const ConstString &user_provided_name,
+      const ConstString &full_name_with_context) const override;
+
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_LANGUAGE_OBJCPLUSPLUS_OBJCPLUSPLUSLANGUAGE_H

lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp

 Language *ObjCPlusPlusLanguage::CreateInstance(lldb::LanguageType language) {
   switch (language) {
   case lldb::eLanguageTypeObjC_plus_plus:
     return new ObjCPlusPlusLanguage();
   default:
     return nullptr;
   }
 }
+
+bool ObjCPlusPlusLanguage::NamesAreEquivalentWithContext(
+    const ConstString &user_provided_name,
+    const ConstString &full_name_with_context) const {
+  if (const auto *cpp_lang = Language::FindPlugin(eLanguageTypeC_plus_plus)) {
+    if (!cpp_lang->NamesAreEquivalentWithContext(user_provided_name,
+                                                 full_name_with_context))
+      return false;
+  } else if (const auto *objc_lang = Language::FindPlugin(eLanguageTypeObjC)) {
+    if (!objc_lang->NamesAreEquivalentWithContext(user_provided_name,
+                                                  full_name_with_context))
+      return false;
+  }
+  return true;
+}