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:
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 ?
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?
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
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();
+
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) =
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.
+
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:
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:
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,
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,
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 =
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
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'
+
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:
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:
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
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
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
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
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
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
22 matches
Mail list logo
