[Lldb-commits] [PATCH] D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

Shafik Yaghmour via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Oct 2 10:15:44 PDT 2019

shafik added inline comments.

Comment at: clang/include/clang/AST/ExternalASTMerger.h:95
-    ImporterSource(ASTContext &_AST, FileManager &_FM, const OriginMap &_OM)
-        : AST(_AST), FM(_FM), OM(_OM) {}
+    ImporterSource(ASTContext &_AST, FileManager &_FM, const OriginMap &_OM,
+                   bool _Temporary = false, ExternalASTMerger *Merger = nullptr)
Identifiers the begin with an underscore and followed by a capital letter are reserved see [lex.name/p3.1](http://eel.is/c++draft/lex.name#3.1):

>Each identifier that contains a double underscore __ or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use.

Comment at: clang/lib/AST/ExternalASTMerger.cpp:141
+    // that doesn't cause having minimally imported declarations in the target
+    // ASTContext that no connected ASTImporter has imported (and can complete).
+    //
martong wrote:
> This line/sentence is hard to parse for me.
> I get this part: 
> ```
> This way the ExternalASTMerger can safely do a minimal import that doesn't cause having minimally imported declarations in the target ASTContext.
> ```
> But this I don't: 
> ```
> that no connected ASTImporter has imported.
> ```
I agree this is hard to parse, although I am happy that you are explaining the rationale.

Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:183
-namespace {
-/// This class walks an AST and ensures that all DeclContexts defined inside the
So we are removing this b/c we are now doing a minimal import from the temporary source and then in the next patch you will change that?




More information about the lldb-commits mailing list