[Lldb-commits] [lldb] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine (PR #95078)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Tue Jun 11 03:24:42 PDT 2024


https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/95078

>From 75d1bc01ca42e327594cceba753bec483228efbd Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 10 Jun 2024 17:46:31 +0100
Subject: [PATCH 1/4] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method
 specifics out of ParseSubroutine

This patch moves some of the `is_cxx_method`/`objc_method` logic out
of `DWARFASTParserClang::ParseSubroutine` into their own functions.
Mainly the purpose of this is to (hopefully) make this function more
readable by turning the deeply nested if-statements into early-returns.
This will be useful in an upcoming change where we remove some of the
branches of said if-statement.
---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 447 +++++++++---------
 .../SymbolFile/DWARF/DWARFASTParserClang.h    |  15 +
 2 files changed, 251 insertions(+), 211 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 579a538af3634..585ae4e58cc1a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -975,6 +975,226 @@ ConvertDWARFCallingConventionToClang(const ParsedDWARFTypeAttributes &attrs) {
   return clang::CC_C;
 }
 
+bool DWARFASTParserClang::HandleObjCMethod(
+    ObjCLanguage::MethodName const &objc_method, DWARFDIE const &die,
+    CompilerType clang_type, ParsedDWARFTypeAttributes const &attrs,
+    bool is_variadic) {
+  SymbolFileDWARF *dwarf = die.GetDWARF();
+  const auto tag = die.Tag();
+  ConstString class_name(objc_method.GetClassName());
+  if (!class_name)
+    return false;
+
+  TypeSP complete_objc_class_type_sp(
+      dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(), class_name,
+                                                  false));
+
+  if (!complete_objc_class_type_sp)
+    return false;
+
+  CompilerType type_clang_forward_type =
+      complete_objc_class_type_sp->GetForwardCompilerType();
+
+  if (!type_clang_forward_type)
+    return false;
+
+  if (!TypeSystemClang::IsObjCObjectOrInterfaceType(type_clang_forward_type))
+    return false;
+
+  clang::ObjCMethodDecl *objc_method_decl = m_ast.AddMethodToObjCObjectType(
+      type_clang_forward_type, attrs.name.GetCString(), clang_type,
+      attrs.is_artificial, is_variadic, attrs.is_objc_direct_call);
+
+  if (!objc_method_decl) {
+    dwarf->GetObjectFile()->GetModule()->ReportError(
+        "[{0:x16}]: invalid Objective-C method {1:x4} ({2}), "
+        "please file a bug and attach the file at the start of "
+        "this error message",
+        die.GetOffset(), tag, DW_TAG_value_to_name(tag));
+    return false;
+  }
+
+  LinkDeclContextToDIE(objc_method_decl, die);
+  m_ast.SetMetadataAsUserID(objc_method_decl, die.GetID());
+
+  return true;
+}
+
+std::pair<bool, TypeSP> DWARFASTParserClang::HandleCXXMethod(
+    DWARFDIE const &die, CompilerType clang_type,
+    ParsedDWARFTypeAttributes const &attrs, DWARFDIE const &decl_ctx_die,
+    bool is_static, bool &ignore_containing_context) {
+  Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
+  SymbolFileDWARF *dwarf = die.GetDWARF();
+  // Look at the parent of this DIE and see if it is a class or
+  // struct and see if this is actually a C++ method
+  Type *class_type = dwarf->ResolveType(decl_ctx_die);
+  if (!class_type)
+    return {};
+
+  if (class_type->GetID() != decl_ctx_die.GetID() ||
+      IsClangModuleFwdDecl(decl_ctx_die)) {
+
+    // We uniqued the parent class of this function to another
+    // class so we now need to associate all dies under
+    // "decl_ctx_die" to DIEs in the DIE for "class_type"...
+    DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
+
+    if (class_type_die) {
+      std::vector<DWARFDIE> failures;
+
+      CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die, class_type,
+                                 failures);
+
+      // FIXME do something with these failures that's
+      // smarter than just dropping them on the ground.
+      // Unfortunately classes don't like having stuff added
+      // to them after their definitions are complete...
+
+      Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
+      if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
+        return {true, type_ptr->shared_from_this()};
+      }
+    }
+  }
+
+  if (attrs.specification.IsValid()) {
+    // We have a specification which we are going to base our
+    // function prototype off of, so we need this type to be
+    // completed so that the m_die_to_decl_ctx for the method in
+    // the specification has a valid clang decl context.
+    class_type->GetForwardCompilerType();
+    // If we have a specification, then the function type should
+    // have been made with the specification and not with this
+    // die.
+    DWARFDIE spec_die = attrs.specification.Reference();
+    clang::DeclContext *spec_clang_decl_ctx =
+        GetClangDeclContextForDIE(spec_die);
+    if (spec_clang_decl_ctx) {
+      LinkDeclContextToDIE(spec_clang_decl_ctx, die);
+    } else {
+      dwarf->GetObjectFile()->GetModule()->ReportWarning(
+          "{0:x8}: DW_AT_specification({1:x16}"
+          ") has no decl\n",
+          die.GetID(), spec_die.GetOffset());
+    }
+    return {true, nullptr};
+  }
+
+  if (attrs.abstract_origin.IsValid()) {
+    // We have a specification which we are going to base our
+    // function prototype off of, so we need this type to be
+    // completed so that the m_die_to_decl_ctx for the method in
+    // the abstract origin has a valid clang decl context.
+    class_type->GetForwardCompilerType();
+
+    DWARFDIE abs_die = attrs.abstract_origin.Reference();
+    clang::DeclContext *abs_clang_decl_ctx = GetClangDeclContextForDIE(abs_die);
+    if (abs_clang_decl_ctx) {
+      LinkDeclContextToDIE(abs_clang_decl_ctx, die);
+    } else {
+      dwarf->GetObjectFile()->GetModule()->ReportWarning(
+          "{0:x8}: DW_AT_abstract_origin({1:x16}"
+          ") has no decl\n",
+          die.GetID(), abs_die.GetOffset());
+    }
+
+    return {true, nullptr};
+  }
+
+  CompilerType class_opaque_type = class_type->GetForwardCompilerType();
+  if (!TypeSystemClang::IsCXXClassType(class_opaque_type))
+    return {};
+
+  if (class_opaque_type.IsBeingDefined()) {
+    if (!is_static && !die.HasChildren()) {
+      // We have a C++ member function with no children (this
+      // pointer!) and clang will get mad if we try and make
+      // a function that isn't well formed in the DWARF, so
+      // we will just skip it...
+      return {true, nullptr};
+    }
+
+    llvm::PrettyStackTraceFormat stack_trace(
+        "SymbolFileDWARF::ParseType() is adding a method "
+        "%s to class %s in DIE 0x%8.8" PRIx64 " from %s",
+        attrs.name.GetCString(), class_type->GetName().GetCString(),
+        die.GetID(), dwarf->GetObjectFile()->GetFileSpec().GetPath().c_str());
+
+    const bool is_attr_used = false;
+    // Neither GCC 4.2 nor clang++ currently set a valid
+    // accessibility in the DWARF for C++ methods...
+    // Default to public for now...
+    const auto accessibility = attrs.accessibility == eAccessNone
+                                   ? eAccessPublic
+                                   : attrs.accessibility;
+
+    clang::CXXMethodDecl *cxx_method_decl = m_ast.AddMethodToCXXRecordType(
+        class_opaque_type.GetOpaqueQualType(), attrs.name.GetCString(),
+        attrs.mangled_name, clang_type, accessibility, attrs.is_virtual,
+        is_static, attrs.is_inline, attrs.is_explicit, is_attr_used,
+        attrs.is_artificial);
+
+    if (cxx_method_decl) {
+      LinkDeclContextToDIE(cxx_method_decl, die);
+
+      ClangASTMetadata metadata;
+      metadata.SetUserID(die.GetID());
+
+      char const *object_pointer_name =
+          attrs.object_pointer ? attrs.object_pointer.GetName() : nullptr;
+      if (object_pointer_name) {
+        metadata.SetObjectPtrName(object_pointer_name);
+        LLDB_LOGF(log,
+                  "Setting object pointer name: %s on method "
+                  "object %p.\n",
+                  object_pointer_name, static_cast<void *>(cxx_method_decl));
+      }
+      m_ast.SetMetadata(cxx_method_decl, metadata);
+    } else {
+      ignore_containing_context = true;
+    }
+
+    // Artificial methods are always handled even when we
+    // don't create a new declaration for them.
+    const bool type_handled = cxx_method_decl != nullptr || attrs.is_artificial;
+
+    return {type_handled, nullptr};
+  }
+
+  // We were asked to parse the type for a method in a
+  // class, yet the class hasn't been asked to complete
+  // itself through the clang::ExternalASTSource protocol,
+  // so we need to just have the class complete itself and
+  // do things the right way, then our
+  // DIE should then have an entry in the
+  // dwarf->GetDIEToType() map. First
+  // we need to modify the dwarf->GetDIEToType() so it
+  // doesn't think we are trying to parse this DIE
+  // anymore...
+  dwarf->GetDIEToType()[die.GetDIE()] = NULL;
+
+  // Now we get the full type to force our class type to
+  // complete itself using the clang::ExternalASTSource
+  // protocol which will parse all base classes and all
+  // methods (including the method for this DIE).
+  class_type->GetFullCompilerType();
+
+  // The type for this DIE should have been filled in the
+  // function call above.
+  Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
+  if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
+    return {true, type_ptr->shared_from_this()};
+  }
+
+  // The previous comment isn't actually true if the class wasn't
+  // resolved using the current method's parent DIE as source
+  // data. We need to ensure that we look up the method correctly
+  // in the class and then link the method's DIE to the unique
+  // CXXMethodDecl appropriately.
+  return {true, nullptr};
+}
+
 TypeSP
 DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
                                      const ParsedDWARFTypeAttributes &attrs) {
@@ -989,13 +1209,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
 
   unsigned type_quals = 0;
 
-  std::string object_pointer_name;
-  if (attrs.object_pointer) {
-    const char *object_pointer_name_cstr = attrs.object_pointer.GetName();
-    if (object_pointer_name_cstr)
-      object_pointer_name = object_pointer_name_cstr;
-  }
-
   DEBUG_PRINTF("0x%8.8" PRIx64 ": %s (\"%s\")\n", die.GetID(),
                DW_TAG_value_to_name(tag), type_name_cstr);
 
@@ -1066,208 +1279,19 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
   if (attrs.name) {
     bool type_handled = false;
     if (tag == DW_TAG_subprogram || tag == DW_TAG_inlined_subroutine) {
-      std::optional<const ObjCLanguage::MethodName> objc_method =
-          ObjCLanguage::MethodName::Create(attrs.name.GetStringRef(), true);
-      if (objc_method) {
-        CompilerType class_opaque_type;
-        ConstString class_name(objc_method->GetClassName());
-        if (class_name) {
-          TypeSP complete_objc_class_type_sp(
-              dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(),
-                                                          class_name, false));
-
-          if (complete_objc_class_type_sp) {
-            CompilerType type_clang_forward_type =
-                complete_objc_class_type_sp->GetForwardCompilerType();
-            if (TypeSystemClang::IsObjCObjectOrInterfaceType(
-                    type_clang_forward_type))
-              class_opaque_type = type_clang_forward_type;
-          }
-        }
-
-        if (class_opaque_type) {
-          clang::ObjCMethodDecl *objc_method_decl =
-              m_ast.AddMethodToObjCObjectType(
-                  class_opaque_type, attrs.name.GetCString(), clang_type,
-                  attrs.is_artificial, is_variadic, attrs.is_objc_direct_call);
-          type_handled = objc_method_decl != nullptr;
-          if (type_handled) {
-            LinkDeclContextToDIE(objc_method_decl, die);
-            m_ast.SetMetadataAsUserID(objc_method_decl, die.GetID());
-          } else {
-            dwarf->GetObjectFile()->GetModule()->ReportError(
-                "[{0:x16}]: invalid Objective-C method {1:x4} ({2}), "
-                "please file a bug and attach the file at the start of "
-                "this error message",
-                die.GetOffset(), tag, DW_TAG_value_to_name(tag));
-          }
-        }
+      if (std::optional<const ObjCLanguage::MethodName> objc_method =
+              ObjCLanguage::MethodName::Create(attrs.name.GetStringRef(),
+                                               true)) {
+        type_handled =
+            HandleObjCMethod(*objc_method, die, clang_type, attrs, is_variadic);
       } else if (is_cxx_method) {
-        // Look at the parent of this DIE and see if it is a class or
-        // struct and see if this is actually a C++ method
-        Type *class_type = dwarf->ResolveType(decl_ctx_die);
-        if (class_type) {
-          if (class_type->GetID() != decl_ctx_die.GetID() ||
-              IsClangModuleFwdDecl(decl_ctx_die)) {
-
-            // We uniqued the parent class of this function to another
-            // class so we now need to associate all dies under
-            // "decl_ctx_die" to DIEs in the DIE for "class_type"...
-            DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-            if (class_type_die) {
-              std::vector<DWARFDIE> failures;
-
-              CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
-                                         class_type, failures);
-
-              // FIXME do something with these failures that's
-              // smarter than just dropping them on the ground.
-              // Unfortunately classes don't like having stuff added
-              // to them after their definitions are complete...
-
-              Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
-              if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-                return type_ptr->shared_from_this();
-              }
-            }
-          }
+        auto [handled, type_sp] =
+            HandleCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static,
+                            ignore_containing_context);
+        if (type_sp)
+          return type_sp;
 
-          if (attrs.specification.IsValid()) {
-            // We have a specification which we are going to base our
-            // function prototype off of, so we need this type to be
-            // completed so that the m_die_to_decl_ctx for the method in
-            // the specification has a valid clang decl context.
-            class_type->GetForwardCompilerType();
-            // If we have a specification, then the function type should
-            // have been made with the specification and not with this
-            // die.
-            DWARFDIE spec_die = attrs.specification.Reference();
-            clang::DeclContext *spec_clang_decl_ctx =
-                GetClangDeclContextForDIE(spec_die);
-            if (spec_clang_decl_ctx) {
-              LinkDeclContextToDIE(spec_clang_decl_ctx, die);
-            } else {
-              dwarf->GetObjectFile()->GetModule()->ReportWarning(
-                  "{0:x8}: DW_AT_specification({1:x16}"
-                  ") has no decl\n",
-                  die.GetID(), spec_die.GetOffset());
-            }
-            type_handled = true;
-          } else if (attrs.abstract_origin.IsValid()) {
-            // We have a specification which we are going to base our
-            // function prototype off of, so we need this type to be
-            // completed so that the m_die_to_decl_ctx for the method in
-            // the abstract origin has a valid clang decl context.
-            class_type->GetForwardCompilerType();
-
-            DWARFDIE abs_die = attrs.abstract_origin.Reference();
-            clang::DeclContext *abs_clang_decl_ctx =
-                GetClangDeclContextForDIE(abs_die);
-            if (abs_clang_decl_ctx) {
-              LinkDeclContextToDIE(abs_clang_decl_ctx, die);
-            } else {
-              dwarf->GetObjectFile()->GetModule()->ReportWarning(
-                  "{0:x8}: DW_AT_abstract_origin({1:x16}"
-                  ") has no decl\n",
-                  die.GetID(), abs_die.GetOffset());
-            }
-            type_handled = true;
-          } else {
-            CompilerType class_opaque_type =
-                class_type->GetForwardCompilerType();
-            if (TypeSystemClang::IsCXXClassType(class_opaque_type)) {
-              if (class_opaque_type.IsBeingDefined()) {
-                if (!is_static && !die.HasChildren()) {
-                  // We have a C++ member function with no children (this
-                  // pointer!) and clang will get mad if we try and make
-                  // a function that isn't well formed in the DWARF, so
-                  // we will just skip it...
-                  type_handled = true;
-                } else {
-                  llvm::PrettyStackTraceFormat stack_trace(
-                      "SymbolFileDWARF::ParseType() is adding a method "
-                      "%s to class %s in DIE 0x%8.8" PRIx64 " from %s",
-                      attrs.name.GetCString(),
-                      class_type->GetName().GetCString(), die.GetID(),
-                      dwarf->GetObjectFile()->GetFileSpec().GetPath().c_str());
-
-                  const bool is_attr_used = false;
-                  // Neither GCC 4.2 nor clang++ currently set a valid
-                  // accessibility in the DWARF for C++ methods...
-                  // Default to public for now...
-                  const auto accessibility = attrs.accessibility == eAccessNone
-                                                 ? eAccessPublic
-                                                 : attrs.accessibility;
-
-                  clang::CXXMethodDecl *cxx_method_decl =
-                      m_ast.AddMethodToCXXRecordType(
-                          class_opaque_type.GetOpaqueQualType(),
-                          attrs.name.GetCString(), attrs.mangled_name,
-                          clang_type, accessibility, attrs.is_virtual,
-                          is_static, attrs.is_inline, attrs.is_explicit,
-                          is_attr_used, attrs.is_artificial);
-
-                  type_handled = cxx_method_decl != nullptr;
-                  // Artificial methods are always handled even when we
-                  // don't create a new declaration for them.
-                  type_handled |= attrs.is_artificial;
-
-                  if (cxx_method_decl) {
-                    LinkDeclContextToDIE(cxx_method_decl, die);
-
-                    ClangASTMetadata metadata;
-                    metadata.SetUserID(die.GetID());
-
-                    if (!object_pointer_name.empty()) {
-                      metadata.SetObjectPtrName(object_pointer_name.c_str());
-                      LLDB_LOGF(log,
-                                "Setting object pointer name: %s on method "
-                                "object %p.\n",
-                                object_pointer_name.c_str(),
-                                static_cast<void *>(cxx_method_decl));
-                    }
-                    m_ast.SetMetadata(cxx_method_decl, metadata);
-                  } else {
-                    ignore_containing_context = true;
-                  }
-                }
-              } else {
-                // We were asked to parse the type for a method in a
-                // class, yet the class hasn't been asked to complete
-                // itself through the clang::ExternalASTSource protocol,
-                // so we need to just have the class complete itself and
-                // do things the right way, then our
-                // DIE should then have an entry in the
-                // dwarf->GetDIEToType() map. First
-                // we need to modify the dwarf->GetDIEToType() so it
-                // doesn't think we are trying to parse this DIE
-                // anymore...
-                dwarf->GetDIEToType()[die.GetDIE()] = NULL;
-
-                // Now we get the full type to force our class type to
-                // complete itself using the clang::ExternalASTSource
-                // protocol which will parse all base classes and all
-                // methods (including the method for this DIE).
-                class_type->GetFullCompilerType();
-
-                // The type for this DIE should have been filled in the
-                // function call above.
-                Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
-                if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-                  return type_ptr->shared_from_this();
-                }
-
-                // The previous comment isn't actually true if the class wasn't
-                // resolved using the current method's parent DIE as source
-                // data. We need to ensure that we look up the method correctly
-                // in the class and then link the method's DIE to the unique
-                // CXXMethodDecl appropriately.
-                type_handled = true;
-              }
-            }
-          }
-        }
+        type_handled = handled;
       }
     }
 
