[lldb] Delete IRExecutionUnit::CollectCandidateCPlusPlusNames

[bulbazord]

Summary

My goal is to remove lldbExpression's dependence on
lldbPluginCPlusPlusLanguage. I initially deleted this code in
to see what tests failed so that I could better understand how to
trigger this code path. No tests failed and I was unable to figure out
how to trigger the path manually, so I would like to delete it.

[teemperor]

I'm very supportive of the idea that we delete everything that isn't tested :) (and I'm only partly joking here). However removing this should cause a bunch of test failures on Linux as it effectively disables D70721 (the test added in that patch was just a unit test that is preserved with this patch, but the bigger problem describes there will break a few tests calling constructors on Linux).

I believe the usual 'iterate over all language plugins' approach would again work here? We can guess from the mangled symbol what language we deal with (and in the worst case we could encode that in the IRExecutionUnit), so the plugins would just deliver alternative names. GetAlternativeSymbolNames(ConstString symbol) or something like that maybe?

[bulbazord]

In D108395#2957194, @teemperor wrote:

I'm very supportive of the idea that we delete everything that isn't tested :) (and I'm only partly joking here).

I can sympathize, I'm only partly serious with this change! :D

However removing this should cause a bunch of test failures on Linux as it effectively disables D70721 (the test added in that patch was just a unit test that is preserved with this patch, but the bigger problem describes there will break a few tests calling constructors on Linux).

It doesn't cause test any test failures on my Linux machine unfortunately. I don't think this codepath is currently exercised by any tests. Certainly the FindAlternateFunctionManglings test in CPlusPlusLanguageTest.cpp also uses CPlusPlusLanguage::FindAlternateFunctionManglings, but that's not the same thing as testing IRExecutionUnit::CollectCandidateCPlusPlusNames being tested as a part of IRExecutionUnit::FindSymbol.

I believe the usual 'iterate over all language plugins' approach would again work here? We can guess from the mangled symbol what language we deal with (and in the worst case we could encode that in the IRExecutionUnit), so the plugins would just deliver alternative names. GetAlternativeSymbolNames(ConstString symbol) or something like that maybe?

I definitely think that the usual "iterate over all language plugins" approach would work here if we don't delete this (which it seems like we shouldn't). I mostly uploaded this change to see if anybody knew how to trigger this codepath. Before refactoring I'd like to write a test for this codepath, though I'm still not entirely sure how to do that yet.

[bulbazord]

Embarassingly I found out my test environment was not working correctly, this does indeed fail some tests. I will upload a patch to actually refactor it :)

lldb/source/Expression/CMakeLists.txt

add_lldb_library(lldbExpression
 
   LINK_LIBS
     lldbCore
     lldbHost
     lldbInterpreter
     lldbSymbol
     lldbTarget
     lldbUtility
-    lldbPluginCPlusPlusLanguage
     lldbPluginObjectFileJIT
 
   LINK_COMPONENTS
     Core
     ExecutionEngine
     Support
   )

lldb/source/Expression/IRExecutionUnit.cpp

 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 
-#include "lldb/../../source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "lldb/../../source/Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 
 using namespace lldb_private;
 
 IRExecutionUnit::IRExecutionUnit(std::unique_ptr<llvm::LLVMContext> &context_up,
                                  std::unique_ptr<llvm::Module> &module_up,
                                  ConstString &name,
                                  const lldb::TargetSP &target_sp,
lldb::ProcessSP process_sp =
         m_parent.GetBestExecutionContextScope()->CalculateProcess();
 
     m_parent.CommitOneAllocation(process_sp, err, m_parent.m_records.back());
   }
 
   return return_value;
 }
 
