[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Simplify ParseChildParameters (PR #123790)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 21 10:02:50 PST 2025
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/123790
>From 505eb69920a291604378bc3ce0adb14ce8843138 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Tue, 21 Jan 2025 17:30:44 +0000
Subject: [PATCH 1/3] [lldb][DWARFASTParserClang][NFCI] Simplify
ParseChildParameters
This patch refactors `ParseChildParameters` in a way which makes it (in my opinion) more readable, removing some redundant local variables in the process and reduces the scope of some variables.
Since `DW_AT_object_pointer`s are now attached to declarations, we can make test for their existence to check whether a C++ method is static or not (whereas currently we're deducing this from `ParseChildParameters` based on some heuristics we know are true for most compilers). So my plan is to move the code for determining `type_quals` and `is_static` out of `ParseChildParameters`. The refactoring in this PR will make this follow-up patch hopefully easier to review.
**Testing**
* This should be NFC. The main changes is that we now no longer iterate over `GetAttributes()` but instead retrieve the name, type and is_artificial attributes of the parameters individually.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 58 +++----------------
1 file changed, 7 insertions(+), 51 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 68fa1d13943a16..4df95513f8dfa2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3084,50 +3084,11 @@ size_t DWARFASTParserClang::ParseChildParameters(
const dw_tag_t tag = die.Tag();
switch (tag) {
case DW_TAG_formal_parameter: {
- DWARFAttributes attributes = die.GetAttributes();
- if (attributes.Size() == 0) {
- arg_idx++;
- break;
- }
-
- const char *name = nullptr;
- DWARFFormValue param_type_die_form;
- bool is_artificial = false;
- // one of None, Auto, Register, Extern, Static, PrivateExtern
-
- clang::StorageClass storage = clang::SC_None;
- uint32_t i;
- for (i = 0; i < attributes.Size(); ++i) {
- const dw_attr_t attr = attributes.AttributeAtIndex(i);
- DWARFFormValue form_value;
- if (attributes.ExtractFormValueAtIndex(i, form_value)) {
- switch (attr) {
- case DW_AT_name:
- name = form_value.AsCString();
- break;
- case DW_AT_type:
- param_type_die_form = form_value;
- break;
- case DW_AT_artificial:
- is_artificial = form_value.Boolean();
- break;
- case DW_AT_location:
- case DW_AT_const_value:
- case DW_AT_default_value:
- case DW_AT_description:
- case DW_AT_endianity:
- case DW_AT_is_optional:
- case DW_AT_segment:
- case DW_AT_variable_parameter:
- default:
- case DW_AT_abstract_origin:
- case DW_AT_sibling:
- break;
- }
- }
- }
+ arg_idx++;
+ const char *name = die.GetName();
+ DWARFDIE param_type_die = die.GetAttributeValueAsReferenceDIE(DW_AT_type);
- if (is_artificial) {
+ if (die.GetAttributeValueAsUnsigned(DW_AT_artificial, 0)) {
// In order to determine if a C++ member function is "const" we
// have to look at the const-ness of "this"...
if (arg_idx == 0 &&
@@ -3136,8 +3097,7 @@ size_t DWARFASTParserClang::ParseChildParameters(
// specification DIEs, so we can't rely upon the name being in
// the formal parameter DIE...
(name == nullptr || ::strcmp(name, "this") == 0)) {
- Type *this_type = die.ResolveTypeUID(param_type_die_form.Reference());
- if (this_type) {
+ if (Type *this_type = die.ResolveTypeUID(param_type_die)) {
uint32_t encoding_mask = this_type->GetEncodingMask();
if (encoding_mask & Type::eEncodingIsPointerUID) {
is_static = false;
@@ -3149,21 +3109,17 @@ size_t DWARFASTParserClang::ParseChildParameters(
}
}
}
- } else {
- Type *type = die.ResolveTypeUID(param_type_die_form.Reference());
- if (type) {
+ } else if (Type *type = die.ResolveTypeUID(param_type_die)) {
function_param_types.push_back(type->GetForwardCompilerType());
clang::ParmVarDecl *param_var_decl = m_ast.CreateParameterDeclaration(
containing_decl_ctx, GetOwningClangModule(die), name,
- type->GetForwardCompilerType(), storage);
+ type->GetForwardCompilerType(), clang::StorageClass::SC_None);
assert(param_var_decl);
function_param_decls.push_back(param_var_decl);
m_ast.SetMetadataAsUserID(param_var_decl, die.GetID());
- }
}
- arg_idx++;
} break;
case DW_TAG_unspecified_parameters:
>From c05a011cb9272563e68f0c5d6e2560a9b74aec2b Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Tue, 21 Jan 2025 17:47:07 +0000
Subject: [PATCH 2/3] fixup! clang-format
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 4df95513f8dfa2..677b8884c781cc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3110,15 +3110,15 @@ size_t DWARFASTParserClang::ParseChildParameters(
}
}
} else if (Type *type = die.ResolveTypeUID(param_type_die)) {
- function_param_types.push_back(type->GetForwardCompilerType());
+ function_param_types.push_back(type->GetForwardCompilerType());
- clang::ParmVarDecl *param_var_decl = m_ast.CreateParameterDeclaration(
- containing_decl_ctx, GetOwningClangModule(die), name,
- type->GetForwardCompilerType(), clang::StorageClass::SC_None);
- assert(param_var_decl);
- function_param_decls.push_back(param_var_decl);
+ clang::ParmVarDecl *param_var_decl = m_ast.CreateParameterDeclaration(
+ containing_decl_ctx, GetOwningClangModule(die), name,
+ type->GetForwardCompilerType(), clang::StorageClass::SC_None);
+ assert(param_var_decl);
+ function_param_decls.push_back(param_var_decl);
- m_ast.SetMetadataAsUserID(param_var_decl, die.GetID());
+ m_ast.SetMetadataAsUserID(param_var_decl, die.GetID());
}
} break;
>From f32a46755b6eee07ff0fb147d61824d91533ac5f Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Tue, 21 Jan 2025 18:02:11 +0000
Subject: [PATCH 3/3] fixup! increment argument index at the end of the block
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 677b8884c781cc..81a1375c037182 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3084,7 +3084,6 @@ size_t DWARFASTParserClang::ParseChildParameters(
const dw_tag_t tag = die.Tag();
switch (tag) {
case DW_TAG_formal_parameter: {
- arg_idx++;
const char *name = die.GetName();
DWARFDIE param_type_die = die.GetAttributeValueAsReferenceDIE(DW_AT_type);
@@ -3120,6 +3119,8 @@ size_t DWARFASTParserClang::ParseChildParameters(
m_ast.SetMetadataAsUserID(param_var_decl, die.GetID());
}
+
+ arg_idx++;
} break;
case DW_TAG_unspecified_parameters:
More information about the lldb-commits
mailing list