Skip to content

[lldb][FormatEntity][NFCI] Refactor FunctionNameWithArgs into helper functions and use LLVM style #135031

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 1 commit into from
Apr 10, 2025

Conversation

Michael137
Copy link
Member

I've always found this hard to read. Some upcoming changes make similar computations, so I thought it's a good time to factor out this logic into re-usable helpers and clean it up using LLVM's preferred early-return style.

@Michael137 Michael137 requested a review from labath April 9, 2025 14:23
@Michael137 Michael137 requested a review from JDevlieghere as a code owner April 9, 2025 14:23
@llvmbot llvmbot added the lldb label Apr 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

I've always found this hard to read. Some upcoming changes make similar computations, so I thought it's a good time to factor out this logic into re-usable helpers and clean it up using LLVM's preferred early-return style.


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

1 Files Affected:

  • (modified) lldb/source/Core/FormatEntity.cpp (+69-49)
diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp
index 04dea7efde54d..a9370595c11e7 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -1160,6 +1160,64 @@ static void FormatInlinedBlock(Stream &out_stream, Block *block) {
   }
 }
 
+static VariableListSP GetFunctionVariableList(const SymbolContext &sc) {
+  assert(sc.function);
+
+  if (sc.block)
+    if (Block *inline_block = sc.block->GetContainingInlinedBlock())
+      return inline_block->GetBlockVariableList(true);
+
+  return sc.function->GetBlock(true).GetBlockVariableList(true);
+}
+
+static char const *GetInlinedFunctionName(const SymbolContext &sc) {
+  if (!sc.block)
+    return nullptr;
+
+  const Block *inline_block = sc.block->GetContainingInlinedBlock();
+  if (!inline_block)
+    return nullptr;
+
+  const InlineFunctionInfo *inline_info =
+      inline_block->GetInlinedFunctionInfo();
+  if (!inline_info)
+    return nullptr;
+
+  return inline_info->GetName().AsCString(nullptr);
+}
+
+static bool PrintFunctionNameWithArgs(Stream &s,
+                                      const ExecutionContext *exe_ctx,
+                                      const SymbolContext &sc) {
+  assert(sc.function);
+
+  ExecutionContextScope *exe_scope =
+      exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr;
+
+  const char *cstr = sc.function->GetName().AsCString(nullptr);
+  if (!cstr)
+    return false;
+
+  if (const char *inlined_name = GetInlinedFunctionName(sc)) {
+    s.PutCString(cstr);
+    s.PutCString(" [inlined] ");
+    cstr = inlined_name;
+  }
+
+  VariableList args;
+  if (auto variable_list_sp = GetFunctionVariableList(sc))
+    variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument,
+                                               args);
+
+  if (args.GetSize() > 0) {
+    PrettyPrintFunctionNameWithArgs(s, cstr, exe_scope, args);
+  } else {
+    s.PutCString(cstr);
+  }
+
+  return true;
+}
+
 bool FormatEntity::FormatStringRef(const llvm::StringRef &format_str, Stream &s,
                                    const SymbolContext *sc,
                                    const ExecutionContext *exe_ctx,
@@ -1736,59 +1794,21 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
     if (language_plugin_handled) {
       s << ss.GetString();
       return true;
-    } else {
-      // Print the function name with arguments in it
-      if (sc->function) {
-        ExecutionContextScope *exe_scope =
-            exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr;
-        const char *cstr = sc->function->GetName().AsCString(nullptr);
-        if (cstr) {
-          const InlineFunctionInfo *inline_info = nullptr;
-          VariableListSP variable_list_sp;
-          bool get_function_vars = true;
-          if (sc->block) {
-            Block *inline_block = sc->block->GetContainingInlinedBlock();
-
-            if (inline_block) {
-              get_function_vars = false;
-              inline_info = inline_block->GetInlinedFunctionInfo();
-              if (inline_info)
-                variable_list_sp = inline_block->GetBlockVariableList(true);
-            }
-          }
+    }
 
-          if (get_function_vars) {
-            variable_list_sp =
-                sc->function->GetBlock(true).GetBlockVariableList(true);
-          }
+    if (sc->function)
+      return PrintFunctionNameWithArgs(s, exe_ctx, *sc);
 
-          if (inline_info) {
-            s.PutCString(cstr);
-            s.PutCString(" [inlined] ");
-            cstr = inline_info->GetName().GetCString();
-          }
+    if (!sc->symbol)
+      return false;
 
-          VariableList args;
-          if (variable_list_sp)
-            variable_list_sp->AppendVariablesWithScope(
-                eValueTypeVariableArgument, args);
-          if (args.GetSize() > 0) {
-            PrettyPrintFunctionNameWithArgs(s, cstr, exe_scope, args);
-          } else {
-            s.PutCString(cstr);
-          }
-          return true;
-        }
-      } else if (sc->symbol) {
-        const char *cstr = sc->symbol->GetName().AsCString(nullptr);
-        if (cstr) {
-          s.PutCString(cstr);
-          return true;
-        }
-      }
-    }
+    const char *cstr = sc->symbol->GetName().AsCString(nullptr);
+    if (!cstr)
+      return false;
+
+    s.PutCString(cstr);
+    return true;
   }
-    return false;
 
   case Entry::Type::FunctionMangledName: {
     if (!sc)

@Michael137 Michael137 requested a review from adrian-prantl April 9, 2025 16:29
@Michael137 Michael137 merged commit f030f6f into llvm:main Apr 10, 2025
12 checks passed
@Michael137 Michael137 deleted the lldb/name-with-args-cleanpu branch April 10, 2025 08:40
Michael137 added a commit to Michael137/llvm-project that referenced this pull request Apr 11, 2025
…me into helpers and use LLVM style

Same cleanup as in llvm#135031. It pretty much is the same code that we had to duplicate in the language plugin. Maybe eventually we'll find a way of getting rid of the duplication.
Michael137 added a commit that referenced this pull request Apr 11, 2025
…me into helpers and use LLVM style (#135331)

Same cleanup as in #135031. It
pretty much is the same code that we had to duplicate in the language
plugin. Maybe eventually we'll find a way of getting rid of the
duplication.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Apr 11, 2025
…onDisplayName into helpers and use LLVM style (#135331)

Same cleanup as in llvm/llvm-project#135031. It
pretty much is the same code that we had to duplicate in the language
plugin. Maybe eventually we'll find a way of getting rid of the
duplication.
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
…functions and use LLVM style (llvm#135031)

I've always found this hard to read. Some upcoming changes make similar
computations, so I thought it's a good time to factor out this logic
into re-usable helpers and clean it up using LLVM's preferred
early-return style.
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
…me into helpers and use LLVM style (llvm#135331)

Same cleanup as in llvm#135031. It
pretty much is the same code that we had to duplicate in the language
plugin. Maybe eventually we'll find a way of getting rid of the
duplication.
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Apr 26, 2025
…functions and use LLVM style (llvm#135031)

I've always found this hard to read. Some upcoming changes make similar
computations, so I thought it's a good time to factor out this logic
into re-usable helpers and clean it up using LLVM's preferred
early-return style.

(cherry picked from commit f030f6f)
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Apr 26, 2025
…me into helpers and use LLVM style (llvm#135331)

Same cleanup as in llvm#135031. It
pretty much is the same code that we had to duplicate in the language
plugin. Maybe eventually we'll find a way of getting rid of the
duplication.

(cherry picked from commit b656915)
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