[Lldb-commits] [lldb] a0329ea - [lldb] upgrade HandleFrameFormatVariable callees to llvm::Expected (#144731)

via lldb-commits lldb-commits at lists.llvm.org
Mon Jun 23 09:41:51 PDT 2025


Author: Charles Zablit
Date: 2025-06-23T18:41:48+02:00
New Revision: a0329eaa0cdcc86046c66533feb002b227553c33

URL: https://github.com/llvm/llvm-project/commit/a0329eaa0cdcc86046c66533feb002b227553c33
DIFF: https://github.com/llvm/llvm-project/commit/a0329eaa0cdcc86046c66533feb002b227553c33.diff

LOG: [lldb] upgrade HandleFrameFormatVariable callees to llvm::Expected (#144731)

Upgrade the callees of `HandleFrameFormatVariable`
(`GetDemangledTemplateArguments`, etc), to return a `llvm::Expected`
instead of an `std::optional`.

This patch also bundles the logic of validating the demangled name and
information into a single reusable function to reduce code duplication.

Added: 
    

Modified: 
    lldb/include/lldb/Core/DemangledNameInfo.h
    lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
    lldb/unittests/Core/MangledTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/DemangledNameInfo.h b/lldb/include/lldb/Core/DemangledNameInfo.h
index a2f3fde90c611..9f567232dc50f 100644
--- a/lldb/include/lldb/Core/DemangledNameInfo.h
+++ b/lldb/include/lldb/Core/DemangledNameInfo.h
@@ -71,27 +71,28 @@ struct DemangledNameInfo {
 
   /// Returns \c true if this object holds a valid basename range.
   bool hasBasename() const {
+    // A function always has a name.
     return BasenameRange.second > BasenameRange.first;
   }
 
   /// Returns \c true if this object holds a valid scope range.
-  bool hasScope() const { return ScopeRange.second > ScopeRange.first; }
+  bool hasScope() const { return ScopeRange.second >= ScopeRange.first; }
 
   /// Returns \c true if this object holds a valid arguments range.
   bool hasArguments() const {
-    return ArgumentsRange.second > ArgumentsRange.first;
+    return ArgumentsRange.second >= ArgumentsRange.first;
   }
 
   /// Returns \c true if this object holds a valid qualifiers range.
   bool hasQualifiers() const {
-    return QualifiersRange.second > QualifiersRange.first;
+    return QualifiersRange.second >= QualifiersRange.first;
   }
 
   /// Returns \c true if this object holds a valid prefix range.
-  bool hasPrefix() const { return PrefixRange.second > PrefixRange.first; }
+  bool hasPrefix() const { return PrefixRange.second >= PrefixRange.first; }
 
   /// Returns \c true if this object holds a valid suffix range.
-  bool hasSuffix() const { return SuffixRange.second > SuffixRange.first; }
+  bool hasSuffix() const { return SuffixRange.second >= SuffixRange.first; }
 };
 
 /// An OutputBuffer which keeps a record of where certain parts of a

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 0f18abb47591d..7ae2e141a63e0 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -236,199 +236,158 @@ static bool PrettyPrintFunctionNameWithArgs(Stream &out_stream,
   return true;
 }
 
-static std::optional<llvm::StringRef>
-GetDemangledBasename(const SymbolContext &sc) {
+static llvm::Expected<std::pair<llvm::StringRef, DemangledNameInfo>>
+GetAndValidateInfo(const SymbolContext &sc) {
   Mangled mangled = sc.GetPossiblyInlinedFunctionName();
   if (!mangled)
-    return std::nullopt;
+    return llvm::createStringError("Function does not have a mangled name.");
 
   auto demangled_name = mangled.GetDemangledName().GetStringRef();
   if (demangled_name.empty())
-    return std::nullopt;
+    return llvm::createStringError(
+        "Function '%s' does not have a demangled name.",
+        mangled.GetMangledName().AsCString(""));
 
   const std::optional<DemangledNameInfo> &info = mangled.GetDemangledInfo();
   if (!info)
-    return std::nullopt;
+    return llvm::createStringError(
+        "Function '%s' does not have demangled info.", demangled_name.data());
 
   // Function without a basename is nonsense.
   if (!info->hasBasename())
-    return std::nullopt;
+    return llvm::createStringError(
+        "DemangledInfo for '%s does not have basename range.",
+        demangled_name.data());
 
-  return demangled_name.slice(info->BasenameRange.first,
-                              info->BasenameRange.second);
+  return std::make_pair(demangled_name, *info);
 }
 
-static std::optional<llvm::StringRef>
-GetDemangledTemplateArguments(const SymbolContext &sc) {
-  Mangled mangled = sc.GetPossiblyInlinedFunctionName();
-  if (!mangled)
-    return std::nullopt;
+static llvm::Expected<llvm::StringRef>
+GetDemangledBasename(const SymbolContext &sc) {
+  auto info_or_err = GetAndValidateInfo(sc);
+  if (!info_or_err)
+    return info_or_err.takeError();
 
-  auto demangled_name = mangled.GetDemangledName().GetStringRef();
-  if (demangled_name.empty())
-    return std::nullopt;
+  auto [demangled_name, info] = *info_or_err;
 
-  const std::optional<DemangledNameInfo> &info = mangled.GetDemangledInfo();
-  if (!info)
-    return std::nullopt;
+  return demangled_name.slice(info.BasenameRange.first,
+                              info.BasenameRange.second);
+}
 
-  // Function without a basename is nonsense.
-  if (!info->hasBasename())
-    return std::nullopt;
+static llvm::Expected<llvm::StringRef>
+GetDemangledTemplateArguments(const SymbolContext &sc) {
+  auto info_or_err = GetAndValidateInfo(sc);
+  if (!info_or_err)
+    return info_or_err.takeError();
+
+  auto [demangled_name, info] = *info_or_err;
 
-  if (info->ArgumentsRange.first < info->BasenameRange.second)
-    return std::nullopt;
+  if (info.ArgumentsRange.first < info.BasenameRange.second)
+    return llvm::createStringError("Arguments range for '%s' is invalid.",
+                                   demangled_name.data());
 
-  return demangled_name.slice(info->BasenameRange.second,
-                              info->ArgumentsRange.first);
+  return demangled_name.slice(info.BasenameRange.second,
+                              info.ArgumentsRange.first);
 }
 
-static std::optional<llvm::StringRef>
+static llvm::Expected<llvm::StringRef>
 GetDemangledReturnTypeLHS(const SymbolContext &sc) {
-  Mangled mangled = sc.GetPossiblyInlinedFunctionName();
-  if (!mangled)
-    return std::nullopt;
+  auto info_or_err = GetAndValidateInfo(sc);
+  if (!info_or_err)
+    return info_or_err.takeError();
 
-  auto demangled_name = mangled.GetDemangledName().GetStringRef();
-  if (demangled_name.empty())
-    return std::nullopt;
+  auto [demangled_name, info] = *info_or_err;
 
-  const std::optional<DemangledNameInfo> &info = mangled.GetDemangledInfo();
-  if (!info)
-    return std::nullopt;
+  if (info.ScopeRange.first >= demangled_name.size())
+    return llvm::createStringError(
+        "Scope range for '%s' LHS return type is invalid.",
+        demangled_name.data());
 
-  // Function without a basename is nonsense.
-  if (!info->hasBasename())
-    return std::nullopt;
-
-  if (info->ScopeRange.first >= demangled_name.size())
-    return std::nullopt;
-
-  return demangled_name.substr(0, info->ScopeRange.first);
+  return demangled_name.substr(0, info.ScopeRange.first);
 }
 
-static std::optional<llvm::StringRef>
+static llvm::Expected<llvm::StringRef>
 GetDemangledFunctionQualifiers(const SymbolContext &sc) {
-  Mangled mangled = sc.GetPossiblyInlinedFunctionName();
-  if (!mangled)
-    return std::nullopt;
+  auto info_or_err = GetAndValidateInfo(sc);
+  if (!info_or_err)
+    return info_or_err.takeError();
 
-  auto demangled_name = mangled.GetDemangledName().GetStringRef();
-  if (demangled_name.empty())
-    return std::nullopt;
+  auto [demangled_name, info] = *info_or_err;
 
-  const std::optional<DemangledNameInfo> &info = mangled.GetDemangledInfo();
-  if (!info)
-    return std::nullopt;
-
-  // Function without a basename is nonsense.
-  if (!info->hasBasename())
-    return std::nullopt;
+  if (!info.hasQualifiers())
+    return llvm::createStringError("Qualifiers range for '%s' is invalid.",
+                                   demangled_name.data());
 
-  if (info->QualifiersRange.second < info->QualifiersRange.first)
-    return std::nullopt;
-
-  return demangled_name.slice(info->QualifiersRange.first,
-                              info->QualifiersRange.second);
+  return demangled_name.slice(info.QualifiersRange.first,
+                              info.QualifiersRange.second);
 }
 
-static std::optional<llvm::StringRef>
+static llvm::Expected<llvm::StringRef>
 GetDemangledReturnTypeRHS(const SymbolContext &sc) {
-  Mangled mangled = sc.GetPossiblyInlinedFunctionName();
-  if (!mangled)
-    return std::nullopt;
-
-  auto demangled_name = mangled.GetDemangledName().GetStringRef();
-  if (demangled_name.empty())
-    return std::nullopt;
+  auto info_or_err = GetAndValidateInfo(sc);
+  if (!info_or_err)
+    return info_or_err.takeError();
 
-  const std::optional<DemangledNameInfo> &info = mangled.GetDemangledInfo();
-  if (!info)
-    return std::nullopt;
+  auto [demangled_name, info] = *info_or_err;
 
-  // Function without a basename is nonsense.
-  if (!info->hasBasename())
-    return std::nullopt;
+  if (info.QualifiersRange.first < info.ArgumentsRange.second)
+    return llvm::createStringError(
+        "Qualifiers range for '%s' RHS return type  is invalid.",
+        demangled_name.data());
 
-  if (info->QualifiersRange.first < info->ArgumentsRange.second)
-    return std::nullopt;
-
-  return demangled_name.slice(info->ArgumentsRange.second,
-                              info->QualifiersRange.first);
+  return demangled_name.slice(info.ArgumentsRange.second,
+                              info.QualifiersRange.first);
 }
 
-static std::optional<llvm::StringRef>
+static llvm::Expected<llvm::StringRef>
 GetDemangledScope(const SymbolContext &sc) {
-  Mangled mangled = sc.GetPossiblyInlinedFunctionName();
-  if (!mangled)
-    return std::nullopt;
-
-  auto demangled_name = mangled.GetDemangledName().GetStringRef();
-  if (demangled_name.empty())
-    return std::nullopt;
+  auto info_or_err = GetAndValidateInfo(sc);
+  if (!info_or_err)
+    return info_or_err.takeError();
 
-  const std::optional<DemangledNameInfo> &info = mangled.GetDemangledInfo();
-  if (!info)
-    return std::nullopt;
-
-  // Function without a basename is nonsense.
-  if (!info->hasBasename())
-    return std::nullopt;
+  auto [demangled_name, info] = *info_or_err;
 
-  if (info->ScopeRange.second < info->ScopeRange.first)
-    return std::nullopt;
+  if (!info.hasScope())
+    return llvm::createStringError("Scope range for '%s' is invalid.",
+                                   demangled_name.data());
 
-  return demangled_name.slice(info->ScopeRange.first, info->ScopeRange.second);
+  return demangled_name.slice(info.ScopeRange.first, info.ScopeRange.second);
 }
 
 /// Handles anything printed after the FunctionEncoding ItaniumDemangle
 /// node. Most notably the DotSUffix node.
-static std::optional<llvm::StringRef>
+static llvm::Expected<llvm::StringRef>
 GetDemangledFunctionSuffix(const SymbolContext &sc) {
-  Mangled mangled = sc.GetPossiblyInlinedFunctionName();
-  if (!mangled)
-    return std::nullopt;
-
-  auto demangled_name = mangled.GetDemangledName().GetStringRef();
-  if (demangled_name.empty())
-    return std::nullopt;
+  auto info_or_err = GetAndValidateInfo(sc);
+  if (!info_or_err)
+    return info_or_err.takeError();
 
-  const std::optional<DemangledNameInfo> &info = mangled.GetDemangledInfo();
-  if (!info)
-    return std::nullopt;
+  auto [demangled_name, info] = *info_or_err;
 
-  // Function without a basename is nonsense.
-  if (!info->hasBasename())
-    return std::nullopt;
+  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 demangled_name.slice(info.SuffixRange.first, info.SuffixRange.second);
 }
 
 static bool PrintDemangledArgumentList(Stream &s, const SymbolContext &sc) {
   assert(sc.symbol);
 
-  Mangled mangled = sc.GetPossiblyInlinedFunctionName();
-  if (!mangled)
-    return false;
-
-  auto demangled_name = mangled.GetDemangledName().GetStringRef();
-  if (demangled_name.empty())
-    return false;
-
-  const std::optional<DemangledNameInfo> &info = mangled.GetDemangledInfo();
-  if (!info)
-    return false;
-
-  // Function without a basename is nonsense.
-  if (!info->hasBasename())
+  auto info_or_err = GetAndValidateInfo(sc);
+  if (!info_or_err) {
+    LLDB_LOG_ERROR(
+        GetLog(LLDBLog::Language), info_or_err.takeError(),
+        "Failed to handle ${{function.basename}} frame-format variable: {0}");
     return false;
+  }
+  auto [demangled_name, info] = *info_or_err;
 
-  if (info->ArgumentsRange.second < info->ArgumentsRange.first)
+  if (!info.hasArguments())
     return false;
 
-  s << demangled_name.slice(info->ArgumentsRange.first,
-                            info->ArgumentsRange.second);
+  s << demangled_name.slice(info.ArgumentsRange.first,
+                            info.ArgumentsRange.second);
 
   return true;
 }
@@ -1954,32 +1913,44 @@ bool CPlusPlusLanguage::HandleFrameFormatVariable(
     FormatEntity::Entry::Type type, Stream &s) {
   switch (type) {
   case FormatEntity::Entry::Type::FunctionScope: {
-    std::optional<llvm::StringRef> scope = GetDemangledScope(sc);
-    if (!scope)
+    auto scope_or_err = GetDemangledScope(sc);
+    if (!scope_or_err) {
+      LLDB_LOG_ERROR(
+          GetLog(LLDBLog::Language), scope_or_err.takeError(),
+          "Failed to handle ${{function.scope}} frame-format variable: {0}");
       return false;
+    }
 
-    s << *scope;
+    s << *scope_or_err;
 
     return true;
   }
 
   case FormatEntity::Entry::Type::FunctionBasename: {
-    std::optional<llvm::StringRef> name = GetDemangledBasename(sc);
-    if (!name)
+    auto name_or_err = GetDemangledBasename(sc);
+    if (!name_or_err) {
+      LLDB_LOG_ERROR(
+          GetLog(LLDBLog::Language), name_or_err.takeError(),
+          "Failed to handle ${{function.basename}} frame-format variable: {0}");
       return false;
+    }
 
-    s << *name;
+    s << *name_or_err;
 
     return true;
   }
 
   case FormatEntity::Entry::Type::FunctionTemplateArguments: {
-    std::optional<llvm::StringRef> template_args =
-        GetDemangledTemplateArguments(sc);
-    if (!template_args)
+    auto template_args_or_err = GetDemangledTemplateArguments(sc);
+    if (!template_args_or_err) {
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Language),
+                     template_args_or_err.takeError(),
+                     "Failed to handle ${{function.template-arguments}} "
+                     "frame-format variable: {0}");
       return false;
+    }
 
-    s << *template_args;
+    s << *template_args_or_err;
 
     return true;
   }
@@ -2008,38 +1979,54 @@ bool CPlusPlusLanguage::HandleFrameFormatVariable(
     return true;
   }
   case FormatEntity::Entry::Type::FunctionReturnRight: {
-    std::optional<llvm::StringRef> return_rhs = GetDemangledReturnTypeRHS(sc);
-    if (!return_rhs)
+    auto return_rhs_or_err = GetDemangledReturnTypeRHS(sc);
+    if (!return_rhs_or_err) {
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Language), return_rhs_or_err.takeError(),
+                     "Failed to handle ${{function.return-right}} frame-format "
+                     "variable: {0}");
       return false;
+    }
 
-    s << *return_rhs;
+    s << *return_rhs_or_err;
 
     return true;
   }
   case FormatEntity::Entry::Type::FunctionReturnLeft: {
-    std::optional<llvm::StringRef> return_lhs = GetDemangledReturnTypeLHS(sc);
-    if (!return_lhs)
+    auto return_lhs_or_err = GetDemangledReturnTypeLHS(sc);
+    if (!return_lhs_or_err) {
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Language), return_lhs_or_err.takeError(),
+                     "Failed to handle ${{function.return-left}} frame-format "
+                     "variable: {0}");
       return false;
+    }
 
-    s << *return_lhs;
+    s << *return_lhs_or_err;
 
     return true;
   }
   case FormatEntity::Entry::Type::FunctionQualifiers: {
-    std::optional<llvm::StringRef> quals = GetDemangledFunctionQualifiers(sc);
-    if (!quals)
+    auto quals_or_err = GetDemangledFunctionQualifiers(sc);
+    if (!quals_or_err) {
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Language), quals_or_err.takeError(),
+                     "Failed to handle ${{function.qualifiers}} frame-format "
+                     "variable: {0}");
       return false;
