[lldb] Introduce Language::MethodNameInfo

[bulbazord]

Summary

There are several instances where one may want to ask questions about
the name of a method/function/whatever. Classes like
CPlusPlusLanguage::MethodName and ObjCLanguage::MethodName fulfill this
purpose right now, but users of these have to have knowledge on these
classes (from plugins) to function. I propose a new class
Language::MethodNameInfo to abstract this away so that lldb's Core
libraries can ask questions about a method name without knowing the
details of a plugin.

Currently the class only has one method to override, GetSelector, but
I imagine it will have more in the future. I only added the one method
needed to replace the use of ObjCLanguage in Symtab.

[bulbazord]

Let me know how you feel about this abstraction/idea. I think we'll need something like this at some point to remove most of the remaining dependencies (e.g. in Module.cpp) so I'd like to get something good going here.

[jingham]

The idea seems good, but I'm not sure how you are going to extend it? "Selector" seems a pretty ObjC specific term to me. Will this be what you use to get the "method name" part of the C++ names? That would look a bit odd to me, but OTOH having a GetSelector in all the subclasses, most of which don't have selectors also seems a little odd. This might be solved by just thinking of a better name, however. Also note, in the C++ case, it would be useful to distinguish between the method name, like Bar in Foo::Bar, and the fully qualified method name (like Bar(int, double) in Foo::Bar(int, double). That might effect the choice of names. The selector is the equivalent of a C++ fully qualified method name.

[teemperor]

+1 to not add any language-specific terms to the Language API.

Looking over the code, it seems what the code is doing its:

• Going over the symbol names.
• Creating a search index from the symbol names that is optionally grouped via the FunctionNameType cases (and there is a 'eFunctionNameTypeSelector' which isn't that nice but that's why we try to do selector-specific stuff).

So what about having an API that for a given symbol name gives us all the alternative names we should put into this search index (and optionally a FunctionNameType so we can put it into the right group)? I guess C++ could also benefit from the same API as it would just return the right basenames then?

If we make the different m_*to_index maps in Symtab into something two nested maps with the form FunctionNameType -> name -> index, then that would also remove quite a bit of copy-pasted code in Symtab.cpp

[bulbazord]

In D103210#2784645, @teemperor wrote:

+1 to not add any language-specific terms to the Language API.

Looking over the code, it seems what the code is doing its:

• Going over the symbol names.
• Creating a search index from the symbol names that is optionally grouped via the FunctionNameType cases (and there is a 'eFunctionNameTypeSelector' which isn't that nice but that's why we try to do selector-specific stuff).

So what about having an API that for a given symbol name gives us all the alternative names we should put into this search index (and optionally a FunctionNameType so we can put it into the right group)? I guess C++ could also benefit from the same API as it would just return the right basenames then?

We have a method called Language::GetMethodNameVariants, that could be extended/modified to be more suitable. Unless you had something else in mind? Maybe something more specific to Symtab mapping like Language::GetFunctionNameSearchVariants.

If we make the different m_*to_index maps in Symtab into something two nested maps with the form FunctionNameType -> name -> index, then that would also remove quite a bit of copy-pasted code in Symtab.cpp

This makes sense to me, I'll also make this change in this patch :)

[teemperor]

Sorry for the delay!

We have a method called Language::GetMethodNameVariants, that could be extended/modified to be more suitable. Unless you had something else in mind? Maybe something more specific to Symtab mapping like Language::GetFunctionNameSearchVariants.

That actually seems like a great idea! Module::LookupInfo also seems quite close to the data structure we would need to return, so we could in theory return a list of that from the function instead.

This makes sense to me, I'll also make this change in this patch :)

Feel free to also put it up as a NFC patch. It seems in general like a good cleanup from what I can see, so that should be a quick review :)

[bulbazord]

In D103210#2795619, @teemperor wrote:

We have a method called Language::GetMethodNameVariants, that could be extended/modified to be more suitable. Unless you had something else in mind? Maybe something more specific to Symtab mapping like Language::GetFunctionNameSearchVariants.