-static ConstString FindBestAlternateMangledName(ConstString demangled,
-                                                const SymbolContext &sym_ctx) {
-  CPlusPlusLanguage::MethodName cpp_name(demangled);
-  std::string scope_qualified_name = cpp_name.GetScopeQualifiedName();
-
-  if (!scope_qualified_name.size())
-    return ConstString();
-
-  if (!sym_ctx.module_sp)
-    return ConstString();
-
-  lldb_private::SymbolFile *sym_file = sym_ctx.module_sp->GetSymbolFile();
-  if (!sym_file)
-    return ConstString();
-
-  std::vector<ConstString> alternates;
-  sym_file->GetMangledNamesForFunction(scope_qualified_name, alternates);
-
-  std::vector<ConstString> param_and_qual_matches;
-  std::vector<ConstString> param_matches;
-  for (size_t i = 0; i < alternates.size(); i++) {
-    ConstString alternate_mangled_name = alternates[i];
-    Mangled mangled(alternate_mangled_name);
-    ConstString demangled = mangled.GetDemangledName();
-
-    CPlusPlusLanguage::MethodName alternate_cpp_name(demangled);
-    if (!cpp_name.IsValid())
-      continue;
-
-    if (alternate_cpp_name.GetArguments() == cpp_name.GetArguments()) {
-      if (alternate_cpp_name.GetQualifiers() == cpp_name.GetQualifiers())
-        param_and_qual_matches.push_back(alternate_mangled_name);
-      else
-        param_matches.push_back(alternate_mangled_name);
-    }
-  }
-
-  if (param_and_qual_matches.size())
-    return param_and_qual_matches[0]; // It is assumed that there will be only
-                                      // one!
-  else if (param_matches.size())
-    return param_matches[0]; // Return one of them as a best match
-  else
-    return ConstString();
-}
-
 struct IRExecutionUnit::SearchSpec {
   ConstString name;
   lldb::FunctionNameType mask;
 
   SearchSpec(ConstString n,
              lldb::FunctionNameType m = lldb::eFunctionNameTypeFull)
       : name(n), mask(m) {}
 };
 
 void IRExecutionUnit::CollectCandidateCNames(
     std::vector<IRExecutionUnit::SearchSpec> &C_specs,
     ConstString name) {
   if (m_strip_underscore && name.AsCString()[0] == '_')
     C_specs.insert(C_specs.begin(), ConstString(&name.AsCString()[1]));
   C_specs.push_back(SearchSpec(name));
 }
 
-void IRExecutionUnit::CollectCandidateCPlusPlusNames(
-    std::vector<IRExecutionUnit::SearchSpec> &CPP_specs,
-    const std::vector<SearchSpec> &C_specs, const SymbolContext &sc) {
-  for (const SearchSpec &C_spec : C_specs) {
-    ConstString name = C_spec.name;
-
-    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) {
-          CPP_specs.push_back(best_alternate_mangled_name);
-        }
-      }
-    }
-
-    std::set<ConstString> alternates;
-    CPlusPlusLanguage::FindAlternateFunctionManglings(name, alternates);
-    CPP_specs.insert(CPP_specs.end(), alternates.begin(), alternates.end());
-  }
-}
-
 void IRExecutionUnit::CollectFallbackNames(
     std::vector<SearchSpec> &fallback_specs,
     const std::vector<SearchSpec> &C_specs) {
   // 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 SearchSpec &C_spec : C_specs) {
     ConstString name = C_spec.name;
 
-    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)
lldb::addr_t IRExecutionUnit::FindInUserDefinedSymbols(
   }
 
   return LLDB_INVALID_ADDRESS;
 }
 
 lldb::addr_t
 IRExecutionUnit::FindSymbol(lldb_private::ConstString name, bool &missing_weak) {
   std::vector<SearchSpec> candidate_C_names;
-  std::vector<SearchSpec> candidate_CPlusPlus_names;
 
   CollectCandidateCNames(candidate_C_names, name);
   
   lldb::addr_t ret = FindInSymbols(candidate_C_names, m_sym_ctx, missing_weak);
   if (ret != LLDB_INVALID_ADDRESS)
     return ret;
   
   // If we find the symbol in runtimes or user defined symbols it can't be 
   // a missing weak symbol.
   missing_weak = false;
   ret = FindInRuntimes(candidate_C_names, m_sym_ctx);
   if (ret != LLDB_INVALID_ADDRESS)
     return ret;
 
   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<SearchSpec> candidate_fallback_names;
 
   CollectFallbackNames(candidate_fallback_names, candidate_C_names);
   ret = FindInSymbols(candidate_fallback_names, m_sym_ctx, missing_weak);
 
   return ret;
 }