[lldb] Improve RichManglingContext ergonomics (NFC)

[JDevlieghere]

Summary

Have the different ::Parse.* methods return the demangled string directly instead of having to go through ::GetBufferRef.

[clayborg]

Nice, thanks! One of your patches will need to be rebased depending on which one goes in first (this one or https://reviews.llvm.org/D118814).

lldb/include/lldb/Core/RichManglingContext.h

public:
   /// Use the legacy language parser implementation to obtain rich mangling
   /// information from the given demangled name.
   bool FromCxxMethodName(ConstString demangled);
 
   /// If this symbol describes a constructor or destructor.
   bool IsCtorOrDtor() const;
 
   /// Get the base name of a function. This doesn't include trailing template
-  /// arguments, ie "a::b<int>" gives "b". The result will overwrite the
-  /// internal buffer. It can be obtained via GetBufferRef().
-  void ParseFunctionBaseName();
+  /// arguments, ie "a::b<int>" gives "b".
+  llvm::StringRef ParseFunctionBaseName();
 
   /// Get the context name for a function. For "a::b::c", this function returns
-  /// "a::b". The result will overwrite the internal buffer. It can be obtained
-  /// via GetBufferRef().
-  void ParseFunctionDeclContextName();
-
-  /// Get the entire demangled name. The result will overwrite the internal
-  /// buffer. It can be obtained via GetBufferRef().
-  void ParseFullName();
-
-  /// Obtain a StringRef to the internal buffer that holds the result of the
-  /// most recent ParseXy() operation. The next ParseXy() call invalidates it.
-  llvm::StringRef GetBufferRef() const {
-    assert(m_provider != None && "Initialize a provider first");
-    return m_buffer;
-  }
+  /// "a::b".
+  llvm::StringRef ParseFunctionDeclContextName();
+
+  /// Get the entire demangled name.
+  llvm::StringRef ParseFullName();
 
 private:
   enum InfoProvider { None, ItaniumPartialDemangler, PluginCxxLanguage };
 
   /// Selects the rich mangling info provider.
   InfoProvider m_provider = None;
 
-  /// Reference to the buffer used for results of ParseXy() operations.
-  llvm::StringRef m_buffer;
-
   /// Members for ItaniumPartialDemangler
   llvm::ItaniumPartialDemangler m_ipd;
   /// Note: m_ipd_buf is a raw pointer due to being resized by realloc via
   /// ItaniumPartialDemangler. It should be managed with malloc/free, not
   /// new/delete.
   char *m_ipd_buf;
   size_t m_ipd_buf_size = 2048;
 
   /// Members for PluginCxxLanguage
   /// Cannot forward declare inner class CPlusPlusLanguage::MethodName. The
   /// respective header is in Plugins and including it from here causes cyclic
   /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
   llvm::Any m_cxx_method_parser;
 
   /// Clean up memory when using PluginCxxLanguage
   void ResetCxxMethodParser();
 
   /// Clean up memory and set a new info provider for this instance.
   void ResetProvider(InfoProvider new_provider);
 
   /// Uniform handling of string buffers for ItaniumPartialDemangler.
-  void processIPDStrResult(char *ipd_res, size_t res_len);
+  llvm::StringRef processIPDStrResult(char *ipd_res, size_t res_len);
 
   /// Cast the given parser to the given type. Ideally we would have a type
   /// trait to deduce \a ParserT from a given InfoProvider, but unfortunately we
   /// can't access CPlusPlusLanguage::MethodName from within the header.
   template <class ParserT> static ParserT *get(llvm::Any parser) {
     assert(parser.hasValue());
     assert(llvm::any_isa<ParserT *>(parser));
     return llvm::any_cast<ParserT *>(parser);
   }
 };
 
 } // namespace lldb_private
 
 #endif

lldb/source/Core/Mangled.cpp

