bruno added a comment. Thanks for the additional docs! More comments below.
================ Comment at: lib/AST/ExternalASTMerger.cpp:116 + if (auto *ToDC = dyn_cast<DeclContext>(To)) { + logs() << "(ExternalASTMerger*)" << (void*)&Parent + << " imported (DeclContext*)" << (void*)ToDC ---------------- Can you conditionalize the logging? ================ Comment at: lib/AST/ExternalASTMerger.cpp:126 + Parent.HasImporterForOrigin(*FromOrigins.at(FromDC).AST)) { + logs() << "(ExternalASTMerger*)" << (void*)&Parent + << " forced origin (DeclContext*)" ---------------- Same here. ================ Comment at: lib/AST/ExternalASTMerger.cpp:134 + } else { + logs() << "(ExternalASTMerger*)" << (void*)&Parent + << " maybe recording origin (DeclContext*)" << (void*)FromDC ---------------- Same here. ================ Comment at: lib/AST/ExternalASTMerger.cpp:213 + if (!DidCallback) + logs() << "(ExternalASTMerger*)" << (void*)this + << " asserting for (DeclContext*)" << (void*)DC ---------------- Same here. ================ Comment at: lib/AST/ExternalASTMerger.cpp:286 + if (!FoundFromDC || !IsSameDC(FoundFromDC.get(), Origin.DC)) { + logs() << "(ExternalASTMerger*)" << (void*)this + << " decided to record origin (DeclContext*)" << (void*)Origin.DC ---------------- Ditto ================ Comment at: lib/AST/ExternalASTMerger.cpp:292 + } else { + logs() << "(ExternalASTMerger*)" << (void*)this + << " decided NOT to record origin (DeclContext*)" << (void*)Origin.DC ---------------- Ditto ================ Comment at: tools/clang-import-test/clang-import-test.cpp:232 + +struct CIAndOrigins { + using OriginMap = clang::ExternalASTMerger::OriginMap; ---------------- Can you also add a brief documentation for this one? ================ Comment at: tools/clang-import-test/clang-import-test.cpp:242 + return static_cast<ExternalASTMerger *>(Source)->GetOrigins(); + else + return EmptyOriginMap; ---------------- No need for the `else` here. ================ Comment at: tools/clang-import-test/clang-import-test.cpp:256 + llvm::SmallVector<ExternalASTMerger::ImporterSource, 3> Sources; + for (CIAndOrigins &Import : Imports) { + Sources.push_back( ---------------- No need for curly braces here. ================ Comment at: tools/clang-import-test/clang-import-test.cpp:326 "Errors occured while parsing the expression.", std::error_code()); } else { return std::move(CI); ---------------- No need for the `else` here. ================ Comment at: tools/clang-import-test/clang-import-test.cpp:333 + llvm::SmallVector<ExternalASTMerger::ImporterSource, 3> Sources; + for (CIAndOrigins &Import : Imports) { + Sources.push_back( ---------------- No need for curly braces here. ================ Comment at: tools/clang-import-test/clang-import-test.cpp:354 exit(-1); } else { ImportCIs.push_back(std::move(*ImportCI)); ---------------- No need for the `else` here. ================ Comment at: tools/clang-import-test/clang-import-test.cpp:365 } - llvm::Expected<std::unique_ptr<CompilerInstance>> ExpressionCI = - Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST, DumpIR); + if (UseOrigins) { + for (auto &ImportCI : ImportCIs) { ---------------- No need for braces in this entire block ================ Comment at: tools/clang-import-test/clang-import-test.cpp:376 exit(-1); } else { + Forget(*ExpressionCI, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs); ---------------- No need for the `else` here. https://reviews.llvm.org/D36589 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits