[Breakpoint] Make breakpoint language agnostic

[xiaobai]

Summary

Breakpoint shouldn't need to depend on any specific details from a
programming language. Currently the only language-specific detail it takes
advantage of are the different qualified names an objective-c method name might
have when adding a name lookup. This is reasonably generalizable.

The current method name I introduced is "GetVariantMethodNames", which I'm not
particularly tied to. If you have a better suggestion, please do let me know.

[JDevlieghere]

include/lldb/Target/Language.h

197 ↗

(On Diff #198878)

GetMethodNameVariants?

source/Breakpoint/BreakpointResolverName.cpp

237 ↗

(On Diff #198878)

You could write this as

if (Language *lang = Language::FindPlugin(m_language)) {
  add_variant_funcs(lang);
} else {
  // Most likely m_language is eLanguageTypeUnknown. We check each language for
  // possible variants or more qualified names and create lookups for those as
  // well.
  Language::ForEach(add_variant_funcs);
}

[jingham]

That looks fine, thanks for untangling this.

I do like Jonas' suggestion better. GetVariantMethodNames sounds like "VariantMethods" are a specific thing and you are trying to find their names, whereas this is just variants of the method name. GetMethodNameVariants makes that clearer.

I was going to quibble a bit about "Method" since this can also happen for regular functions. This isn't entirely theoretical, macOS used to used to have different variants of some of the standard library routines based on the whether the program was running in strict posix mode or had xmm registers, etc. Then it had the kernel map in the little block of function pointers and used assembly tricks so the function name would call through this block. In that case since lldb didn't know which instance of the function was going to be used we had to look for these variants and break on all of them. That code is gone now - macOS switched to dynamic resolver functions instead. But this would have been another use for these "Variants".

OTOH, I can't think of a good name for "callable entities" that isn't awful, so I'm fine with Method...

[xiaobai]

Thanks for the feedback!

I don't quite feel that "Method" is a great descriptor here either, but I think that it's probably fine for now until we can think of a better name. I'll update this when I get the chance.

[xiaobai]

Rework function
GetVariantMethodNames -> GetMethodNameVariants

[labath]

include/lldb/Target/Language.h

198 ↗

(On Diff #198911)

Return the vector by value ?

[xiaobai]

• Fix minor bug
• Return vector by value instead of passing in one by parameter.

lldb/trunk/include/lldb/Target/Language.h

public:
   virtual lldb_private::formatters::StringPrinter::EscapingHelper
       GetStringPrinterEscapingHelper(
           lldb_private::formatters::StringPrinter::GetPrintableElementType);
 
   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>
+  GetMethodNameVariants(ConstString method_name) const {
+    return std::vector<ConstString>();
+  };
+
   // if an individual data formatter can apply to several types and cross a
   // language boundary it makes sense for individual languages to want to
   // customize the printing of values of that type by appending proper
   // prefix/suffix information in language-specific ways
   virtual bool GetFormatterPrefixSuffix(ValueObject &valobj,
                                         ConstString type_hint,
                                         std::string &prefix,
                                         std::string &suffix);

lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp

 //===-- BreakpointResolverName.cpp ------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Breakpoint/BreakpointResolverName.h"
 
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Architecture.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/Block.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 BreakpointResolverName::BreakpointResolverName(
     Breakpoint *bkpt, const char *name_cstr, FunctionNameType name_type_mask,
StructuredData::ObjectSP BreakpointResolverName::SerializeToStructuredData() {
   options_dict_sp->AddBooleanItem(GetKey(OptionNames::SkipPrologue),
                                   m_skip_prologue);
 
   return WrapOptionsDict(options_dict_sp);
 }
 
 void BreakpointResolverName::AddNameLookup(ConstString name,
                                            FunctionNameType name_type_mask) {
-  ObjCLanguage::MethodName objc_method(name.GetCString(), false);
-  if (objc_method.IsValid(false)) {
-    std::vector<ConstString> objc_names;
-    objc_method.GetFullNames(objc_names, true);
-    for (ConstString objc_name : objc_names) {
-      Module::LookupInfo lookup;
-      lookup.SetName(name);
-      lookup.SetLookupName(objc_name);
-      lookup.SetNameTypeMask(eFunctionNameTypeFull);
-      m_lookups.push_back(lookup);
+
+  Module::LookupInfo lookup(name, name_type_mask, m_language);
+  m_lookups.emplace_back(lookup);
+
+  auto add_variant_funcs = [&](Language *lang) {
+    for (ConstString variant_name : lang->GetMethodNameVariants(name)) {
+      Module::LookupInfo variant_lookup(name, name_type_mask,
+                                        lang->GetLanguageType());
+      variant_lookup.SetLookupName(variant_name);
+      m_lookups.emplace_back(variant_lookup);
     }
+    return true;
+  };
+
+  if (Language *lang = Language::FindPlugin(m_language)) {
+    add_variant_funcs(lang);
   } else {
-    Module::LookupInfo lookup(name, name_type_mask, m_language);
-    m_lookups.push_back(lookup);
+    // Most likely m_language is eLanguageTypeUnknown. We check each language for
+    // possible variants or more qualified names and create lookups for those as
+    // well.
+    Language::ForEach(add_variant_funcs);
   }
 }
 
 // FIXME: Right now we look at the module level, and call the module's
 // "FindFunctions".
 // Greg says he will add function tables, maybe at the CompileUnit level to
 // accelerate function lookup.  At that point, we should switch the depth to
 // CompileUnit, and look in these tables.

lldb/trunk/source/Breakpoint/CMakeLists.txt

add_lldb_library(lldbBreakpoint
 
   LINK_LIBS
     lldbCore
     lldbExpression
     lldbInterpreter
     lldbSymbol
     lldbTarget
     lldbUtility
-    lldbPluginObjCLanguage
 
   LINK_COMPONENTS
     Support
   )

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

public:
     ConstString GetClassName();
 
     ConstString GetClassNameWithCategory();
 
     ConstString GetCategory();
 
     ConstString GetSelector();
 
-    // Get all possible names for a method. Examples:
-    // If name is "+[NSString(my_additions) myStringWithCString:]"
-    //  names[0] => "+[NSString(my_additions) myStringWithCString:]"
-    //  names[1] => "+[NSString myStringWithCString:]"
-    // If name is specified without the leading '+' or '-' like
-    // "[NSString(my_additions) myStringWithCString:]"
-    //  names[0] => "+[NSString(my_additions) myStringWithCString:]"
-    //  names[1] => "-[NSString(my_additions) myStringWithCString:]"
-    //  names[2] => "+[NSString myStringWithCString:]"
-    //  names[3] => "-[NSString myStringWithCString:]"
-    size_t GetFullNames(std::vector<ConstString> &names, bool append);
-
   protected:
     ConstString
         m_full; // Full name:   "+[NSString(my_additions) myStringWithCString:]"
     ConstString m_class; // Class name:  "NSString"
     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;
   };
 
   ObjCLanguage() = default;
 
   ~ObjCLanguage() override = default;
 
   lldb::LanguageType GetLanguageType() const override {
     return lldb::eLanguageTypeObjC;
   }
 
+  // Get all possible names for a method. Examples:
+  // If method_name is "+[NSString(my_additions) myStringWithCString:]"
+  //   variant_names[0] => "+[NSString myStringWithCString:]"
+  // If name is specified without the leading '+' or '-' like
+  // "[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[0] => "+[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[1] => "-[NSString(my_additions) myStringWithCString:]"
+  //  variant_names[2] => "+[NSString myStringWithCString:]"
+  //  variant_names[3] => "-[NSString myStringWithCString:]"
+  std::vector<ConstString>
+  GetMethodNameVariants(ConstString method_name) const override;
+
   lldb::TypeCategoryImplSP GetFormatters() override;
 
   std::vector<ConstString>
   GetPossibleFormattersMatches(ValueObject &valobj,
                                lldb::DynamicValueType use_dynamic) override;
 
   std::unique_ptr<TypeScavenger> GetTypeScavenger() override;
 

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

if (IsValid(false)) {
     if (!empty_if_no_category) {
       // Just return the full name since it doesn't have a category
       return GetFullName();
     }
   }
   return ConstString();
 }
 
-size_t ObjCLanguage::MethodName::GetFullNames(std::vector<ConstString> &names,
-                                              bool append) {
-  if (!append)
-    names.clear();
-  if (IsValid(false)) {
-    StreamString strm;
-    const bool is_class_method = m_type == eTypeClassMethod;
-    const bool is_instance_method = m_type == eTypeInstanceMethod;
-    ConstString category = GetCategory();
-    if (is_class_method || is_instance_method) {
-      names.push_back(m_full);
-      if (category) {
-        strm.Printf("%c[%s %s]", is_class_method ? '+' : '-',
-                    GetClassName().GetCString(), GetSelector().GetCString());
-        names.emplace_back(strm.GetString());
-      }
-    } else {
-      ConstString class_name = GetClassName();
-      ConstString selector = GetSelector();
-      strm.Printf("+[%s %s]", class_name.GetCString(), selector.GetCString());
-      names.emplace_back(strm.GetString());
-      strm.Clear();
-      strm.Printf("-[%s %s]", class_name.GetCString(), selector.GetCString());
-      names.emplace_back(strm.GetString());
-      strm.Clear();
-      if (category) {
-        strm.Printf("+[%s(%s) %s]", class_name.GetCString(),
-                    category.GetCString(), selector.GetCString());
-        names.emplace_back(strm.GetString());
-        strm.Clear();
-        strm.Printf("-[%s(%s) %s]", class_name.GetCString(),
-                    category.GetCString(), selector.GetCString());
-        names.emplace_back(strm.GetString());
-      }
+std::vector<ConstString>
+ObjCLanguage::GetMethodNameVariants(ConstString method_name) const {
+  std::vector<ConstString> variant_names;
+  ObjCLanguage::MethodName objc_method(method_name.GetCString(), false);
+  if (!objc_method.IsValid(false)) {
+    return variant_names;
+
+  const bool is_class_method =
+      objc_method.GetType() == MethodName::eTypeClassMethod;
+  const bool is_instance_method =
+      objc_method.GetType() == MethodName::eTypeInstanceMethod;
+  ConstString name_sans_category =
+      objc_method.GetFullNameWithoutCategory(/*empty_if_no_category*/ true);
+
+  if (is_class_method || is_instance_method) {
+    if (name_sans_category)
+      variant_names.emplace_back(name_sans_category);
+    StreamString strm;
+
+    strm.Printf("+%s", objc_method.GetFullName().GetCString());
+    variant_names.emplace_back(strm.GetString());
+
+    strm.Printf("-%s", objc_method.GetFullName().GetCString());
+    variant_names.emplace_back(strm.GetString());
+
+    if (name_sans_category) {
+      strm.Printf("+%s", name_sans_category.GetCString());
+      variant_names.emplace_back(strm.GetString());
+
+      strm.Printf("-%s", name_sans_category.GetCString());
+      variant_names.emplace_back(strm.GetString());
     }
   }
-  return names.size();
+
+  return variant_names;
 }
 
 static void LoadObjCFormatters(TypeCategoryImplSP objc_category_sp) {
   if (!objc_category_sp)
     return;
 
   TypeSummaryImpl::Flags objc_flags;
   objc_flags.SetCascades(false)