@@ -1356,13 +1380,14 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
           ClangASTMetadata metadata;
           metadata.SetUserID(die.GetID());
 
-          if (!object_pointer_name.empty()) {
-            metadata.SetObjectPtrName(object_pointer_name.c_str());
+          char const *object_pointer_name =
+              attrs.object_pointer ? attrs.object_pointer.GetName() : nullptr;
+          if (object_pointer_name) {
+            metadata.SetObjectPtrName(object_pointer_name);
             LLDB_LOGF(log,
                       "Setting object pointer name: %s on function "
                       "object %p.",
-                      object_pointer_name.c_str(),
-                      static_cast<void *>(function_decl));
+                      object_pointer_name, static_cast<void *>(function_decl));
           }
           m_ast.SetMetadata(function_decl, metadata);
         }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 8d4af203bb287..c29a82437c8b1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -23,6 +23,7 @@
 #include "lldb/Core/PluginInterface.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 
 #include <optional>
@@ -370,6 +371,20 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
                          ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParseSubroutine(const lldb_private::plugin::dwarf::DWARFDIE &die,
                                const ParsedDWARFTypeAttributes &attrs);
+
+  bool
+  HandleObjCMethod(lldb_private::ObjCLanguage::MethodName const &objc_method,
+                   lldb_private::plugin::dwarf::DWARFDIE const &die,
+                   lldb_private::CompilerType clang_type,
+                   ParsedDWARFTypeAttributes const &attrs, bool is_variadic);
+
+  std::pair<bool, lldb::TypeSP>
+  HandleCXXMethod(lldb_private::plugin::dwarf::DWARFDIE const &die,
+                  lldb_private::CompilerType clang_type,
+                  ParsedDWARFTypeAttributes const &attrs,
+                  lldb_private::plugin::dwarf::DWARFDIE const &decl_ctx_die,
+                  bool is_static, bool &ignore_containing_context);
+
   lldb::TypeSP ParseArrayType(const lldb_private::plugin::dwarf::DWARFDIE &die,
                               const ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP

>From 7d614613f3bbe29f2fccc25b7d97f7ad812d4067 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Tue, 11 Jun 2024 11:21:32 +0100
Subject: [PATCH 2/4] fixup! fix style, add doxygen comments, rename functions,
 remove PrettyStackTraceFormat, remove confusing comment

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 25 ++++-------
 .../SymbolFile/DWARF/DWARFASTParserClang.h    | 45 +++++++++++++++----
 2 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 585ae4e58cc1a..8141bbaa8ad04 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -975,9 +975,9 @@ ConvertDWARFCallingConventionToClang(const ParsedDWARFTypeAttributes &attrs) {
   return clang::CC_C;
 }
 
-bool DWARFASTParserClang::HandleObjCMethod(
-    ObjCLanguage::MethodName const &objc_method, DWARFDIE const &die,
-    CompilerType clang_type, ParsedDWARFTypeAttributes const &attrs,
+bool DWARFASTParserClang::ParseObjCMethod(
+    const ObjCLanguage::MethodName &objc_method, const DWARFDIE &die,
+    CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs,
     bool is_variadic) {
   SymbolFileDWARF *dwarf = die.GetDWARF();
   const auto tag = die.Tag();
@@ -1020,14 +1020,13 @@ bool DWARFASTParserClang::HandleObjCMethod(
   return true;
 }
 
-std::pair<bool, TypeSP> DWARFASTParserClang::HandleCXXMethod(
-    DWARFDIE const &die, CompilerType clang_type,
-    ParsedDWARFTypeAttributes const &attrs, DWARFDIE const &decl_ctx_die,
+std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod(
+    const DWARFDIE &die, CompilerType clang_type,
+    const ParsedDWARFTypeAttributes &attrs, const DWARFDIE &decl_ctx_die,
     bool is_static, bool &ignore_containing_context) {
   Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
   SymbolFileDWARF *dwarf = die.GetDWARF();
-  // Look at the parent of this DIE and see if it is a class or
-  // struct and see if this is actually a C++ method
+
   Type *class_type = dwarf->ResolveType(decl_ctx_die);
   if (!class_type)
     return {};
@@ -1115,12 +1114,6 @@ std::pair<bool, TypeSP> DWARFASTParserClang::HandleCXXMethod(
       return {true, nullptr};
     }
 
-    llvm::PrettyStackTraceFormat stack_trace(
-        "SymbolFileDWARF::ParseType() is adding a method "
-        "%s to class %s in DIE 0x%8.8" PRIx64 " from %s",
-        attrs.name.GetCString(), class_type->GetName().GetCString(),
-        die.GetID(), dwarf->GetObjectFile()->GetFileSpec().GetPath().c_str());
-
     const bool is_attr_used = false;
     // Neither GCC 4.2 nor clang++ currently set a valid
     // accessibility in the DWARF for C++ methods...
@@ -1283,10 +1276,10 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
               ObjCLanguage::MethodName::Create(attrs.name.GetStringRef(),
                                                true)) {
         type_handled =
-            HandleObjCMethod(*objc_method, die, clang_type, attrs, is_variadic);
+            ParseObjCMethod(*objc_method, die, clang_type, attrs, is_variadic);
       } else if (is_cxx_method) {
         auto [handled, type_sp] =
-            HandleCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static,
+            ParseCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static,
                             ignore_containing_context);
         if (type_sp)
           return type_sp;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index c29a82437c8b1..e8e3ca0b6a368 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -372,18 +372,45 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
   lldb::TypeSP ParseSubroutine(const lldb_private::plugin::dwarf::DWARFDIE &die,
                                const ParsedDWARFTypeAttributes &attrs);
 
+  /// Helper function called by \ref ParseSubroutine when parsing ObjC-methods.
+  ///
+  /// \param[in] objc_method Name of the ObjC method being parsed.
+  /// \param[in] die The DIE that represents the ObjC method being parsed.
+  /// \param[in] clang_type The CompilerType representing the function prototype
+  ///                       of the ObjC method being parsed.
+  /// \param[in] attrs DWARF attributes for \ref die.
+  /// \param[in] is_variadic Is true iff we're parsing a variadic method.
+  ///
+  /// \returns true on success
   bool
-  HandleObjCMethod(lldb_private::ObjCLanguage::MethodName const &objc_method,
-                   lldb_private::plugin::dwarf::DWARFDIE const &die,
-                   lldb_private::CompilerType clang_type,
-                   ParsedDWARFTypeAttributes const &attrs, bool is_variadic);
+  ParseObjCMethod(const lldb_private::ObjCLanguage::MethodName &objc_method,
+                  const lldb_private::plugin::dwarf::DWARFDIE &die,
+                  lldb_private::CompilerType clang_type,
+                  const ParsedDWARFTypeAttributes &attrs, bool is_variadic);
 
+  /// Helper function called by \ref ParseSubroutine when parsing C++ methods.
+  ///
+  /// \param[in] die The DIE that represents the C++ method being parsed.
+  /// \param[in] clang_type The CompilerType representing the function prototype
+  ///                       of the C++ method being parsed.
+  /// \param[in] attrs DWARF attributes for \ref die.
+  /// \param[in] decl_ctx_die The DIE representing the DeclContext of the C++
+  ///                         method being parsed.
+  /// \param[in] is_static Is true iff we're parsing a static method.
+  /// \param[out] ignore_containing_context Will get set to true by this function
+  ///             if the caller should treat this C++ method as-if it was not
+  ///             a C++ method. Currently used as a hack to work around templated
+  ///             C++ methods causing class definitions to mismatch between CUs.
+  ///
+  /// \returns A pair of <bool, TypeSP>. The first element is 'true' on success.
+  ///          The second element is non-null if we have previously parsed this
+  ///          method (a null TypeSP does not indicate failure).
   std::pair<bool, lldb::TypeSP>
-  HandleCXXMethod(lldb_private::plugin::dwarf::DWARFDIE const &die,
-                  lldb_private::CompilerType clang_type,
-                  ParsedDWARFTypeAttributes const &attrs,
-                  lldb_private::plugin::dwarf::DWARFDIE const &decl_ctx_die,
-                  bool is_static, bool &ignore_containing_context);
+  ParseCXXMethod(const lldb_private::plugin::dwarf::DWARFDIE &die,
+                 lldb_private::CompilerType clang_type,
+                 const ParsedDWARFTypeAttributes &attrs,
+                 const lldb_private::plugin::dwarf::DWARFDIE &decl_ctx_die,
+                 bool is_static, bool &ignore_containing_context);
 
   lldb::TypeSP ParseArrayType(const lldb_private::plugin::dwarf::DWARFDIE &die,
                               const ParsedDWARFTypeAttributes &attrs);

>From eb8af81b3320710054ec2214e303a5e5cf2efc25 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Tue, 11 Jun 2024 11:21:43 +0100
Subject: [PATCH 3/4] fixup! clang-format

---
 .../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp      | 2 +-
 .../source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8141bbaa8ad04..5511f2667f4a9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1280,7 +1280,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
       } else if (is_cxx_method) {
         auto [handled, type_sp] =
             ParseCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static,
-                            ignore_containing_context);
+                           ignore_containing_context);
         if (type_sp)
           return type_sp;
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index e8e3ca0b6a368..43e6aa8626160 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -397,10 +397,12 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
   /// \param[in] decl_ctx_die The DIE representing the DeclContext of the C++
   ///                         method being parsed.
   /// \param[in] is_static Is true iff we're parsing a static method.
-  /// \param[out] ignore_containing_context Will get set to true by this function
+  /// \param[out] ignore_containing_context Will get set to true by this
+  /// function
   ///             if the caller should treat this C++ method as-if it was not
-  ///             a C++ method. Currently used as a hack to work around templated
-  ///             C++ methods causing class definitions to mismatch between CUs.
+  ///             a C++ method. Currently used as a hack to work around
+  ///             templated C++ methods causing class definitions to mismatch
+  ///             between CUs.
   ///
   /// \returns A pair of <bool, TypeSP>. The first element is 'true' on success.
   ///          The second element is non-null if we have previously parsed this

>From d39953f663971cf9377767da1922d2354d347b39 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Tue, 11 Jun 2024 11:22:59 +0100
Subject: [PATCH 4/4] fixup! fix comment formatting

---
 .../SymbolFile/DWARF/DWARFASTParserClang.h    | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 43e6aa8626160..136a49e462fbb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -375,10 +375,14 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
   /// Helper function called by \ref ParseSubroutine when parsing ObjC-methods.
   ///
   /// \param[in] objc_method Name of the ObjC method being parsed.
+  ///
   /// \param[in] die The DIE that represents the ObjC method being parsed.
+  ///
   /// \param[in] clang_type The CompilerType representing the function prototype
   ///                       of the ObjC method being parsed.
+  ///
   /// \param[in] attrs DWARF attributes for \ref die.
+  ///
   /// \param[in] is_variadic Is true iff we're parsing a variadic method.
   ///
   /// \returns true on success
@@ -391,18 +395,21 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
   /// Helper function called by \ref ParseSubroutine when parsing C++ methods.
   ///
   /// \param[in] die The DIE that represents the C++ method being parsed.
+  ///
   /// \param[in] clang_type The CompilerType representing the function prototype
   ///                       of the C++ method being parsed.
+  ///
   /// \param[in] attrs DWARF attributes for \ref die.
+  ///
   /// \param[in] decl_ctx_die The DIE representing the DeclContext of the C++
   ///                         method being parsed.
+  ///
   /// \param[in] is_static Is true iff we're parsing a static method.
-  /// \param[out] ignore_containing_context Will get set to true by this
-  /// function
-  ///             if the caller should treat this C++ method as-if it was not
-  ///             a C++ method. Currently used as a hack to work around
-  ///             templated C++ methods causing class definitions to mismatch
-  ///             between CUs.
+  ///
+  /// \param[out] ignore_containing_context Will get set to true if the caller
+  ///             should treat this C++ method as-if it was not a C++ method.
+  ///             Currently used as a hack to work around templated C++ methods
+  ///             causing class definitions to mismatch between CUs.
   ///
   /// \returns A pair of <bool, TypeSP>. The first element is 'true' on success.
   ///          The second element is non-null if we have previously parsed this



More information about the lldb-commits mailing list