case eManglingSchemeNone:
     return false;
 
   case eManglingSchemeItanium:
     // We want the rich mangling info here, so we don't care whether or not
     // there is a demangled string in the pool already.
     if (context.FromItaniumName(m_mangled)) {
       // If we got an info, we have a name. Copy to string pool and connect the
       // counterparts to accelerate later access in GetDemangledName().
-      context.ParseFullName();
-      m_demangled.SetStringWithMangledCounterpart(context.GetBufferRef(),
+      m_demangled.SetStringWithMangledCounterpart(context.ParseFullName(),
                                                   m_mangled);
       return true;
     } else {
       m_demangled.SetCString("");
       return false;
     }
 
   case eManglingSchemeMSVC: {

lldb/source/Core/RichManglingContext.cpp

case PluginCxxLanguage: {
     return base_name.startswith("~");
   }
   case None:
     return false;
   }
   llvm_unreachable("Fully covered switch above!");
 }
 
-void RichManglingContext::processIPDStrResult(char *ipd_res, size_t res_size) {
+llvm::StringRef RichManglingContext::processIPDStrResult(char *ipd_res,
+                                                         size_t res_size) {
   // Error case: Clear the buffer.
   if (LLVM_UNLIKELY(ipd_res == nullptr)) {
     assert(res_size == m_ipd_buf_size &&
            "Failed IPD queries keep the original size in the N parameter");
 
     m_ipd_buf[0] = '\0';
-    m_buffer = llvm::StringRef(m_ipd_buf, 0);
-    return;
+    return llvm::StringRef(m_ipd_buf, 0);
   }
 
   // IPD's res_size includes null terminator.
   assert(ipd_res[res_size - 1] == '\0' &&
          "IPD returns null-terminated strings and we rely on that");
 
   // Update buffer/size on realloc.
   if (LLVM_UNLIKELY(ipd_res != m_ipd_buf || res_size > m_ipd_buf_size)) {
     m_ipd_buf = ipd_res;       // std::realloc freed or reused the old buffer.
     m_ipd_buf_size = res_size; // May actually be bigger, but we can't know.
 
     if (Log *log = GetLog(LLDBLog::Demangle))
       LLDB_LOG(log, "ItaniumPartialDemangler Realloc: new buffer size is {0}",
                m_ipd_buf_size);
   }
 
   // 99% case: Just remember the string length.
-  m_buffer = llvm::StringRef(m_ipd_buf, res_size - 1);
+  return llvm::StringRef(m_ipd_buf, res_size - 1);
 }
 
-void RichManglingContext::ParseFunctionBaseName() {
+llvm::StringRef RichManglingContext::ParseFunctionBaseName() {
   assert(m_provider != None && "Initialize a provider first");
   switch (m_provider) {
   case ItaniumPartialDemangler: {
     auto n = m_ipd_buf_size;
     auto buf = m_ipd.getFunctionBaseName(m_ipd_buf, &n);
-    processIPDStrResult(buf, n);
-    return;
+    return processIPDStrResult(buf, n);
   }
   case PluginCxxLanguage:
-    m_buffer =
-        get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)->GetBasename();
-    return;
+    return get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)
+        ->GetBasename();
   case None:
-    return;
+    return {};
   }
 }
 
-void RichManglingContext::ParseFunctionDeclContextName() {
+llvm::StringRef RichManglingContext::ParseFunctionDeclContextName() {
   assert(m_provider != None && "Initialize a provider first");
   switch (m_provider) {
   case ItaniumPartialDemangler: {
     auto n = m_ipd_buf_size;
     auto buf = m_ipd.getFunctionDeclContextName(m_ipd_buf, &n);
-    processIPDStrResult(buf, n);
-    return;
+    return processIPDStrResult(buf, n);
   }
   case PluginCxxLanguage:
-    m_buffer =
-        get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)->GetContext();
-    return;
+    return get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)
+        ->GetContext();
   case None:
-    return;
+    return {};
   }
 }
 
-void RichManglingContext::ParseFullName() {
+llvm::StringRef RichManglingContext::ParseFullName() {
   assert(m_provider != None && "Initialize a provider first");
   switch (m_provider) {
   case ItaniumPartialDemangler: {
     auto n = m_ipd_buf_size;
     auto buf = m_ipd.finishDemangle(m_ipd_buf, &n);
-    processIPDStrResult(buf, n);
-    return;
+    return processIPDStrResult(buf, n);
   }
   case PluginCxxLanguage:
-    m_buffer = get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)
-                   ->GetFullName()
-                   .GetStringRef();
-    return;
+    return get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)
   case None:
-    return;
+    return {};
   }
 }

