[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
This revision was automatically updated to reflect the committed changes. Closed by commit rG4354dfbdf5c8: Preserve the owning module information from DWARF in the synthesized AST (authored by aprantl). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D75488?vs=254027=254383#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/include/lldb/Symbol/CompilerType.h lldb/include/lldb/Symbol/TypeSystem.h lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/Symbol/CompilerType.cpp lldb/source/Symbol/Type.cpp lldb/source/Symbol/TypeSystem.cpp lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm lldb/unittests/Symbol/TestTypeSystemClang.cpp lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h === --- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h +++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h @@ -28,8 +28,8 @@ inline CompilerType createRecord(TypeSystemClang , llvm::StringRef name) { return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), - lldb::AccessType::eAccessPublic, name, 0, - lldb::LanguageType::eLanguageTypeC); + OptionalClangModuleID(), lldb::AccessType::eAccessPublic, name, + 0, lldb::LanguageType::eLanguageTypeC); } /// Create a record with the given name and a field with the given type Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp === --- lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -14,6 +14,7 @@ #include "lldb/Host/HostInfo.h" #include "lldb/Symbol/Declaration.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclObjC.h" #include "clang/AST/ExprCXX.h" #include "gtest/gtest.h" @@ -257,9 +258,10 @@ CompilerType basic_compiler_type = ast.GetBasicType(basic_type); EXPECT_TRUE(basic_compiler_type.IsValid()); - CompilerType enum_type = - ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), -Declaration(), basic_compiler_type, scoped); + CompilerType enum_type = ast.CreateEnumerationType( + "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(), Declaration(), + basic_compiler_type, scoped); + CompilerType t = ast.GetEnumerationIntegerType(enum_type); // Check that the type we put in at the start is found again. EXPECT_EQ(basic_compiler_type.GetTypeName(), t.GetTypeName()); @@ -267,14 +269,38 @@ } } +TEST_F(TestTypeSystemClang, TestOwningModule) { + TypeSystemClang ast("module_ast", HostInfo::GetTargetTriple()); + CompilerType basic_compiler_type = ast.GetBasicType(BasicType::eBasicTypeInt); + CompilerType enum_type = ast.CreateEnumerationType( + "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(100), Declaration(), + basic_compiler_type, false); + auto *ed = TypeSystemClang::GetAsEnumDecl(enum_type); + EXPECT_FALSE(!ed); + EXPECT_EQ(ed->getOwningModuleID(), 100u); + + CompilerType record_type = ast.CreateRecordType( + nullptr, OptionalClangModuleID(200), lldb::eAccessPublic, "FooRecord", + clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); + auto *rd = TypeSystemClang::GetAsRecordDecl(record_type); + EXPECT_FALSE(!rd); + EXPECT_EQ(rd->getOwningModuleID(), 200u); + + CompilerType class_type = ast.CreateObjCClass( + "objc_class", ast.GetTranslationUnitDecl(), OptionalClangModuleID(300), false, false); + auto *cd = TypeSystemClang::GetAsObjCInterfaceDecl(class_type); + EXPECT_FALSE(!cd); +
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. Ok, no more complaints from my side. LGTM Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1256 + name, + parent_desc ? const_cast(parent_desc->getModuleOrNull()) + : nullptr, aprantl wrote: > here ^^ True, it seems we can't fix that. Also the ExternalASTSource uses non-const modules, so I guess that's fine then. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl marked an inline comment as done. aprantl added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1260 + + module->Name = name.str(); + return ast_source->RegisterModule(module); teemperor wrote: > Why is that done? The module should already have the correct name when it is > returned from findOrCreateModule (where we pass the same name and then create > the new module with that name). > > If this line is removed, do we still need D75561? Good catch! This is a leftover from my first version of the patch where I created the Module object manually without instantiating a clang::ModuleMap. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1256 + name, + parent_desc ? const_cast(parent_desc->getModuleOrNull()) + : nullptr, here ^^ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 254027. aprantl marked an inline comment as done. aprantl added a comment. (Made const_cast explicit to illustrate mismatch.) However, I still need to de-constify in order to create the module. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/include/lldb/Symbol/CompilerType.h lldb/include/lldb/Symbol/TypeSystem.h lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/Symbol/CompilerType.cpp lldb/source/Symbol/Type.cpp lldb/source/Symbol/TypeSystem.cpp lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm lldb/unittests/Symbol/TestTypeSystemClang.cpp lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h === --- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h +++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h @@ -28,8 +28,8 @@ inline CompilerType createRecord(TypeSystemClang , llvm::StringRef name) { return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), - lldb::AccessType::eAccessPublic, name, 0, - lldb::LanguageType::eLanguageTypeC); + OptionalClangModuleID(), lldb::AccessType::eAccessPublic, name, + 0, lldb::LanguageType::eLanguageTypeC); } /// Create a record with the given name and a field with the given type Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp === --- lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -14,6 +14,7 @@ #include "lldb/Host/HostInfo.h" #include "lldb/Symbol/Declaration.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclObjC.h" #include "clang/AST/ExprCXX.h" #include "gtest/gtest.h" @@ -257,9 +258,10 @@ CompilerType basic_compiler_type = ast.GetBasicType(basic_type); EXPECT_TRUE(basic_compiler_type.IsValid()); - CompilerType enum_type = - ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), -Declaration(), basic_compiler_type, scoped); + CompilerType enum_type = ast.CreateEnumerationType( + "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(), Declaration(), + basic_compiler_type, scoped); + CompilerType t = ast.GetEnumerationIntegerType(enum_type); // Check that the type we put in at the start is found again. EXPECT_EQ(basic_compiler_type.GetTypeName(), t.GetTypeName()); @@ -267,14 +269,38 @@ } } +TEST_F(TestTypeSystemClang, TestOwningModule) { + TypeSystemClang ast("module_ast", HostInfo::GetTargetTriple()); + CompilerType basic_compiler_type = ast.GetBasicType(BasicType::eBasicTypeInt); + CompilerType enum_type = ast.CreateEnumerationType( + "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(100), Declaration(), + basic_compiler_type, false); + auto *ed = TypeSystemClang::GetAsEnumDecl(enum_type); + EXPECT_FALSE(!ed); + EXPECT_EQ(ed->getOwningModuleID(), 100u); + + CompilerType record_type = ast.CreateRecordType( + nullptr, OptionalClangModuleID(200), lldb::eAccessPublic, "FooRecord", + clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); + auto *rd = TypeSystemClang::GetAsRecordDecl(record_type); + EXPECT_FALSE(!rd); + EXPECT_EQ(rd->getOwningModuleID(), 200u); + + CompilerType class_type = ast.CreateObjCClass( + "objc_class", ast.GetTranslationUnitDecl(), OptionalClangModuleID(300), false, false); + auto *cd = TypeSystemClang::GetAsObjCInterfaceDecl(class_type); + EXPECT_FALSE(!cd); + EXPECT_EQ(cd->getOwningModuleID(), 300u); +} + TEST_F(TestTypeSystemClang, TestIsClangType) { clang::ASTContext =
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Sorry for resetting this back from accepted :) Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1260 + + module->Name = name.str(); + return ast_source->RegisterModule(module); Why is that done? The module should already have the correct name when it is returned from findOrCreateModule (where we pass the same name and then create the new module with that name). If this line is removed, do we still need D75561? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl marked an inline comment as done. aprantl added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1254 + auto parent_desc = ast_source->getSourceDescriptor(parent.GetValue()); + std::tie(module, created) = m_module_map_up->findOrCreateModule( + name, parent_desc ? parent_desc->getModuleOrNull() : nullptr, @teemperor This is the place where we need `Module *` instead of a `const Module *`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:318 + /// Synthesize a clang::Module and return its ID or a default-constructed ID. + OptionalClangModuleID GetOrCreateClangModule(llvm::StringRef name, OptionalClangModuleID parent, + bool is_framework = false, This probably deserves a hint that this can only be called on an TypeSystemClang that has created its own clang::ASTContext (otherwise the ASTSource could be anything and this asserts) as all other functions in TypeSystemClang otherwise work in both modes. `Should not be called when TypeSystemClang adopted an existing clang::ASTContext` or something like that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 251116. aprantl added a comment. Rolled https://reviews.llvm.org/D75626 back into this patch, addressed more review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/include/lldb/Symbol/CompilerType.h lldb/include/lldb/Symbol/TypeSystem.h lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/Symbol/CompilerType.cpp lldb/source/Symbol/Type.cpp lldb/source/Symbol/TypeSystem.cpp lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm lldb/unittests/Symbol/TestTypeSystemClang.cpp lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h === --- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h +++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h @@ -28,8 +28,8 @@ inline CompilerType createRecord(TypeSystemClang , llvm::StringRef name) { return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), - lldb::AccessType::eAccessPublic, name, 0, - lldb::LanguageType::eLanguageTypeC); + OptionalClangModuleID(), lldb::AccessType::eAccessPublic, name, + 0, lldb::LanguageType::eLanguageTypeC); } /// Create a record with the given name and a field with the given type Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp === --- lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -14,6 +14,7 @@ #include "lldb/Host/HostInfo.h" #include "lldb/Symbol/Declaration.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclObjC.h" #include "clang/AST/ExprCXX.h" #include "gtest/gtest.h" @@ -257,9 +258,10 @@ CompilerType basic_compiler_type = ast.GetBasicType(basic_type); EXPECT_TRUE(basic_compiler_type.IsValid()); - CompilerType enum_type = - ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), -Declaration(), basic_compiler_type, scoped); + CompilerType enum_type = ast.CreateEnumerationType( + "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(), Declaration(), + basic_compiler_type, scoped); + CompilerType t = ast.GetEnumerationIntegerType(enum_type); // Check that the type we put in at the start is found again. EXPECT_EQ(basic_compiler_type.GetTypeName(), t.GetTypeName()); @@ -267,14 +269,38 @@ } } +TEST_F(TestTypeSystemClang, TestOwningModule) { + TypeSystemClang ast("module_ast", HostInfo::GetTargetTriple()); + CompilerType basic_compiler_type = ast.GetBasicType(BasicType::eBasicTypeInt); + CompilerType enum_type = ast.CreateEnumerationType( + "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(100), Declaration(), + basic_compiler_type, false); + auto *ed = TypeSystemClang::GetAsEnumDecl(enum_type); + EXPECT_FALSE(!ed); + EXPECT_EQ(ed->getOwningModuleID(), 100u); + + CompilerType record_type = ast.CreateRecordType( + nullptr, OptionalClangModuleID(200), lldb::eAccessPublic, "FooRecord", + clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); + auto *rd = TypeSystemClang::GetAsRecordDecl(record_type); + EXPECT_FALSE(!rd); + EXPECT_EQ(rd->getOwningModuleID(), 200u); + + CompilerType class_type = ast.CreateObjCClass( + "objc_class", ast.GetTranslationUnitDecl(), OptionalClangModuleID(300), false, false); + auto *cd = TypeSystemClang::GetAsObjCInterfaceDecl(class_type); + EXPECT_FALSE(!cd); + EXPECT_EQ(cd->getOwningModuleID(), 300u); +} + TEST_F(TestTypeSystemClang, TestIsClangType) { clang::ASTContext = m_ast->getASTContext(); lldb::opaque_compiler_type_t bool_ctype =
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 250971. aprantl marked an inline comment as done. aprantl added a comment. Address review feedback from Raphael. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm Index: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm @@ -0,0 +1,41 @@ +// RUN: %clang --target=x86_64-apple-macosx -g -gmodules \ +// RUN:-fmodules -fmodules-cache-path=%t.cache \ +// RUN:-c -o %t.o %s -I%S/Inputs +// RUN: lldb-test symbols -dump-clang-ast %t.o | FileCheck %s +// Verify that the owning module information from DWARF is preserved in the AST. + +@import A; + +Typedef t1; +// CHECK-DAG: TypedefDecl {{.*}} imported in A Typedef + +TopLevelStruct s1; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct TopLevelStruct +// CHECK-DAG: -FieldDecl {{.*}} in A a 'int' + +Struct s2; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct + +StructB s3; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A.B struct +// CHECK-DAG: -FieldDecl {{.*}} in A.B b 'int' + +Nested s4; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct Nested +// CHECK-DAG: -FieldDecl {{.*}} in A fromb 'StructB' + +Enum e1; +// CHECK-DAG: EnumDecl {{.*}} imported in A {{.*}} Enum_e +// FIXME: : -EnumConstantDecl {{.*}} imported in A a + +SomeClass *obj1; +// CHECK-DAG: ObjCInterfaceDecl {{.*}} imported in A SomeClass + +Template t2; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A struct Template + +Namespace::InNamespace t3; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A struct InNamespace + +Namespace::AlsoInNamespace t4; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A.B struct AlsoInNamespace Index: lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg === --- lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg +++ lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg @@ -1 +1 @@ -config.suffixes = ['.cpp', '.m', '.s', '.test', '.ll'] +config.suffixes = ['.cpp', '.m', '.mm', '.s', '.test', '.ll'] Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap @@ -0,0 +1,6 @@ +module A { + header "A.h" + module B { +header "B.h" + } +} Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h @@ -0,0 +1,8 @@ +typedef struct { + int b; +} StructB; + +namespace Namespace { +template struct AlsoInNamespace { T field; }; + extern template struct AlsoInNamespace; +} Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h @@ -0,0 +1,30 @@ +#include "B.h" // -*- ObjC -*- + +typedef int Typedef; + +struct TopLevelStruct { + int a; +}; + +typedef struct Struct_s { + int a; +} Struct; + +struct Nested { + StructB fromb; +}; + +typedef enum Enum_e { + a = 0 +} Enum; + +@interface SomeClass {} +@end + +template struct Template { T field; }; +extern template struct Template; + +namespace Namespace { +template struct InNamespace { T field; }; +extern template struct InNamespace; +} Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h === --- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -75,7 +75,8 @@ uint32_t m_payload; public: TypePayloadClang() = default; - explicit TypePayloadClang(bool is_complete_objc_class, ModuleID owning_module); + explicit TypePayloadClang(ModuleID owning_module, +bool is_complete_objc_class = false); explicit
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl marked an inline comment as done. aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:224 +if (auto *rd = llvm::dyn_cast(tag_decl)) + for (auto *f : rd->fields()) +TypeSystemClang::SetOwningModule(f, owning_module); labath wrote: > Maybe spell out the type here? I have no idea what's the type of this.. Yeah, this should either be a Visitor or in our ASTImporterDelegate in lldb. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl marked an inline comment as done. aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:224 +if (auto *rd = llvm::dyn_cast(tag_decl)) + for (auto *f : rd->fields()) +TypeSystemClang::SetOwningModule(f, owning_module); aprantl wrote: > labath wrote: > > Maybe spell out the type here? I have no idea what's the type of this.. > Yeah, this should either be a Visitor or in our ASTImporterDelegate in lldb. Another comment from an in-person discussion with @teemperor: We probably should not attach owning module information in the scratch/expression context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
teemperor added inline comments. Comment at: lldb/include/lldb/Symbol/Type.h:198 uint32_t GetEncodingMask(); - - bool IsCompleteObjCClass() { return m_is_complete_objc_class; } - - void SetIsCompleteObjCClass(bool is_complete_objc_class) { -m_is_complete_objc_class = is_complete_objc_class; - } + uint32_t () { return m_payload; } Remove reference? Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:91 DeclContextToDIEMap m_decl_ctx_to_die; + DIEToModuleMap m_die_to_module; std::unique_ptr m_clang_ast_importer_up; Maybe I'm missing something but this is `DIEToModuleMap` but it seems the value in this map is actually a module **ID** and not a module **map**? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
labath added a comment. Thanks for splitting this up. This does make things much easier to understand. I don't have any real objections to this, but I have some "thoughts" in inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:214-215 _type_sp->GetDeclaration(), type, Type::ResolveState::Forward)); + TypePayloadClang(type_sp->GetPayload()) + .SetOwningModuleID(GetOwningModuleID(die)); Did you deliberately not include the payload as an argument to the Type constructor? I can understand not wanting to add a extra argument to that constructor, but OTOH, having this in the constructor would make it harder to forget setting the module id when creating a type elsewhere. (And it's always nice to have immutable members if it is possible.) Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:221-237 + if (tag_decl) { +TypeSystemClang::SetOwningModule(tag_decl, owning_module); +if (auto *rd = llvm::dyn_cast(tag_decl)) + for (clang::FieldDecl *fd : rd->fields()) +TypeSystemClang::SetOwningModule(fd, owning_module); +if (auto *ed = llvm::dyn_cast(tag_decl)) + for (clang::EnumConstantDecl *ecd : ed->enumerators()) I don't know how feasible this is, but it has occurred to me that this is basically repeating the same structure iteration that would be done as a part of the import a couple of lines above. If would be nice if the importer could somehow set this property automatically. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl marked an inline comment as done. aprantl added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm:25 +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct Nested +// CHECK-DAG: -FieldDecl {{.*}} in A fromb 'StructB' + Note that the *Field* decl still comes from A, but its type (see above) doesn't. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 248244. aprantl marked an inline comment as done. aprantl added a comment. Hardcode triple in test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm Index: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm @@ -0,0 +1,41 @@ +// RUN: %clang --target=x86_64-apple-macosx -g -gmodules \ +// RUN:-fmodules -fmodules-cache-path=%t.cache \ +// RUN:-c -o %t.o %s -I%S/Inputs +// RUN: lldb-test symbols -dump-clang-ast %t.o | FileCheck %s +// Verify that the owning module information from DWARF is preserved in the AST. + +@import A; + +Typedef t1; +// CHECK-DAG: TypedefDecl {{.*}} imported in A Typedef + +TopLevelStruct s1; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct TopLevelStruct +// CHECK-DAG: -FieldDecl {{.*}} in A a 'int' + +Struct s2; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct + +StructB s3; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A.B struct +// CHECK-DAG: -FieldDecl {{.*}} in A.B b 'int' + +Nested s4; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct Nested +// CHECK-DAG: -FieldDecl {{.*}} in A fromb 'StructB' + +Enum e1; +// CHECK-DAG: EnumDecl {{.*}} imported in A Enum_e +// CHECK-DAG: -EnumConstantDecl {{.*}} imported in A a + +SomeClass *obj1; +// CHECK-DAG: ObjCInterfaceDecl {{.*}} imported in A SomeClass + +Template t2; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A struct Template + +Namespace::InNamespace t3; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A struct InNamespace + +Namespace::AlsoInNamespace t4; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A.B struct AlsoInNamespace Index: lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg === --- lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg +++ lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg @@ -1 +1 @@ -config.suffixes = ['.cpp', '.m', '.s', '.test', '.ll'] +config.suffixes = ['.cpp', '.m', '.mm', '.s', '.test', '.ll'] Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap @@ -0,0 +1,6 @@ +module A { + header "A.h" + module B { +header "B.h" + } +} Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h @@ -0,0 +1,8 @@ +typedef struct { + int b; +} StructB; + +namespace Namespace { +template struct AlsoInNamespace { T field; }; + extern template struct AlsoInNamespace; +} Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h @@ -0,0 +1,30 @@ +#include "B.h" // -*- ObjC -*- + +typedef int Typedef; + +struct TopLevelStruct { + int a; +}; + +typedef struct Struct_s { + int a; +} Struct; + +struct Nested { + StructB fromb; +}; + +typedef enum Enum_e { + a = 0 +} Enum; + +@interface SomeClass {} +@end + +template struct Template { T field; }; +extern template struct Template; + +namespace Namespace { +template struct InNamespace { T field; }; +extern template struct InNamespace; +} Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h === --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -78,6 +78,7 @@ DIEToDeclContextMap; typedef std::multimap DeclContextToDIEMap; + typedef llvm::DenseMap DIEToModuleMap; typedef llvm::DenseMap DIEToDeclMap; typedef llvm::DenseMap DeclToDIEMap; @@ -87,6 +88,7 @@ DeclToDIEMap m_decl_to_die; DIEToDeclContextMap m_die_to_decl_ctx; DeclContextToDIEMap m_decl_ctx_to_die; + DIEToModuleMap m_die_to_module; std::unique_ptr m_clang_ast_importer_up; /// @} @@ -140,6 +142,7 @@ clang::DeclContext *GetClangDeclContextContainingDIE(const DWARFDIE , DWARFDIE *decl_ctx_die); + unsigned
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 248241. aprantl marked 3 inline comments as done. aprantl added a comment. Address feedback from Pavel. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm Index: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm @@ -0,0 +1,40 @@ +// RUN: %clang_host -g -gmodules -fmodules -fmodules-cache-path=%t.cache \ +// RUN:-c -o %t.o %s -I%S/Inputs +// RUN: lldb-test symbols -dump-clang-ast %t.o | FileCheck %s +// Verify that the owning module information from DWARF is preserved in the AST. + +@import A; + +Typedef t1; +// CHECK-DAG: TypedefDecl {{.*}} imported in A Typedef + +TopLevelStruct s1; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct TopLevelStruct +// CHECK-DAG: -FieldDecl {{.*}} in A a 'int' + +Struct s2; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct + +StructB s3; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A.B struct +// CHECK-DAG: -FieldDecl {{.*}} in A.B b 'int' + +Nested s4; +// CHECK-DAG: CXXRecordDecl {{.*}} imported in A struct Nested +// CHECK-DAG: -FieldDecl {{.*}} in A fromb 'StructB' + +Enum e1; +// CHECK-DAG: EnumDecl {{.*}} imported in A Enum_e +// CHECK-DAG: -EnumConstantDecl {{.*}} imported in A a + +SomeClass *obj1; +// CHECK-DAG: ObjCInterfaceDecl {{.*}} imported in A SomeClass + +Template t2; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A struct Template + +Namespace::InNamespace t3; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A struct InNamespace + +Namespace::AlsoInNamespace t4; +// CHECK-DAG: ClassTemplateSpecializationDecl {{.*}} imported in A.B struct AlsoInNamespace Index: lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg === --- lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg +++ lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg @@ -1 +1 @@ -config.suffixes = ['.cpp', '.m', '.s', '.test', '.ll'] +config.suffixes = ['.cpp', '.m', '.mm', '.s', '.test', '.ll'] Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap @@ -0,0 +1,6 @@ +module A { + header "A.h" + module B { +header "B.h" + } +} Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h @@ -0,0 +1,8 @@ +typedef struct { + int b; +} StructB; + +namespace Namespace { +template struct AlsoInNamespace { T field; }; + extern template struct AlsoInNamespace; +} Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h === --- /dev/null +++ lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h @@ -0,0 +1,30 @@ +#include "B.h" // -*- ObjC -*- + +typedef int Typedef; + +struct TopLevelStruct { + int a; +}; + +typedef struct Struct_s { + int a; +} Struct; + +struct Nested { + StructB fromb; +}; + +typedef enum Enum_e { + a = 0 +} Enum; + +@interface SomeClass {} +@end + +template struct Template { T field; }; +extern template struct Template; + +namespace Namespace { +template struct InNamespace { T field; }; +extern template struct InNamespace; +} Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h === --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -78,6 +78,7 @@ DIEToDeclContextMap; typedef std::multimap DeclContextToDIEMap; + typedef llvm::DenseMap DIEToModuleMap; typedef llvm::DenseMap DIEToDeclMap; typedef llvm::DenseMap DeclToDIEMap; @@ -87,6 +88,7 @@ DeclToDIEMap m_decl_to_die; DIEToDeclContextMap m_die_to_decl_ctx; DeclContextToDIEMap m_decl_ctx_to_die; + DIEToModuleMap m_die_to_module; std::unique_ptr m_clang_ast_importer_up; /// @} @@ -140,6 +142,7 @@ clang::DeclContext *GetClangDeclContextContainingDIE(const DWARFDIE , DWARFDIE *decl_ctx_die); + unsigned GetOwningModuleID(const DWARFDIE ); bool
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
labath added a comment. This sounds like it could be useful though I can't say I really no much about modules. I appreciate the effort in splitting this patch up, but I am wondering if we couldn't do this is one more step. Would it be possible to split the TypeSystem changes into a patch of its own (with some TypeSystemClang unit tests), and then have separate a patch where SymbolFileDWARF starts making use of this functionality? I think that would make easier to follow what's going in the two subsystems/plugins as well as reduce some of the noise caused by the interface changes. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:224 +if (auto *rd = llvm::dyn_cast(tag_decl)) + for (auto *f : rd->fields()) +TypeSystemClang::SetOwningModule(f, owning_module); Maybe spell out the type here? I have no idea what's the type of this.. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:57-78 + /// The Layout is as follows: + /// \verbatim + /// bit 0..30 ... Owning Module ID. + /// bit 31 .. IsCompleteObjCClass. + /// \endverbatim uint32_t _payload; public: Maybe it would be simpler to have a struct full of bitfields and then just memcpy it to/from the uint32_t in the constructor/destructor? Comment at: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h:24 +extern template struct InNamespace; +} How about a case where A.h defines a type which references a type from B.h (e.g. contains it as a member variable) ? Comment at: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm:1 +// RUN: %clang_host -g -gmodules -fmodules -fmodules-cache-path=%t.cache \ +// RUN:-c -o %t.o %s -I%S/Inputs I think this test will fail on windows. At least it did fail for me when I manually forced a windows target here (linux was fine though). I think the problem is that lldb does not know how to open an unlinked COFF file. I think it would be better to hardcode a triple here instead of using %clang_host. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 248036. aprantl added a comment. Address feedback from Shafik. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/include/lldb/Symbol/CompilerType.h lldb/include/lldb/Symbol/TypeSystem.h lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/Symbol/CompilerType.cpp lldb/source/Symbol/Type.cpp lldb/source/Symbol/TypeSystem.cpp lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm lldb/unittests/Symbol/TestTypeSystemClang.cpp lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h === --- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h +++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h @@ -27,7 +27,7 @@ } inline CompilerType createRecord(TypeSystemClang , llvm::StringRef name) { - return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), + return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), 0, lldb::AccessType::eAccessPublic, name, 0, lldb::LanguageType::eLanguageTypeC); } Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp === --- lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -258,7 +258,7 @@ EXPECT_TRUE(basic_compiler_type.IsValid()); CompilerType enum_type = - ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), + ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), 0, Declaration(), basic_compiler_type, scoped); CompilerType t = ast.GetEnumerationIntegerType(enum_type); // Check that the type we put in at the start is found again. @@ -273,7 +273,7 @@ TypeSystemClang::GetOpaqueCompilerType(, lldb::eBasicTypeBool); CompilerType bool_type(m_ast.get(), bool_ctype); CompilerType record_type = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); // Clang builtin type and record type should pass EXPECT_TRUE(ClangUtil::IsClangType(bool_type)); @@ -285,7 +285,7 @@ TEST_F(TestTypeSystemClang, TestRemoveFastQualifiers) { CompilerType record_type = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); QualType qt; @@ -357,7 +357,7 @@ // Test that a record with no fields returns false CompilerType empty_base = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "EmptyBase", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "EmptyBase", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); TypeSystemClang::StartTagDeclarationDefinition(empty_base); TypeSystemClang::CompleteTagDeclarationDefinition(empty_base); @@ -368,7 +368,7 @@ // Test that a record with direct fields returns true CompilerType non_empty_base = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "NonEmptyBase", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "NonEmptyBase", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); TypeSystemClang::StartTagDeclarationDefinition(non_empty_base); FieldDecl *non_empty_base_field_decl = m_ast->AddFieldToRecordType( @@ -384,7 +384,7 @@ // Test that a record with no direct fields, but fields in a base returns true CompilerType empty_derived = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "EmptyDerived", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "EmptyDerived",
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 248011. aprantl added a comment. Add C++ tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/include/lldb/Symbol/CompilerType.h lldb/include/lldb/Symbol/TypeSystem.h lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/Symbol/CompilerType.cpp lldb/source/Symbol/Type.cpp lldb/source/Symbol/TypeSystem.cpp lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm lldb/unittests/Symbol/TestTypeSystemClang.cpp lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h === --- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h +++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h @@ -27,7 +27,7 @@ } inline CompilerType createRecord(TypeSystemClang , llvm::StringRef name) { - return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), + return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), 0, lldb::AccessType::eAccessPublic, name, 0, lldb::LanguageType::eLanguageTypeC); } Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp === --- lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -258,7 +258,7 @@ EXPECT_TRUE(basic_compiler_type.IsValid()); CompilerType enum_type = - ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), + ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), 0, Declaration(), basic_compiler_type, scoped); CompilerType t = ast.GetEnumerationIntegerType(enum_type); // Check that the type we put in at the start is found again. @@ -273,7 +273,7 @@ TypeSystemClang::GetOpaqueCompilerType(, lldb::eBasicTypeBool); CompilerType bool_type(m_ast.get(), bool_ctype); CompilerType record_type = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); // Clang builtin type and record type should pass EXPECT_TRUE(ClangUtil::IsClangType(bool_type)); @@ -285,7 +285,7 @@ TEST_F(TestTypeSystemClang, TestRemoveFastQualifiers) { CompilerType record_type = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); QualType qt; @@ -357,7 +357,7 @@ // Test that a record with no fields returns false CompilerType empty_base = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "EmptyBase", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "EmptyBase", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); TypeSystemClang::StartTagDeclarationDefinition(empty_base); TypeSystemClang::CompleteTagDeclarationDefinition(empty_base); @@ -368,7 +368,7 @@ // Test that a record with direct fields returns true CompilerType non_empty_base = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "NonEmptyBase", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "NonEmptyBase", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); TypeSystemClang::StartTagDeclarationDefinition(non_empty_base); FieldDecl *non_empty_base_field_decl = m_ast->AddFieldToRecordType( @@ -384,7 +384,7 @@ // Test that a record with no direct fields, but fields in a base returns true CompilerType empty_derived = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "EmptyDerived", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "EmptyDerived", clang::TTK_Struct,
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
shafik added inline comments. Comment at: lldb/include/lldb/Symbol/CompilerType.h:236 // type is valid and the type system supports typedefs, else return an - // invalid type. + // invalid type. The payload argument is the typesystem-specific Type payload. CompilerType CreateTypedef(const char *name, Why not convert this comment style? Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6965 + member->setFromASTFile(); + member->setOwningModuleID(id); +} Do we also need to make sure `setModuleOwnershipKind(...)` is consistent? Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:71 + void SetOwningModuleID(unsigned id) { +assert(id < ObjCClassBit); +bool is_complete = IsCompleteObjCClass(); So we at least expect the last bit to be available, do we expect to add more bits like `ObjCClassBit`, how many do we expect to have available? I guess conversely what range do we expect `id` to fit in? Does it make sense to document this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 248009. aprantl added a comment. Factored out NFC changes into separate reviews. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: lldb/include/lldb/Symbol/CompilerType.h lldb/include/lldb/Symbol/TypeSystem.h lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/Symbol/CompilerType.cpp lldb/source/Symbol/Type.cpp lldb/source/Symbol/TypeSystem.cpp lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm lldb/unittests/Symbol/TestTypeSystemClang.cpp lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h === --- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h +++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h @@ -27,7 +27,7 @@ } inline CompilerType createRecord(TypeSystemClang , llvm::StringRef name) { - return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), + return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), 0, lldb::AccessType::eAccessPublic, name, 0, lldb::LanguageType::eLanguageTypeC); } Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp === --- lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -258,7 +258,7 @@ EXPECT_TRUE(basic_compiler_type.IsValid()); CompilerType enum_type = - ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), + ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), 0, Declaration(), basic_compiler_type, scoped); CompilerType t = ast.GetEnumerationIntegerType(enum_type); // Check that the type we put in at the start is found again. @@ -273,7 +273,7 @@ TypeSystemClang::GetOpaqueCompilerType(, lldb::eBasicTypeBool); CompilerType bool_type(m_ast.get(), bool_ctype); CompilerType record_type = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); // Clang builtin type and record type should pass EXPECT_TRUE(ClangUtil::IsClangType(bool_type)); @@ -285,7 +285,7 @@ TEST_F(TestTypeSystemClang, TestRemoveFastQualifiers) { CompilerType record_type = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); QualType qt; @@ -357,7 +357,7 @@ // Test that a record with no fields returns false CompilerType empty_base = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "EmptyBase", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "EmptyBase", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); TypeSystemClang::StartTagDeclarationDefinition(empty_base); TypeSystemClang::CompleteTagDeclarationDefinition(empty_base); @@ -368,7 +368,7 @@ // Test that a record with direct fields returns true CompilerType non_empty_base = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "NonEmptyBase", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "NonEmptyBase", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); TypeSystemClang::StartTagDeclarationDefinition(non_empty_base); FieldDecl *non_empty_base_field_decl = m_ast->AddFieldToRecordType( @@ -384,7 +384,7 @@ // Test that a record with no direct fields, but fields in a base returns true CompilerType empty_derived = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "EmptyDerived", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic,
[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST
aprantl updated this revision to Diff 247992. aprantl retitled this revision from "RFC: Preserve the owning module information from DWARF in the synthesized AST" to "Preserve the owning module information from DWARF in the synthesized AST". aprantl added a comment. Now with a more comprehensive testcase and support for deferred completion via DWARFASTParserClang::ParseTypeFromClangModule(). This is good to review in earnest now! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75488/new/ https://reviews.llvm.org/D75488 Files: clang/include/clang/AST/DeclBase.h clang/include/clang/AST/ExternalASTSource.h clang/lib/AST/ExternalASTSource.cpp clang/lib/Serialization/ASTReader.cpp lldb/include/lldb/Symbol/CompilerType.h lldb/include/lldb/Symbol/Type.h lldb/include/lldb/Symbol/TypeSystem.h lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/Symbol/CompilerType.cpp lldb/source/Symbol/Type.cpp lldb/source/Symbol/TypeSystem.cpp lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm lldb/unittests/Symbol/TestTypeSystemClang.cpp lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h === --- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h +++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h @@ -27,7 +27,7 @@ } inline CompilerType createRecord(TypeSystemClang , llvm::StringRef name) { - return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), + return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(), 0, lldb::AccessType::eAccessPublic, name, 0, lldb::LanguageType::eLanguageTypeC); } Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp === --- lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -258,7 +258,7 @@ EXPECT_TRUE(basic_compiler_type.IsValid()); CompilerType enum_type = - ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), + ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(), 0, Declaration(), basic_compiler_type, scoped); CompilerType t = ast.GetEnumerationIntegerType(enum_type); // Check that the type we put in at the start is found again. @@ -273,7 +273,7 @@ TypeSystemClang::GetOpaqueCompilerType(, lldb::eBasicTypeBool); CompilerType bool_type(m_ast.get(), bool_ctype); CompilerType record_type = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); // Clang builtin type and record type should pass EXPECT_TRUE(ClangUtil::IsClangType(bool_type)); @@ -285,7 +285,7 @@ TEST_F(TestTypeSystemClang, TestRemoveFastQualifiers) { CompilerType record_type = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "FooRecord", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); QualType qt; @@ -357,7 +357,7 @@ // Test that a record with no fields returns false CompilerType empty_base = m_ast->CreateRecordType( - nullptr, lldb::eAccessPublic, "EmptyBase", clang::TTK_Struct, + nullptr, 0, lldb::eAccessPublic, "EmptyBase", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); TypeSystemClang::StartTagDeclarationDefinition(empty_base); TypeSystemClang::CompleteTagDeclarationDefinition(empty_base); @@ -368,7 +368,7 @@ // Test that a record with direct fields returns true CompilerType non_empty_base = m_ast->CreateRecordType( - nullptr,