[Lldb-commits] [lldb] 23fd8f6 - [lldb][DWARFASTParserClang][NFCI] Simplify ParseChildParameters (#123790)

[via lldb-commits]

Author: Michael Buch
Date: 2025-01-22T09:21:16Z
New Revision: 23fd8f6f263d14d43fd4b2a599652beaadc9920e

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

LOG: [lldb][DWARFASTParserClang][NFCI] Simplify ParseChildParameters (#123790)

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.

**Motivation**

Since `DW_AT_object_pointer`s are now attached to declarations, we can
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 change is that we now no longer iterate
over `GetAttributes()` but instead retrieve the name, type and
is_artificial attributes of the parameters individually.

Added: 
    

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 68fa1d13943a16..81a1375c037182 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3084,50 +3084,10 @@ 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;
-          }
-        }
-      }
+      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 +3096,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,20 +3108,18 @@ size_t DWARFASTParserClang::ParseChildParameters(
             }
           }
         }
-      } else {
-        Type *type = die.ResolveTypeUID(param_type_die_form.Reference());
-        if (type) {
-          function_param_types.push_back(type->GetForwardCompilerType());
+      } 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);
-          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());
       }
+
       arg_idx++;
     } break;