[lldb] Remove IRExecutionUnit::CollectFallbackNames

[bulbazord]

Summary

The work that IRExecutionUnit::CollectFallbackNames is basically the
work that CPlusPlusLanguage::GetDemangledFunctionNameWithoutArguments
does already. It's also (at time or writing) specific to C++, so it can
be folded into IRExecutionUnit::CollectCandidateCPlusPlusNames.

[clayborg]

Looks fine to me. I am assuming the full test suite passes with this change in place?

lldb/source/Expression/IRExecutionUnit.cpp

717

Seems like we should be passing in the "Mangled" object into "FindBestAlternateMangledName" in case it has extra information. For example constructors and destructors have many different types (in charge, not in charge, etc).

[bulbazord]

In D109928#3005085, @clayborg wrote:

Looks fine to me. I am assuming the full test suite passes with this change in place?

It didn't introduce any new test failures on any of my machines.

lldb/source/Expression/IRExecutionUnit.cpp

717

Yes, I agree. I plan on tackling that function next.

[shafik]

I think this looks fine, just verify that this in a C++ case an expression with an extern "C" function call works fine e.g.

extern "C" {
  int g() { return 10;}
}

int main() {
   return g();  // break here and run expr g()
}

I discovered the other day that we mangle extern "C" functions in C++ mode and I have a fix I wanted to run by everyone but I have not had a chance to do it yet.

So this case get resolved in CollectFallbackNames(...).

[bulbazord]

In D109928#3013627, @shafik wrote:

I think this looks fine, just verify that this in a C++ case an expression with an extern "C" function call works fine e.g.

extern "C" {
  int g() { return 10;}
}

int main() {
   return g();  // break here and run expr g()
}

I discovered the other day that we mangle extern "C" functions in C++ mode and I have a fix I wanted to run by everyone but I have not had a chance to do it yet.

So this case get resolved in CollectFallbackNames(...).

I believe this is already covered by TestExternCSymbols.py in test/API/cpp/extern_c/. CollectFallbackNames did correctly resolve this case, so I made sure this change didn't break that test.

Anything you think I should change or is this good to go?

lldb/include/lldb/Expression/IRExecutionUnit.h

private:
 
   void CollectCandidateCNames(std::vector<ConstString> &C_names,
                               ConstString name);
 
   void CollectCandidateCPlusPlusNames(std::vector<ConstString> &CPP_names,
                                       const std::vector<ConstString> &C_names,
                                       const SymbolContext &sc);
 
-  void CollectFallbackNames(std::vector<ConstString> &fallback_names,
-                            const std::vector<ConstString> &C_names);
-
   lldb::addr_t FindInSymbols(const std::vector<ConstString> &names,
                              const lldb_private::SymbolContext &sc,
                              bool &symbol_was_missing_weak);
 
   lldb::addr_t FindInRuntimes(const std::vector<ConstString> &names,
                               const lldb_private::SymbolContext &sc);
 
   lldb::addr_t FindInUserDefinedSymbols(const std::vector<ConstString> &names,

lldb/source/Expression/IRExecutionUnit.cpp

 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DiagnosticHandler.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Expression/IRExecutionUnit.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
void IRExecutionUnit::CollectCandidateCNames(std::vector<ConstString> &C_names,
   if (m_strip_underscore && name.AsCString()[0] == '_')
     C_names.insert(C_names.begin(), ConstString(&name.AsCString()[1]));
   C_names.push_back(name);
 }
 
 void IRExecutionUnit::CollectCandidateCPlusPlusNames(
     std::vector<ConstString> &CPP_names,
     const std::vector<ConstString> &C_names, const SymbolContext &sc) {
+  if (auto *cpp_lang = Language::FindPlugin(lldb::eLanguageTypeC_plus_plus)) {
     for (const ConstString &name : C_names) {
-    if (CPlusPlusLanguage::IsCPPMangledName(name.GetCString())) {
       Mangled mangled(name);
-      ConstString demangled = mangled.GetDemangledName();
-
-      if (demangled) {
-        ConstString best_alternate_mangled_name =
-            FindBestAlternateMangledName(demangled, sc);
-
-        if (best_alternate_mangled_name) {
+      if (cpp_lang->SymbolNameFitsToLanguage(mangled)) {
+        if (ConstString demangled = mangled.GetDemangledName()) {
+          if (ConstString best_alternate_mangled_name =
+                  FindBestAlternateMangledName(demangled, sc))
             CPP_names.push_back(best_alternate_mangled_name);
         }
       }
-    }
 
-    if (auto *cpp_lang = Language::FindPlugin(lldb::eLanguageTypeC_plus_plus)) {
       std::vector<ConstString> alternates =
           cpp_lang->GenerateAlternateFunctionManglings(name);
       CPP_names.insert(CPP_names.end(), alternates.begin(), alternates.end());
-    }
-  }
-}
 
-void IRExecutionUnit::CollectFallbackNames(
-    std::vector<ConstString> &fallback_names,
-    const std::vector<ConstString> &C_names) {
-  // As a last-ditch fallback, try the base name for C++ names.  It's terrible,
-  // but the DWARF doesn't always encode "extern C" correctly.
-
-  for (const ConstString &name : C_names) {
-    if (!CPlusPlusLanguage::IsCPPMangledName(name.GetCString()))
-      continue;
-
-    Mangled mangled_name(name);
-    ConstString demangled_name = mangled_name.GetDemangledName();
-    if (demangled_name.IsEmpty())
-      continue;
-
-    const char *demangled_cstr = demangled_name.AsCString();
-    const char *lparen_loc = strchr(demangled_cstr, '(');
-    if (!lparen_loc)
-      continue;
-
-    llvm::StringRef base_name(demangled_cstr, lparen_loc - demangled_cstr);
-    fallback_names.push_back(ConstString(base_name));
+      // As a last-ditch fallback, try the base name for C++ names.  It's
+      // terrible, but the DWARF doesn't always encode "extern C" correctly.
+      ConstString basename =
+          cpp_lang->GetDemangledFunctionNameWithoutArguments(mangled);
+      CPP_names.push_back(basename);
+    }
   }
 }
 
 class LoadAddressResolver {
 public:
   LoadAddressResolver(Target *target, bool &symbol_was_missing_weak)
       : m_target(target), m_symbol_was_missing_weak(symbol_was_missing_weak) {}
 
lldb::addr_t IRExecutionUnit::FindSymbol(lldb_private::ConstString name,
 
   ret = FindInUserDefinedSymbols(candidate_C_names, m_sym_ctx);
   if (ret != LLDB_INVALID_ADDRESS)
     return ret;
 
   CollectCandidateCPlusPlusNames(candidate_CPlusPlus_names, candidate_C_names,
                                  m_sym_ctx);
   ret = FindInSymbols(candidate_CPlusPlus_names, m_sym_ctx, missing_weak);
-  if (ret != LLDB_INVALID_ADDRESS)
-    return ret;
-
-  std::vector<ConstString> candidate_fallback_names;
-
-  CollectFallbackNames(candidate_fallback_names, candidate_C_names);
-  ret = FindInSymbols(candidate_fallback_names, m_sym_ctx, missing_weak);
-
   return ret;
 }
 
 void IRExecutionUnit::GetStaticInitializers(
     std::vector<lldb::addr_t> &static_initializers) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   llvm::GlobalVariable *global_ctors =