lldb/source/Symbol/Symtab.cpp

void Symtab::InitNameIndexes() {
   }
 }
 
 void Symtab::RegisterMangledNameEntry(
     uint32_t value, std::set<const char *> &class_contexts,
     std::vector<std::pair<NameToIndexMap::Entry, const char *>> &backlog,
     RichManglingContext &rmc) {
   // Only register functions that have a base name.
-  rmc.ParseFunctionBaseName();
-  llvm::StringRef base_name = rmc.GetBufferRef();
+  llvm::StringRef base_name = rmc.ParseFunctionBaseName();
   if (base_name.empty())
     return;
 
   // The base name will be our entry's name.
   NameToIndexMap::Entry entry(ConstString(base_name), value);
-
-  rmc.ParseFunctionDeclContextName();
-  llvm::StringRef decl_context = rmc.GetBufferRef();
+  llvm::StringRef decl_context = rmc.ParseFunctionDeclContextName();
 
   // Register functions with no context.
   if (decl_context.empty()) {
     // This has to be a basename
     auto &basename_to_index =
         GetNameToSymbolIndexMap(lldb::eFunctionNameTypeBase);
     basename_to_index.Append(entry);
     // If there is no context (no namespaces or class scopes that come before

lldb/unittests/Core/RichManglingContextTest.cpp

 #include "gtest/gtest.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 TEST(RichManglingContextTest, Basic) {
   RichManglingContext RMC;
   ConstString mangled("_ZN3foo3barEv");
-  EXPECT_TRUE(RMC.FromItaniumName(mangled));
 
+  EXPECT_TRUE(RMC.FromItaniumName(mangled));
   EXPECT_FALSE(RMC.IsCtorOrDtor());
-
-  RMC.ParseFunctionDeclContextName();
-  EXPECT_EQ("foo", RMC.GetBufferRef());
-
-  RMC.ParseFunctionBaseName();
-  EXPECT_EQ("bar", RMC.GetBufferRef());
-
-  RMC.ParseFullName();
-  EXPECT_EQ("foo::bar()", RMC.GetBufferRef());
+  EXPECT_EQ("foo", RMC.ParseFunctionDeclContextName());
+  EXPECT_EQ("bar", RMC.ParseFunctionBaseName());
+  EXPECT_EQ("foo::bar()", RMC.ParseFullName());
 }
 
 TEST(RichManglingContextTest, FromCxxMethodName) {
   RichManglingContext ItaniumRMC;
   ConstString mangled("_ZN3foo3barEv");
   EXPECT_TRUE(ItaniumRMC.FromItaniumName(mangled));
 
   RichManglingContext CxxMethodRMC;
   ConstString demangled("foo::bar()");
   EXPECT_TRUE(CxxMethodRMC.FromCxxMethodName(demangled));
 
-  EXPECT_TRUE(ItaniumRMC.IsCtorOrDtor() == CxxMethodRMC.IsCtorOrDtor());
-
-  ItaniumRMC.ParseFunctionDeclContextName();
-  CxxMethodRMC.ParseFunctionDeclContextName();
-  EXPECT_TRUE(ItaniumRMC.GetBufferRef() == CxxMethodRMC.GetBufferRef());
-
-  ItaniumRMC.ParseFunctionBaseName();
-  CxxMethodRMC.ParseFunctionBaseName();
-  EXPECT_TRUE(ItaniumRMC.GetBufferRef() == CxxMethodRMC.GetBufferRef());
-
-  ItaniumRMC.ParseFullName();
-  CxxMethodRMC.ParseFullName();
-  EXPECT_TRUE(ItaniumRMC.GetBufferRef() == CxxMethodRMC.GetBufferRef());
+  EXPECT_EQ(ItaniumRMC.IsCtorOrDtor(), CxxMethodRMC.IsCtorOrDtor());
+  EXPECT_EQ(ItaniumRMC.ParseFunctionDeclContextName(),
+            CxxMethodRMC.ParseFunctionDeclContextName());
+  EXPECT_EQ(ItaniumRMC.ParseFunctionBaseName(),
+            CxxMethodRMC.ParseFunctionBaseName());
+  EXPECT_EQ(ItaniumRMC.ParseFullName(), CxxMethodRMC.ParseFullName());
 
   // Construct with a random name.
   {
     RichManglingContext CxxMethodRMC;
     EXPECT_TRUE(CxxMethodRMC.FromCxxMethodName(ConstString("X")));
   }
 
   // Construct with a function without a context.
   {
     RichManglingContext CxxMethodRMC;
     EXPECT_TRUE(CxxMethodRMC.FromCxxMethodName(
         ConstString("void * operator new(unsigned __int64)")));
 
     // We expect its context is empty.
-    CxxMethodRMC.ParseFunctionDeclContextName();
-    EXPECT_TRUE(CxxMethodRMC.GetBufferRef().empty());
+    EXPECT_TRUE(CxxMethodRMC.ParseFunctionDeclContextName().empty());
   }
 }
 
 TEST(RichManglingContextTest, SwitchProvider) {
   RichManglingContext RMC;
   llvm::StringRef mangled = "_ZN3foo3barEv";
   llvm::StringRef demangled = "foo::bar()";
 
   EXPECT_TRUE(RMC.FromItaniumName(ConstString(mangled)));
-  RMC.ParseFullName();
-  EXPECT_EQ("foo::bar()", RMC.GetBufferRef());
+  EXPECT_EQ("foo::bar()", RMC.ParseFullName());
 
   EXPECT_TRUE(RMC.FromCxxMethodName(ConstString(demangled)));
-  RMC.ParseFullName();
-  EXPECT_EQ("foo::bar()", RMC.GetBufferRef());
+  EXPECT_EQ("foo::bar()", RMC.ParseFullName());
 
   EXPECT_TRUE(RMC.FromItaniumName(ConstString(mangled)));
-  RMC.ParseFullName();
-  EXPECT_EQ("foo::bar()", RMC.GetBufferRef());
+  EXPECT_EQ("foo::bar()", RMC.ParseFullName());
 }
 
 TEST(RichManglingContextTest, IPDRealloc) {
   // The demangled name should fit into the Itanium default buffer.
   const char *ShortMangled = "_ZN3foo3barEv";
 
   // The demangled name for this will certainly not fit into the default buffer.
   const char *LongMangled =
const char *LongMangled =
       "S1F_EUlS7_E_EEEESI_S6_EUlS7_E_EEEESt6vectorIS6_SaIS6_EERKT0_NS_"
       "12ElementCountEbEUlS7_E_ZNSD_IiS1Q_EES1T_S1W_S1X_bEUlOS3_E_ZNSD_IiS1Q_"
       "EES1T_S1W_S1X_bEUlvE_EES1G_S1O_E25ConcatMapValuesSubscriberEEEDaS7_";
 
   RichManglingContext RMC;
 
   // Demangle the short one.
   EXPECT_TRUE(RMC.FromItaniumName(ConstString(ShortMangled)));
-  RMC.ParseFullName();
-  const char *ShortDemangled = RMC.GetBufferRef().data();
+  const char *ShortDemangled = RMC.ParseFullName().data();
 
   // Demangle the long one.
   EXPECT_TRUE(RMC.FromItaniumName(ConstString(LongMangled)));
-  RMC.ParseFullName();
-  const char *LongDemangled = RMC.GetBufferRef().data();
+  const char *LongDemangled = RMC.ParseFullName().data();
 
   // Make sure a new buffer was allocated or the default buffer was extended.
   bool AllocatedNewBuffer = (ShortDemangled != LongDemangled);
   bool ExtendedExistingBuffer = (strlen(LongDemangled) > 2048);
   EXPECT_TRUE(AllocatedNewBuffer || ExtendedExistingBuffer);
 }