[lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext

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

@@ -1272,6 +1272,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
     return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
 
   std::vector<CompilerType> function_param_types;
+  llvm::SmallVector<llvm::StringRef> function_param_names;
 
   // Parse the function children for the parameters
 
@@ -1282,7 +1283,8 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
 
   if (die.HasChildren()) {
     ParseChildParameters(containing_decl_ctx, die, is_variadic,
-                         has_template_params, function_param_types);
+                         has_template_params, function_param_types,
+                         function_param_names);
   }
 
   bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1415,13 +1417,11 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
           const clang::FunctionProtoType *function_prototype(
               llvm::cast<clang::FunctionProtoType>(
                   ClangUtil::GetQualType(clang_type).getTypePtr()));
-          auto params = m_ast.CreateParameterDeclarations(function_decl,
-                                                          *function_prototype);
-          if (!params.empty()) {
-            function_decl->setParams(params);
-            if (template_function_decl)
-              template_function_decl->setParams(params);
-          }
+          const auto params = m_ast.CreateParameterDeclarations(
+              function_decl, *function_prototype, function_param_names);
+          function_decl->setParams(params);
+          if (template_function_decl)
+            template_function_decl->setParams(params);
 
           ClangASTMetadata metadata;
           metadata.SetUserID(die.GetID());
@@ -2382,6 +2382,7 @@ DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   bool is_variadic = false;
   bool has_template_params = false;
   std::vector<CompilerType> param_types;
+  llvm::SmallVector<llvm::StringRef> param_names;
   StreamString sstr;
 
   DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2395,7 +2396,7 @@ DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
       die, GetCXXObjectParameter(die, *containing_decl_ctx));
 
   ParseChildParameters(containing_decl_ctx, die, is_variadic,
-                       has_template_params, param_types);
+                       has_template_params, param_types, param_names);
   sstr << "(";
   for (size_t i = 0; i < param_types.size(); i++) {
     if (i > 0)
@@ -3157,7 +3158,8 @@ bool DWARFASTParserClang::ParseChildMembers(
 void DWARFASTParserClang::ParseChildParameters(
     clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
     bool &is_variadic, bool &has_template_params,
-    std::vector<CompilerType> &function_param_types) {
+    std::vector<CompilerType> &function_param_types,
+    llvm::SmallVector<llvm::StringRef> &function_param_names) {
   if (!parent_die)
     return;
 
@@ -3174,6 +3176,7 @@ void DWARFASTParserClang::ParseChildParameters(
       if (!type)
         break;
 
+      function_param_names.emplace_back(die.GetName());
       function_param_types.push_back(type->GetForwardCompilerType());
     } break;
 
@@ -3196,6 +3199,8 @@ void DWARFASTParserClang::ParseChildParameters(
       break;
     }
   }
+
+  assert(function_param_names.size() == function_param_names.size());
 }
 
 clang::Decl *DWARFASTParserClang::GetClangDeclForDIE(const DWARFDIE &die) {

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

@@ -186,11 +186,12 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
       const lldb::AccessType default_accessibility,
       lldb_private::ClangASTImporter::LayoutInfo &layout_info);
 
-  void
-  ParseChildParameters(clang::DeclContext *containing_decl_ctx,
-                       const lldb_private::plugin::dwarf::DWARFDIE &parent_die,
-                       bool &is_variadic, bool &has_template_params,
-                       std::vector<lldb_private::CompilerType> &function_args);
+  void ParseChildParameters(
+      clang::DeclContext *containing_decl_ctx,
+      const lldb_private::plugin::dwarf::DWARFDIE &parent_die,
+      bool &is_variadic, bool &has_template_params,
+      std::vector<lldb_private::CompilerType> &function_param_types,
+      llvm::SmallVector<llvm::StringRef> &function_param_names);
 
   size_t ParseChildEnumerators(
       const lldb_private::CompilerType &compiler_type, bool is_signed,

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

@@ -7704,14 +7704,20 @@ void TypeSystemClang::SetFloatingInitializerForVariable(
 
 llvm::SmallVector<clang::ParmVarDecl *>
 TypeSystemClang::CreateParameterDeclarations(
-    clang::FunctionDecl *func, const clang::FunctionProtoType &prototype) {
+    clang::FunctionDecl *func, const clang::FunctionProtoType &prototype,
+    const llvm::SmallVector<llvm::StringRef> &parameter_names) {
   assert(func);
+  assert(parameter_names.empty() ||
+         parameter_names.size() == prototype.getNumParams());
 
   llvm::SmallVector<clang::ParmVarDecl *, 12> params;
   for (unsigned param_index = 0; param_index < prototype.getNumParams();
        ++param_index) {
+    llvm::StringRef name =
+        !parameter_names.empty() ? parameter_names[param_index] : "";
+
     auto *param =
-        CreateParameterDeclaration(func, /*owning_module=*/{}, /*name=*/nullptr,
+        CreateParameterDeclaration(func, /*owning_module=*/{}, name.data(),
                                    GetType(prototype.getParamType(param_index)),
                                    clang::SC_None, /*add_decl=*/false);
     assert(param);
@@ -7862,8 +7868,10 @@ clang::CXXMethodDecl *TypeSystemClang::AddMethodToCXXRecordType(
         getASTContext(), mangled_name, /*literal=*/false));
   }
 
-  cxx_method_decl->setParams(
-      CreateParameterDeclarations(cxx_method_decl, *method_function_prototype));
+  // Parameters on member function declarations in DWARF generally don't
+  // have names, so we omit them when creating the ParmVarDecls.
+  cxx_method_decl->setParams(CreateParameterDeclarations(
+      cxx_method_decl, *method_function_prototype, /*parameter_names=*/{}));
 
   AddAccessSpecifierDecl(cxx_record_decl, getASTContext(),
                          GetCXXRecordDeclAccess(cxx_record_decl),

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

@@ -979,12 +979,17 @@ class TypeSystemClang : public TypeSystem {
   /// \param[in] context Non-null \c clang::FunctionDecl which will be the \c
   /// clang::DeclContext of each parameter created/returned by this function.
   /// \param[in] prototype The \c clang::FunctionProtoType of \c context.
+  /// \param[in] param_names The ith element of this vector contains the name
+  /// of the ith parameter. This parameter may be unnamed, in which case the
+  /// ith entry in \c param_names is an empty string. This vector is either
+  /// empty, or will have an entry for *each* parameter of the prototype
+  /// regardless of whether a parameter is unnamed or not.
   ///
   /// \returns A list of newly created of non-null \c clang::ParmVarDecl (one
   /// for each parameter of \c prototype).
-  llvm::SmallVector<clang::ParmVarDecl *>
-  CreateParameterDeclarations(clang::FunctionDecl *context,
-                              const clang::FunctionProtoType &prototype);
+  llvm::SmallVector<clang::ParmVarDecl *> CreateParameterDeclarations(
+      clang::FunctionDecl *context, const clang::FunctionProtoType &prototype,
+      const llvm::SmallVector<llvm::StringRef> &param_names);
 
   clang::CXXMethodDecl *AddMethodToCXXRecordType(
       lldb::opaque_compiler_type_t type, llvm::StringRef name,

lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

@@ -140,7 +140,7 @@ def test_source_and_caret_printing(self):
             """
     1 | foo(1, 2)
       | ^~~
-note: candidate function not viable: requires 1 argument, but 2 were provided
+note: candidate function not viable: requires single argument 'x', but 2 arguments were provided
 """,
             value.GetError().GetCString(),
         )

lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

@@ -1086,6 +1086,9 @@ TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ExplicitObjectParameter) {
 TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ParameterCreation) {
   // Tests parsing of a C++ free function will create clang::ParmVarDecls with
   // the correct clang::DeclContext.
+  //
+  // Also ensures we attach names to the ParmVarDecls (even when DWARF contains
+  // a mix of named/unnamed parameters).
 
   const char *yamldata = R"(
 --- !ELF
@@ -1099,6 +1102,7 @@ TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ParameterCreation) {
     - func
     - int
     - short
+    - namedParam
   debug_abbrev:
     - ID:              0
       Table:
@@ -1131,6 +1135,14 @@ TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ParameterCreation) {
             - Attribute:       DW_AT_type
               Form:            DW_FORM_ref4
         - Code:            0x5
+          Tag:             DW_TAG_formal_parameter
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_type
+              Form:            DW_FORM_ref4
+            - Attribute:       DW_AT_name
+              Form:            DW_FORM_strp
+        - Code:            0x6
           Tag:             DW_TAG_base_type
           Children:        DW_CHILDREN_no
           Attributes:
@@ -1165,13 +1177,15 @@ TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ParameterCreation) {
 #         DW_AT_type [DW_FORM_ref4] (int)
         - AbbrCode:        0x4
           Values:
-            - Value: 0x1f
+            - Value: 0x23
 
 #       DW_TAG_formal_parameter
 #         DW_AT_type [DW_FORM_ref4] (short)
-        - AbbrCode:        0x4
+#         DW_AT_name [DW_FORM_strp] ("namedParam")
+        - AbbrCode:        0x5
           Values:
-            - Value: 0x26
+            - Value: 0x2a
+            - Value: 0xf
 
         - AbbrCode: 0x0
 
@@ -1180,7 +1194,7 @@ TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ParameterCreation) {
 #     DW_AT_encoding  [DW_FORM_data1]
 #     DW_AT_byte_size [DW_FORM_data1]
 
-        - AbbrCode:        0x5
+        - AbbrCode:        0x6
           Values:
             - Value:           0x0000000000000005
             - Value:           0x0000000000000005 # DW_ATE_signed
@@ -1191,7 +1205,7 @@ TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ParameterCreation) {
 #     DW_AT_encoding  [DW_FORM_data1]
 #     DW_AT_byte_size [DW_FORM_data1]
 
-        - AbbrCode:        0x5
+        - AbbrCode:        0x6
           Values:
             - Value:           0x0000000000000009
             - Value:           0x0000000000000005 # DW_ATE_signed
@@ -1230,9 +1244,11 @@ TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ParameterCreation) {
       clang_utils::getDeclarationName(*ts, "func"));
   ASSERT_TRUE(result.isSingleResult());
 
-  auto func = llvm::cast<clang::FunctionDecl>(result.front());
+  auto const *func = llvm::cast<clang::FunctionDecl>(result.front());
 
   EXPECT_EQ(func->getNumParams(), 2U);
   EXPECT_EQ(func->getParamDecl(0)->getDeclContext(), func);
+  EXPECT_TRUE(func->getParamDecl(0)->getName().empty());
   EXPECT_EQ(func->getParamDecl(1)->getDeclContext(), func);
+  EXPECT_EQ(func->getParamDecl(1)->getName(), "namedParam");
 }