Skip to content

[lldb][DWARFASTParserClang] GetCXXObjectParameter to take DeclContext DIE parameter #144876

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

Conversation

Michael137
Copy link
Member

I'm trying to call GetCXXObjectParameter from unit-tests in a follow-up patch and taking a DWARFDIE instead of clang::DeclContext makes that much simpler. These should be equivalent, since all we're trying to check is that the parent context is a record type.

… DIE parameter

I'm trying to call `GetCXXObjectParameter` from unit-tests in a
follow-up patch and taking a `DWARFDIE` instead of `clang::DeclContext`
makes that much simpler. These should be equivalent, since all we're
trying to check is that the parent context is a record type.
@Michael137 Michael137 requested a review from labath June 19, 2025 11:17
@Michael137 Michael137 requested a review from JDevlieghere as a code owner June 19, 2025 11:17
@llvmbot llvmbot added the lldb label Jun 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

I'm trying to call GetCXXObjectParameter from unit-tests in a follow-up patch and taking a DWARFDIE instead of clang::DeclContext makes that much simpler. These should be equivalent, since all we're trying to check is that the parent context is a record type.


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

1 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+9-9)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 620501b304e63..7fc1d70898d1d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -163,14 +163,14 @@ static bool TagIsRecordType(dw_tag_t tag) {
 /// a default DWARFDIE. If \c containing_decl_ctx is not a valid
 /// C++ declaration context for class methods, assume no object
 /// parameter exists for the given \c subprogram.
-static DWARFDIE
-GetCXXObjectParameter(const DWARFDIE &subprogram,
-                      const clang::DeclContext &containing_decl_ctx) {
+static DWARFDIE GetCXXObjectParameter(const DWARFDIE &subprogram,
+                                      const DWARFDIE &decl_ctx_die) {
+  assert(subprogram);
   assert(subprogram.Tag() == DW_TAG_subprogram ||
          subprogram.Tag() == DW_TAG_inlined_subroutine ||
          subprogram.Tag() == DW_TAG_subroutine_type);
 
-  if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+  if (!decl_ctx_die.IsStructUnionOrClass())
     return {};
 
   if (DWARFDIE object_parameter =
@@ -1304,8 +1304,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
   clang::CallingConv calling_convention =
       ConvertDWARFCallingConventionToClang(attrs);
 
-  const DWARFDIE object_parameter =
-      GetCXXObjectParameter(die, *containing_decl_ctx);
+  const DWARFDIE object_parameter = GetCXXObjectParameter(die, decl_ctx_die);
 
   // clang_type will get the function prototype clang type after this
   // call
@@ -2411,12 +2410,13 @@ DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
   sstr << decl_ctx.GetQualifiedName();
 
+  DWARFDIE decl_ctx_die;
   clang::DeclContext *containing_decl_ctx =
-      GetClangDeclContextContainingDIE(die, nullptr);
+      GetClangDeclContextContainingDIE(die, &decl_ctx_die);
   assert(containing_decl_ctx);
 
-  const unsigned cv_quals = GetCXXMethodCVQuals(
-      die, GetCXXObjectParameter(die, *containing_decl_ctx));
+  const unsigned cv_quals =
+      GetCXXMethodCVQuals(die, GetCXXObjectParameter(die, decl_ctx_die));
 
   ParseChildParameters(containing_decl_ctx, die, is_variadic,
                        has_template_params, param_types, param_names);

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Jun 19, 2025
…l it from unit-tests

My goal is to remove the `object_pointer` member on
`ParsedDWARFTypeAttributes` since it's duplicating information that we
retrieve with `GetCXXObjectParameter` anyway. To continue having
coverage for the `DW_AT_object_pointer` code-paths, instead of checking the
`attrs.object_pointer` I'm now calling `GetCXXObjectParameter` directly.
We could find some very roundabout way to go via the Clang AST to check
that the object parameter was parsed correctly, but that quickly became
quite painful.

Depends on llvm#144876
@Michael137 Michael137 merged commit 30824c4 into llvm:main Jun 19, 2025
9 checks passed
@Michael137 Michael137 deleted the lldb/cxx-object-parameter-changes-1 branch June 19, 2025 11:48
Michael137 added a commit that referenced this pull request Jun 19, 2025
…l it from unit-tests (#144879)

My goal is to remove the `object_pointer` member on
`ParsedDWARFTypeAttributes` since it's duplicating information that we
retrieve with `GetCXXObjectParameter` anyway. To continue having
coverage for the `DW_AT_object_pointer` code-paths, instead of checking
the
`attrs.object_pointer` I'm now calling `GetCXXObjectParameter` directly.
We could find some very roundabout way to go via the Clang AST to check
that the object parameter was parsed correctly, but that quickly became
quite painful.

Depends on #144876
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 19, 2025
…lic and call it from unit-tests (#144879)

My goal is to remove the `object_pointer` member on
`ParsedDWARFTypeAttributes` since it's duplicating information that we
retrieve with `GetCXXObjectParameter` anyway. To continue having
coverage for the `DW_AT_object_pointer` code-paths, instead of checking
the
`attrs.object_pointer` I'm now calling `GetCXXObjectParameter` directly.
We could find some very roundabout way to go via the Clang AST to check
that the object parameter was parsed correctly, but that quickly became
quite painful.

Depends on llvm/llvm-project#144876
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