That actually seems like a great idea! Module::LookupInfo also seems quite close to the data structure we would need to return, so we could in theory return a list of that from the function instead.

Awesome, I'll work on that next :)

This makes sense to me, I'll also make this change in this patch :)

Feel free to also put it up as a NFC patch. It seems in general like a good cleanup from what I can see, so that should be a quick review :)

Just uploaded: D103652

[bulbazord]

Not going with this change

lldb/include/lldb/Target/Language.h

 #ifndef LLDB_TARGET_LANGUAGE_H
 #define LLDB_TARGET_LANGUAGE_H
 
 #include <functional>
 #include <memory>
 #include <set>
 #include <vector>
 
 #include "lldb/Core/Highlighter.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/DataFormatters/FormatClasses.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/Symbol/TypeSystem.h"
 #include "lldb/lldb-private.h"
 #include "lldb/lldb-public.h"
 
 namespace lldb_private {
 
 class Language : public PluginInterface {
 public:
+  class MethodNameInfo {
+  public:
+    virtual llvm::Optional<ConstString> GetSelector() = 0;
+    virtual ~MethodNameInfo() = default;
+  };
+
   class TypeScavenger {
   public:
     class Result {
     public:
       virtual bool IsValid() = 0;
 
       virtual bool DumpToStream(Stream &stream,
                                 bool print_help_if_available) = 0;
public:
 
   virtual HardcodedFormatters::HardcodedSyntheticFinder
   GetHardcodedSynthetics();
 
   virtual std::vector<ConstString>
   GetPossibleFormattersMatches(ValueObject &valobj,
                                lldb::DynamicValueType use_dynamic);
 
+  virtual std::unique_ptr<MethodNameInfo>
+  GetMethodNameInfo(llvm::StringRef name);
+
   virtual std::unique_ptr<TypeScavenger> GetTypeScavenger();
 
   virtual const char *GetLanguageSpecificTypeLookupHelp();
 
   // If a language can have more than one possible name for a method, this
   // function can be used to enumerate them. This is useful when doing name
   // lookups.
   virtual std::vector<ConstString>

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

 //===-- ObjCLanguage.h ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_LANGUAGE_OBJC_OBJCLANGUAGE_H
 #define LLDB_SOURCE_PLUGINS_LANGUAGE_OBJC_OBJCLANGUAGE_H
 
 #include <cstring>
 #include <vector>
 
 #include "Plugins/Language/ClangCommon/ClangHighlighter.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
 
 class ObjCLanguage : public Language {
   ClangHighlighter m_highlighter;
protected:
     ConstString
         m_class_category;   // Class with category: "NSString(my_additions)"
     ConstString m_category; // Category:    "my_additions"
     ConstString m_selector; // Selector:    "myStringWithCString:"
     Type m_type;
     bool m_category_is_valid;
   };
 
+  class ObjCMethodNameInfo : public Language::MethodNameInfo {
+    ObjCLanguage::MethodName m_method_name;
+
+  public:
+    ObjCMethodNameInfo(llvm::StringRef name) : m_method_name(name, true) {}
+
+    bool IsValid() { return m_method_name.IsValid(true); }
+    llvm::Optional<ConstString> GetSelector() override {
+      return m_method_name.GetSelector();
+    }
+  };
+
   ObjCLanguage() = default;
 
   ~ObjCLanguage() override = default;
 
   lldb::LanguageType GetLanguageType() const override {
     return lldb::eLanguageTypeObjC;
   }
 
public:
   GetMethodNameVariants(ConstString method_name) const override;
 
   lldb::TypeCategoryImplSP GetFormatters() override;
 
   std::vector<ConstString>
   GetPossibleFormattersMatches(ValueObject &valobj,
                                lldb::DynamicValueType use_dynamic) override;
 
+  std::unique_ptr<Language::MethodNameInfo>
+  GetMethodNameInfo(llvm::StringRef name) override;
+
   std::unique_ptr<TypeScavenger> GetTypeScavenger() override;
 
   bool GetFormatterPrefixSuffix(ValueObject &valobj, ConstString type_hint,
                                 std::string &prefix,
                                 std::string &suffix) override;
 
   bool IsNilReference(ValueObject &valobj) override;
 

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

do {
       if (ConstString name = objc_class_sp->GetClassName())
         result.push_back(name);
     } while (false);
   }
 
   return result;
 }
 
+std::unique_ptr<Language::MethodNameInfo>
+ObjCLanguage::GetMethodNameInfo(llvm::StringRef name) {
+  if (auto ret = std::make_unique<ObjCMethodNameInfo>(name)) {
+    if (ret->IsValid()) {
+      return ret;
+    }
+  }
+  return nullptr;
+}
+
 std::unique_ptr<Language::TypeScavenger> ObjCLanguage::GetTypeScavenger() {
   class ObjCScavengerResult : public Language::TypeScavenger::Result {
   public:
     ObjCScavengerResult(CompilerType type)
         : Language::TypeScavenger::Result(), m_compiler_type(type) {}
 
     bool IsValid() override { return m_compiler_type.IsValid(); }
 

lldb/source/Symbol/Symtab.cpp

 //===-- Symtab.cpp --------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <map>
 #include <set>
 
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
-
 #include "lldb/Core/Module.h"
 #include "lldb/Core/RichManglingContext.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/Symtab.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
 
 #include "llvm/ADT/StringRef.h"
 
 using namespace lldb;
 using namespace lldb_private;
for (uint32_t value = 0; value < num_symbols; ++value) {
           // the annotations.
           name = ConstString(
               m_objfile->StripLinkerSymbolAnnotations(name.GetStringRef()));
           m_name_to_index.Append(name, value);
         }
 
         // If the demangled name turns out to be an ObjC name, and is a category
         // name, add the version without categories to the index too.
-        ObjCLanguage::MethodName objc_method(name.GetStringRef(), true);
-        if (objc_method.IsValid(true)) {
-          m_selector_to_index.Append(objc_method.GetSelector(), value);
-
-          if (ConstString objc_method_no_category =
-                  objc_method.GetFullNameWithoutCategory(true))
-            m_name_to_index.Append(objc_method_no_category, value);
+        if (auto *objc_lang = Language::FindPlugin(lldb::eLanguageTypeObjC)) {
+          if (std::unique_ptr<Language::MethodNameInfo> method_name_info =
+                  objc_lang->GetMethodNameInfo(name.GetStringRef())) {
+            m_selector_to_index.Append(
+                method_name_info->GetSelector().getValue(), value);
+            for (ConstString objc_method_variant :
+                 objc_lang->GetMethodNameVariants(name)) {
+              m_name_to_index.Append(objc_method_variant, value);
+            }
+          }
         }
       }
     }
 
     for (const auto &record : backlog) {
       RegisterBacklogEntry(record.first, record.second, class_contexts);
     }
 

lldb/source/Target/Language.cpp

 //===-- Language.cpp ------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <functional>
 #include <map>
 #include <mutex>
 
 #include "lldb/Target/Language.h"
 
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/Support/Threading.h"

 LanguageSet Language::GetLanguagesSupportingTypeSystemsForExpressions() {
   return PluginManager::GetAllTypeSystemSupportedLanguagesForExpressions();
 }
 
 LanguageSet Language::GetLanguagesSupportingREPLs() {
   return PluginManager::GetREPLAllTypeSystemSupportedLanguages();
 }
 
+std::unique_ptr<Language::MethodNameInfo>
+Language::GetMethodNameInfo(llvm::StringRef name) {
+  return nullptr;
+}
+
 std::unique_ptr<Language::TypeScavenger> Language::GetTypeScavenger() {
   return nullptr;
 }
 
 const char *Language::GetLanguageSpecificTypeLookupHelp() { return nullptr; }
 
 size_t Language::TypeScavenger::Find(ExecutionContextScope *exe_scope,
                                      const char *key, ResultSet &results,