+    }
 
-    s << *quals;
+    s << *quals_or_err;
 
     return true;
   }
   case FormatEntity::Entry::Type::FunctionSuffix: {
-    std::optional<llvm::StringRef> suffix = GetDemangledFunctionSuffix(sc);
-    if (!suffix)
+    auto suffix_or_err = GetDemangledFunctionSuffix(sc);
+    if (!suffix_or_err) {
+      LLDB_LOG_ERROR(
+          GetLog(LLDBLog::Language), suffix_or_err.takeError(),
+          "Failed to handle ${{function.suffix}} frame-format variable: {0}");
       return false;
+    }
 
-    s << *suffix;
+    s << *suffix_or_err;
 
     return true;
   }

diff  --git a/lldb/unittests/Core/MangledTest.cpp b/lldb/unittests/Core/MangledTest.cpp
index 5994d6072481b..4bda657047541 100644
--- a/lldb/unittests/Core/MangledTest.cpp
+++ b/lldb/unittests/Core/MangledTest.cpp
@@ -648,43 +648,43 @@ struct DemangledNameInfoTestCase {
 DemangledNameInfoTestCase g_demangled_name_info_test_cases[] = {
     // clang-format off
    {
-    { /*.BasenameRange=*/{0, 10}, /*.ScopeRange=*/{0, 0}, /*.ArgumentsRange=*/{0, 0},
-      /*.QualifiersRange=*/{0, 0}, /*.PrefixRange=*/{0, 0}, /*.SuffixRange=*/{0, 0}
+    { /*.BasenameRange=*/{0, 10}, /*.ScopeRange=*/{1, 0}, /*.ArgumentsRange=*/{1, 0},
+      /*.QualifiersRange=*/{1, 0}, /*.PrefixRange=*/{1, 0}, /*.SuffixRange=*/{1, 0}
     },
       /*valid_basename=*/true, /*valid_scope=*/false, /*valid_arguments=*/false,
       /*valid_qualifiers=*/false, /*valid_prefix=*/false, /*valid_suffix=*/false,
    },
    {
-    { /*.BasenameRange=*/{0, 0}, /*.ScopeRange=*/{0, 10}, /*.ArgumentsRange=*/{0, 0},
-      /*.QualifiersRange=*/{0, 0}, /*.PrefixRange=*/{0, 0}, /*.SuffixRange=*/{0, 0}
+    { /*.BasenameRange=*/{1, 0}, /*.ScopeRange=*/{0, 10}, /*.ArgumentsRange=*/{1, 0},
+      /*.QualifiersRange=*/{1, 0}, /*.PrefixRange=*/{1, 0}, /*.SuffixRange=*/{1, 0}
     },
       /*valid_basename=*/false, /*valid_scope=*/true, /*valid_arguments=*/false,
       /*valid_qualifiers=*/false, /*valid_prefix=*/false, /*valid_suffix=*/false,
    },
    {
-    { /*.BasenameRange=*/{0, 0}, /*.ScopeRange=*/{0, 0}, /*.ArgumentsRange=*/{0, 10},
-      /*.QualifiersRange=*/{0, 0}, /*.PrefixRange=*/{0, 0}, /*.SuffixRange=*/{0, 0}
+    { /*.BasenameRange=*/{1, 0}, /*.ScopeRange=*/{1, 0}, /*.ArgumentsRange=*/{0, 10},
+      /*.QualifiersRange=*/{1, 0}, /*.PrefixRange=*/{1, 0}, /*.SuffixRange=*/{1, 0}
     },
       /*valid_basename=*/false, /*valid_scope=*/false, /*valid_arguments=*/true,
       /*valid_qualifiers=*/false, /*valid_prefix=*/false, /*valid_suffix=*/false,
    },
    {
-    { /*.BasenameRange=*/{0, 0}, /*.ScopeRange=*/{0, 0}, /*.ArgumentsRange=*/{0, 0},
-      /*.QualifiersRange=*/{0, 10}, /*.PrefixRange=*/{0, 0}, /*.SuffixRange=*/{0, 0}
+    { /*.BasenameRange=*/{1, 0}, /*.ScopeRange=*/{1, 0}, /*.ArgumentsRange=*/{1, 0},
+      /*.QualifiersRange=*/{0, 10}, /*.PrefixRange=*/{1, 0}, /*.SuffixRange=*/{1, 0}
     },
       /*valid_basename=*/false, /*valid_scope=*/false, /*valid_arguments=*/false,
       /*valid_qualifiers=*/true, /*valid_prefix=*/false, /*valid_suffix=*/false,
    },
    {
-    { /*.BasenameRange=*/{0, 0}, /*.ScopeRange=*/{0, 0}, /*.ArgumentsRange=*/{0, 0},
-      /*.QualifiersRange=*/{0, 0}, /*.PrefixRange=*/{0, 10}, /*.SuffixRange=*/{0, 0}
+    { /*.BasenameRange=*/{1, 0}, /*.ScopeRange=*/{1, 0}, /*.ArgumentsRange=*/{1, 0},
+      /*.QualifiersRange=*/{1, 0}, /*.PrefixRange=*/{0, 10}, /*.SuffixRange=*/{1, 0}
     },
       /*valid_basename=*/false, /*valid_scope=*/false, /*valid_arguments=*/false,
       /*valid_qualifiers=*/false, /*valid_prefix=*/true, /*valid_suffix=*/false,
    },
    {
-    { /*.BasenameRange=*/{0, 0}, /*.ScopeRange=*/{0, 0}, /*.ArgumentsRange=*/{0, 0},
-      /*.QualifiersRange=*/{0, 0}, /*.PrefixRange=*/{0, 0}, /*.SuffixRange=*/{0, 10}
+    { /*.BasenameRange=*/{1, 0}, /*.ScopeRange=*/{1, 0}, /*.ArgumentsRange=*/{1, 0},
+      /*.QualifiersRange=*/{1, 0}, /*.PrefixRange=*/{1, 0}, /*.SuffixRange=*/{0, 10}
     },
       /*valid_basename=*/false, /*valid_scope=*/false, /*valid_arguments=*/false,
       /*valid_qualifiers=*/false, /*valid_prefix=*/false, /*valid_suffix=*/true,


        


More information about the lldb-commits mailing list