martong added a comment.
This is a good patch, it is good to separate responsibilities and it makes
cleaner how the clang::ASTImporter is used.
================
Comment at: lldb/source/Symbol/ClangASTImporter.cpp:250
+/// imported while completing the original Decls).
+class DeportQueueScope : public ClangASTImporter::NewDeclListener {
+ ClangASTImporter::ImporterDelegateSP m_delegate;
----------------
The verb `deport` is pretty vague in this context for me. Actually, what this
class does is importing missing members and methods of classes. Perhaps a
better name could be `ImportMembersQueueScope` ?
I still don't understand why is it needed to import the members in two steps in
LLDB: 1. import the class itself without members 2. import the members. So
perhaps we could have some documentation about that too here.
================
Comment at: lldb/source/Symbol/ClangASTImporter.cpp:324
+
+ NamedDecl *to_named_decl = dyn_cast<NamedDecl>(to);
+ // Check if we already deported this type.
----------------
Would it make sense to filter out those TagDecls which are already completed?
E.g.:
```
if (TagDecl *to_tag_decl = dyn_cast<TagDecl>(to))
if (to_tag_decl->isCompleteDefinition()) // skip tags which are already
completed
return;
```
Or this would not work because there are cases when the tag is completed, but
the members are still missing? If that is the case could you please document
that here too?
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61478/new/
https://reviews.llvm.org/D61478
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
