Skip to content

[lldb][Mangled] Move SuffixRange computation into TrackingOutputBuffer #152483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 7, 2025

Conversation

Michael137
Copy link
Member

This way all the tracking is self-contained in TrackingOutputBuffer and we can test the SuffixRange properly.

This way all the tracking is self-contained in `TrackingOutputBuffer`
and we can test the `SuffixRange` properly.
@llvmbot
Copy link
Member

llvmbot commented Aug 7, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

This way all the tracking is self-contained in TrackingOutputBuffer and we can test the SuffixRange properly.


Full diff: https://github.com/llvm/llvm-project/pull/152483.diff

5 Files Affected:

  • (modified) lldb/source/Core/DemangledNameInfo.cpp (+11)
  • (modified) lldb/source/Core/Mangled.cpp (-3)
  • (modified) lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp (+12-13)
  • (modified) lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h (+4)
  • (modified) lldb/unittests/Core/MangledTest.cpp (+4-4)
diff --git a/lldb/source/Core/DemangledNameInfo.cpp b/lldb/source/Core/DemangledNameInfo.cpp
index 00227f02bbff8..4001e699f75c2 100644
--- a/lldb/source/Core/DemangledNameInfo.cpp
+++ b/lldb/source/Core/DemangledNameInfo.cpp
@@ -111,6 +111,11 @@ void TrackingOutputBuffer::finalizeEnd() {
   if (NameInfo.ScopeRange.first > NameInfo.ScopeRange.second)
     NameInfo.ScopeRange.second = NameInfo.ScopeRange.first;
   NameInfo.BasenameRange.first = NameInfo.ScopeRange.second;
+
+  // We call anything past the FunctionEncoding is the suffix.
+  // In practice this would be nodes like `DotSuffix` that wrap
+  // a FunctionEncoding.
+  NameInfo.SuffixRange.first = getCurrentPosition();
 }
 
 ScopedOverride<unsigned> TrackingOutputBuffer::enterFunctionTypePrinting() {
@@ -138,6 +143,9 @@ void TrackingOutputBuffer::printLeft(const Node &N) {
   default:
     OutputBuffer::printLeft(N);
   }
+
+  // Keeps updating suffix until we reach the end.
+  NameInfo.SuffixRange.second = getCurrentPosition();
 }
 
 void TrackingOutputBuffer::printRight(const Node &N) {
@@ -151,6 +159,9 @@ void TrackingOutputBuffer::printRight(const Node &N) {
   default:
     OutputBuffer::printRight(N);
   }
+
+  // Keeps updating suffix until we reach the end.
+  NameInfo.SuffixRange.second = getCurrentPosition();
 }
 
 void TrackingOutputBuffer::printLeftImpl(const FunctionType &N) {
diff --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index 3663f430111c2..ce4db4e0daa8b 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -172,9 +172,6 @@ GetItaniumDemangledStr(const char *M) {
 
     TrackingOutputBuffer OB(demangled_cstr, demangled_size);
     demangled_cstr = ipd.finishDemangle(&OB);
-    // TODO: we should set the SuffixRange inside the TrackingOutputBuffer.
-    OB.NameInfo.SuffixRange.first = OB.NameInfo.QualifiersRange.second;
-    OB.NameInfo.SuffixRange.second = std::string_view(OB).size();
     info = std::move(OB.NameInfo);
 
     assert(demangled_cstr &&
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 3bc870810dc81..3118ff151d1cf 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -392,13 +392,16 @@ GetDemangledScope(const SymbolContext &sc) {
   return CPlusPlusLanguage::GetDemangledScope(demangled_name, info);
 }
 
-/// Handles anything printed after the FunctionEncoding ItaniumDemangle
-/// node. Most notably the DotSuffix node.
-///
-/// FIXME: the suffix should also have an associated
-/// CPlusPlusLanguage::GetDemangledFunctionSuffix
-/// once we start setting the `DemangledNameInfo::SuffixRange`
-/// from inside the `TrackingOutputBuffer`.
+llvm::Expected<llvm::StringRef>
+CPlusPlusLanguage::GetDemangledFunctionSuffix(llvm::StringRef demangled,
+                                              const DemangledNameInfo &info) {
+  if (!info.hasSuffix())
+    return llvm::createStringError("Suffix range for '%s' is invalid.",
+                                   demangled.data());
+
+  return demangled.slice(info.SuffixRange.first, info.SuffixRange.second);
+}
+
 static llvm::Expected<llvm::StringRef>
 GetDemangledFunctionSuffix(const SymbolContext &sc) {
   auto info_or_err = GetAndValidateInfo(sc);
@@ -407,11 +410,7 @@ GetDemangledFunctionSuffix(const SymbolContext &sc) {
 
   auto [demangled_name, info] = *info_or_err;
 
-  if (!info.hasSuffix())
-    return llvm::createStringError("Suffix range for '%s' is invalid.",
-                                   demangled_name.data());
-
-  return demangled_name.slice(info.SuffixRange.first, info.SuffixRange.second);
+  return CPlusPlusLanguage::GetDemangledFunctionSuffix(demangled_name, info);
 }
 
 llvm::Expected<llvm::StringRef>
@@ -2424,7 +2423,7 @@ bool CPlusPlusLanguage::HandleFrameFormatVariable(
     return true;
   }
   case FormatEntity::Entry::Type::FunctionSuffix: {
-    auto suffix_or_err = GetDemangledFunctionSuffix(sc);
+    auto suffix_or_err = ::GetDemangledFunctionSuffix(sc);
     if (!suffix_or_err) {
       LLDB_LOG_ERROR(
           GetLog(LLDBLog::Language), suffix_or_err.takeError(),
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
index 4f449f11257a6..4a30299dd2658 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -138,6 +138,10 @@ class CPlusPlusLanguage : public Language {
   GetDemangledFunctionArguments(llvm::StringRef demangled,
                                 const DemangledNameInfo &info);
 
+  static llvm::Expected<llvm::StringRef>
+  GetDemangledFunctionSuffix(llvm::StringRef demangled,
+                             const DemangledNameInfo &info);
+
   // Extract C++ context and identifier from a string using heuristic matching
   // (as opposed to
   // CPlusPlusLanguage::CxxMethodName which has to have a fully qualified C++
diff --git a/lldb/unittests/Core/MangledTest.cpp b/lldb/unittests/Core/MangledTest.cpp
index a6e6e75fde145..cbc0c5d951b99 100644
--- a/lldb/unittests/Core/MangledTest.cpp
+++ b/lldb/unittests/Core/MangledTest.cpp
@@ -889,10 +889,10 @@ TEST_P(DemanglingInfoCorrectnessTestFixutre, Correctness) {
   EXPECT_THAT_EXPECTED(qualifiers, llvm::Succeeded());
   reconstructed_name += *qualifiers;
 
-  // TODO: should retrieve suffix using the plugin too.
-  auto suffix = tracked_name.slice(OB->NameInfo.QualifiersRange.second,
-                                   llvm::StringRef::npos);
-  reconstructed_name += suffix;
+  auto suffix =
+      CPlusPlusLanguage::GetDemangledFunctionSuffix(tracked_name, OB->NameInfo);
+  EXPECT_THAT_EXPECTED(suffix, llvm::Succeeded());
+  reconstructed_name += *suffix;
 
   EXPECT_EQ(reconstructed_name, demangled);
 }

@Michael137 Michael137 merged commit fac7453 into llvm:main Aug 7, 2025
9 checks passed
@Michael137 Michael137 deleted the lldb/suffix-range-refactor branch August 7, 2025 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants