[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Mon Jan 27 03:50:22 PST 2025
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/124279
>From 23c7f4a3e067739a050964b3f1c2140787b0c31f Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 24 Jan 2025 13:33:07 +0000
Subject: [PATCH 1/5] [lldb][TypeSystem] Ensure that ParmVarDecls have the
correct DeclContext
While sifting through this part of the code I noticed that when we parse C++ methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in `ParseChildParameters` and once in `AddMethodToCXXRecordType`. The former is unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created in `ParseChildParameters` were created with an incorrect `clang::DeclContext` (namely the DeclContext of the function, and not the function itself). In Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a parameter if the parameter was created before the FunctionDecl. But we never used it.
This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures we set the DeclContext correctly.
This wasn't causing any concrete issues (that I know of), but was quite surprising. And this way of setting the parameters seems easier to reason about (in my opinion).
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 31 ++--
.../SymbolFile/DWARF/DWARFASTParserClang.h | 3 +-
.../SymbolFile/NativePDB/PdbAstBuilder.cpp | 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 4 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 43 ++---
.../TypeSystem/Clang/TypeSystemClang.h | 16 +-
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 +++++
.../DWARF/DWARFASTParserClangTests.cpp | 154 ++++++++++++++++++
8 files changed, 247 insertions(+), 48 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..30dec4bbf91c10 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
std::vector<CompilerType> function_param_types;
- std::vector<clang::ParmVarDecl *> function_param_decls;
// Parse the function children for the parameters
@@ -1283,8 +1282,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
if (die.HasChildren()) {
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, function_param_types,
- function_param_decls);
+ has_template_params, function_param_types);
}
bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
LinkDeclContextToDIE(function_decl, die);
- if (!function_param_decls.empty()) {
- m_ast.SetFunctionParameters(function_decl, function_param_decls);
+ 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)
- m_ast.SetFunctionParameters(template_function_decl,
- function_param_decls);
+ template_function_decl->setParams(params);
}
ClangASTMetadata metadata;
@@ -2380,7 +2382,6 @@ DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
bool is_variadic = false;
bool has_template_params = false;
std::vector<CompilerType> param_types;
- std::vector<clang::ParmVarDecl *> param_decls;
StreamString sstr;
DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2394,7 +2395,7 @@ DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
die, GetCXXObjectParameter(die, *containing_decl_ctx));
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, param_types, param_decls);
+ has_template_params, param_types);
sstr << "(";
for (size_t i = 0; i < param_types.size(); i++) {
if (i > 0)
@@ -3156,8 +3157,7 @@ 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<clang::ParmVarDecl *> &function_param_decls) {
+ std::vector<CompilerType> &function_param_types) {
if (!parent_die)
return;
@@ -3168,7 +3168,6 @@ void DWARFASTParserClang::ParseChildParameters(
if (die.GetAttributeValueAsUnsigned(DW_AT_artificial, 0))
continue;
- const char *name = die.GetName();
DWARFDIE param_type_die = die.GetAttributeValueAsReferenceDIE(DW_AT_type);
Type *type = die.ResolveTypeUID(param_type_die);
@@ -3176,14 +3175,6 @@ void DWARFASTParserClang::ParseChildParameters(
break;
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);
-
- m_ast.SetMetadataAsUserID(param_var_decl, die.GetID());
} break;
case DW_TAG_unspecified_parameters:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index a5c3746ada4c36..b1f0186cfa34a2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -190,8 +190,7 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
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,
- std::vector<clang::ParmVarDecl *> &function_param_decls);
+ std::vector<lldb_private::CompilerType> &function_args);
size_t ParseChildEnumerators(
const lldb_private::CompilerType &compiler_type, bool is_signed,
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 0c71df625ae348..5d4b22d08b1110 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -1137,7 +1137,7 @@ void PdbAstBuilder::CreateFunctionParameters(PdbCompilandSymId func_id,
}
if (!params.empty() && params.size() == param_count)
- m_clang.SetFunctionParameters(&function_decl, params);
+ function_decl.setParams(params);
}
clang::QualType PdbAstBuilder::CreateEnumType(PdbTypeSymId id,
diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index fa3530a0c22ff3..990bacd89bf346 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -975,8 +975,8 @@ PDBASTParser::GetDeclForSymbol(const llvm::pdb::PDBSymbol &symbol) {
}
}
}
- if (params.size())
- m_ast.SetFunctionParameters(decl, params);
+ if (params.size() && decl)
+ decl->setParams(params);
m_uid_to_decl[sym_id] = decl;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 47051f2e68090f..1bb4bdf14c49d4 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2217,12 +2217,6 @@ ParmVarDecl *TypeSystemClang::CreateParameterDeclaration(
return decl;
}
-void TypeSystemClang::SetFunctionParameters(
- FunctionDecl *function_decl, llvm::ArrayRef<ParmVarDecl *> params) {
- if (function_decl)
- function_decl->setParams(params);
-}
-
CompilerType
TypeSystemClang::CreateBlockPointerType(const CompilerType &function_type) {
QualType block_type = m_ast_up->getBlockPointerType(
@@ -7708,6 +7702,26 @@ void TypeSystemClang::SetFloatingInitializerForVariable(
ast, init_value, true, qt.getUnqualifiedType(), SourceLocation()));
}
+llvm::SmallVector<clang::ParmVarDecl *>
+TypeSystemClang::CreateParameterDeclarations(
+ clang::FunctionDecl *func, const clang::FunctionProtoType &prototype) {
+ assert(func);
+
+ llvm::SmallVector<clang::ParmVarDecl *, 12> params;
+ for (unsigned param_index = 0; param_index < prototype.getNumParams();
+ ++param_index) {
+ auto *param =
+ CreateParameterDeclaration(func, /*owning_module=*/{}, /*name=*/nullptr,
+ GetType(prototype.getParamType(param_index)),
+ clang::SC_None, /*add_decl=*/false);
+ assert(param);
+
+ params.push_back(param);
+ }
+
+ return params;
+}
+
clang::CXXMethodDecl *TypeSystemClang::AddMethodToCXXRecordType(
lldb::opaque_compiler_type_t type, llvm::StringRef name,
const char *mangled_name, const CompilerType &method_clang_type,
@@ -7848,20 +7862,9 @@ clang::CXXMethodDecl *TypeSystemClang::AddMethodToCXXRecordType(
getASTContext(), mangled_name, /*literal=*/false));
}
- // Populate the method decl with parameter decls
-
- llvm::SmallVector<clang::ParmVarDecl *, 12> params;
-
- for (unsigned param_index = 0; param_index < num_params; ++param_index) {
- params.push_back(clang::ParmVarDecl::Create(
- getASTContext(), cxx_method_decl, clang::SourceLocation(),
- clang::SourceLocation(),
- nullptr, // anonymous
- method_function_prototype->getParamType(param_index), nullptr,
- clang::SC_None, nullptr));
- }
-
- cxx_method_decl->setParams(llvm::ArrayRef<clang::ParmVarDecl *>(params));
+ auto params =
+ CreateParameterDeclarations(cxx_method_decl, *method_function_prototype);
+ cxx_method_decl->setParams(params);
AddAccessSpecifierDecl(cxx_record_decl, getASTContext(),
GetCXXRecordDeclAccess(cxx_record_decl),
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 678eaed381fd49..02b1a1db98e70f 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -489,9 +489,6 @@ class TypeSystemClang : public TypeSystem {
const char *name, const CompilerType ¶m_type,
int storage, bool add_decl = false);
- void SetFunctionParameters(clang::FunctionDecl *function_decl,
- llvm::ArrayRef<clang::ParmVarDecl *> params);
-
CompilerType CreateBlockPointerType(const CompilerType &function_type);
// Array Types
@@ -976,6 +973,19 @@ class TypeSystemClang : public TypeSystem {
SetFloatingInitializerForVariable(clang::VarDecl *var,
const llvm::APFloat &init_value);
+ /// For each parameter type of \c prototype, creates a \c clang::ParmVarDecl
+ /// whose \c clang::DeclContext is \c context.
+ ///
+ /// \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.
+ ///
+ /// \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);
+
clang::CXXMethodDecl *AddMethodToCXXRecordType(
lldb::opaque_compiler_type_t type, llvm::StringRef name,
const char *mangled_name, const CompilerType &method_type,
diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index a2d1f6db802777..23374062127e0e 100644
--- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -1040,3 +1040,45 @@ TEST_F(TestTypeSystemClang, GetDeclContextByNameWhenMissingSymbolFile) {
EXPECT_TRUE(decls.empty());
}
+
+TEST_F(TestTypeSystemClang, AddMethodToCXXRecordType_ParmVarDecls) {
+ // Tests that AddMethodToCXXRecordType creates ParmVarDecl's with
+ // a correct clang::DeclContext.
+
+ llvm::StringRef class_name = "S";
+ CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+ m_ast->StartTagDeclarationDefinition(t);
+
+ CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+ const bool is_virtual = false;
+ const bool is_static = false;
+ const bool is_inline = false;
+ const bool is_explicit = true;
+ const bool is_attr_used = false;
+ const bool is_artificial = false;
+
+ llvm::SmallVector<CompilerType> param_types{
+ m_ast->GetBasicType(lldb::eBasicTypeInt),
+ m_ast->GetBasicType(lldb::eBasicTypeShort)};
+ CompilerType function_type = m_ast->CreateFunctionType(
+ return_type, param_types.data(), /*num_params*/ param_types.size(),
+ /*variadic=*/false, /*quals*/ 0U);
+ m_ast->AddMethodToCXXRecordType(
+ t.GetOpaqueQualType(), "myFunc", nullptr, function_type,
+ lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+ is_explicit, is_attr_used, is_artificial);
+
+ // Complete the definition and check the created record.
+ m_ast->CompleteTagDeclarationDefinition(t);
+
+ auto *record = llvm::cast<CXXRecordDecl>(ClangUtil::GetAsTagDecl(t));
+
+ auto method_it = record->method_begin();
+ ASSERT_NE(method_it, record->method_end());
+
+ EXPECT_EQ(method_it->getNumParams(), param_types.size());
+
+ // DeclContext of each parameter should be the CXXMethodDecl itself.
+ EXPECT_EQ(method_it->getParamDecl(0)->getDeclContext(), *method_it);
+ EXPECT_EQ(method_it->getParamDecl(1)->getDeclContext(), *method_it);
+}
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
index 8adda6fba3a0b0..21da219c1a1468 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -1082,3 +1082,157 @@ TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ExplicitObjectParameter) {
clang::Qualifiers::fromCVRMask(clang::Qualifiers::Const |
clang::Qualifiers::Volatile));
}
+
+TEST_F(DWARFASTParserClangTests, TestParseSubroutine_ParameterCreation) {
+ // Tests parsing of a C++ free function will create clang::ParmVarDecls with
+ // the correct clang::DeclContext.
+
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+DWARF:
+ debug_str:
+ - func
+ - int
+ - short
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Code: 0x2
+ Tag: DW_TAG_structure_type
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_strp
+ - Code: 0x3
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_strp
+ - Attribute: DW_AT_declaration
+ Form: DW_FORM_flag_present
+ - Attribute: DW_AT_external
+ Form: DW_FORM_flag_present
+ - Code: 0x4
+ Tag: DW_TAG_formal_parameter
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref4
+ - Code: 0x5
+ Tag: DW_TAG_base_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_strp
+ - Attribute: DW_AT_encoding
+ Form: DW_FORM_data1
+ - Attribute: DW_AT_byte_size
+ Form: DW_FORM_data1
+ debug_info:
+ - Version: 5
+ UnitType: DW_UT_compile
+ AddrSize: 8
+ Entries:
+
+# DW_TAG_compile_unit
+# DW_AT_language [DW_FORM_data2] (DW_LANG_C_plus_plus)
+
+ - AbbrCode: 0x1
+ Values:
+ - Value: 0x04
+
+# DW_TAG_subprogram
+# DW_AT_name [DW_FORM_strp] ("func")
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0x0
+ - Value: 0x1
+ - Value: 0x1
+
+# DW_TAG_formal_parameter
+# DW_AT_type [DW_FORM_ref4] (int)
+ - AbbrCode: 0x4
+ Values:
+ - Value: 0x1f
+
+# DW_TAG_formal_parameter
+# DW_AT_type [DW_FORM_ref4] (short)
+ - AbbrCode: 0x4
+ Values:
+ - Value: 0x26
+
+ - AbbrCode: 0x0
+
+# DW_TAG_base_type
+# DW_AT_name [DW_FORM_strp] ("int")
+# DW_AT_encoding [DW_FORM_data1]
+# DW_AT_byte_size [DW_FORM_data1]
+
+ - AbbrCode: 0x5
+ Values:
+ - Value: 0x0000000000000005
+ - Value: 0x0000000000000005 # DW_ATE_signed
+ - Value: 0x0000000000000004
+
+# DW_TAG_base_type
+# DW_AT_name [DW_FORM_strp] ("short")
+# DW_AT_encoding [DW_FORM_data1]
+# DW_AT_byte_size [DW_FORM_data1]
+
+ - AbbrCode: 0x5
+ Values:
+ - Value: 0x0000000000000009
+ - Value: 0x0000000000000005 # DW_ATE_signed
+ - Value: 0x0000000000000004
+
+ - AbbrCode: 0x0
+...
+)";
+ YAMLModuleTester t(yamldata);
+
+ DWARFUnit *unit = t.GetDwarfUnit();
+ ASSERT_NE(unit, nullptr);
+ const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+ ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+ ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+ DWARFDIE cu_die(unit, cu_entry);
+
+ auto ts_or_err =
+ cu_die.GetDWARF()->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
+ ASSERT_TRUE(static_cast<bool>(ts_or_err));
+ llvm::consumeError(ts_or_err.takeError());
+
+ auto *ts = static_cast<TypeSystemClang *>(ts_or_err->get());
+ auto *parser = static_cast<DWARFASTParserClang *>(ts->GetDWARFParser());
+
+ auto subprogram = cu_die.GetFirstChild();
+ ASSERT_TRUE(subprogram.IsValid());
+ ASSERT_EQ(subprogram.Tag(), DW_TAG_subprogram);
+
+ SymbolContext sc;
+ bool new_type;
+ auto type_sp = parser->ParseTypeFromDWARF(sc, subprogram, &new_type);
+ ASSERT_NE(type_sp, nullptr);
+
+ auto result = ts->GetTranslationUnitDecl()->lookup(
+ clang_utils::getDeclarationName(*ts, "func"));
+ ASSERT_TRUE(result.isSingleResult());
+
+ auto func = llvm::cast<clang::FunctionDecl>(result.front());
+
+ EXPECT_EQ(func->getNumParams(), 2U);
+ EXPECT_EQ(func->getParamDecl(0)->getDeclContext(), func);
+ EXPECT_EQ(func->getParamDecl(1)->getDeclContext(), func);
+}
>From 12903dfd6a044ec9a079077123e2d023777c0e08 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 24 Jan 2025 16:09:08 +0000
Subject: [PATCH 2/5] fixup! adjust test assertion
---
.../API/commands/expression/diagnostics/TestExprDiagnostics.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index b476a807c6dc0e..5c6eb94f3f1e85 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/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 single argument 'x', but 2 arguments were provided
+note: candidate function not viable: requires 1 argument, but 2 arguments were provided
""",
value.GetError().GetCString(),
)
>From a08e40f7e242adebd1973906882a24441cf3c21d Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 24 Jan 2025 16:26:39 +0000
Subject: [PATCH 3/5] fixup! remove redundant temporary variable
---
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 1bb4bdf14c49d4..2e973e7ea299c5 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7862,9 +7862,8 @@ clang::CXXMethodDecl *TypeSystemClang::AddMethodToCXXRecordType(
getASTContext(), mangled_name, /*literal=*/false));
}
- auto params =
- CreateParameterDeclarations(cxx_method_decl, *method_function_prototype);
- cxx_method_decl->setParams(params);
+ cxx_method_decl->setParams(
+ CreateParameterDeclarations(cxx_method_decl, *method_function_prototype));
AddAccessSpecifierDecl(cxx_record_decl, getASTContext(),
GetCXXRecordDeclAccess(cxx_record_decl),
>From 68406b60b61d36a8810e30c5a805dd2ea3dac4ac Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 24 Jan 2025 20:19:13 +0000
Subject: [PATCH 4/5] fixup! fix test assertion
---
.../API/commands/expression/diagnostics/TestExprDiagnostics.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index 5c6eb94f3f1e85..962d2e321de91f 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/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 arguments were provided
+note: candidate function not viable: requires 1 argument, but 2 were provided
""",
value.GetError().GetCString(),
)
>From f397767c273d87efac02197d60df062da3f40067 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 27 Jan 2025 11:42:39 +0000
Subject: [PATCH 5/5] fixup! attach parameter names if they exist in DWARF
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 25 ++++++++++-------
.../SymbolFile/DWARF/DWARFASTParserClang.h | 11 ++++----
.../TypeSystem/Clang/TypeSystemClang.cpp | 16 ++++++++---
.../TypeSystem/Clang/TypeSystemClang.h | 11 ++++++--
.../diagnostics/TestExprDiagnostics.py | 2 +-
.../DWARF/DWARFASTParserClangTests.cpp | 28 +++++++++++++++----
6 files changed, 64 insertions(+), 29 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 30dec4bbf91c10..747b71437c2a41 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/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) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index b1f0186cfa34a2..4411c0e2f63016 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/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,
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 2e973e7ea299c5..fc3dbfa311c9b8 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/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> ¶meter_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),
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 02b1a1db98e70f..83f954270e3093 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/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> ¶m_names);
clang::CXXMethodDecl *AddMethodToCXXRecordType(
lldb::opaque_compiler_type_t type, llvm::StringRef name,
diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
index 962d2e321de91f..b476a807c6dc0e 100644
--- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ b/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(),
)
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
index 21da219c1a1468..6c77736113da3f 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ b/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");
}
More information about the lldb-commits
mailing list