martong marked 8 inline comments as done. martong added inline comments.
================ Comment at: lib/AST/ASTImporter.cpp:7658 +ASTImporter::FoundDeclsTy +ASTImporter::FindDeclsInToCtx(DeclContext *DC, DeclarationName Name) { + // We search in the redecl context because of transparent contexts. ---------------- a_sidorin wrote: > Naming conventions require method names to start with a small letter. Okay, I changed it. Unfortunately `ASTImporter` does not really follow the conventions. Most of the functions start with a capital letter, and some with a small case. E.g.: ``` /// Retrieve the file manager that AST nodes are being imported from. FileManager &getFromFileManager() const { return FromFileManager; } /// Report a diagnostic in the "to" context. DiagnosticBuilder ToDiag(SourceLocation Loc, unsigned DiagID); ``` ================ Comment at: tools/clang-import-test/clang-import-test.cpp:12 #include "clang/AST/ASTImporter.h" +#include "clang/AST/ASTImporterLookupTable.h" #include "clang/AST/DeclObjC.h" ---------------- a_sidorin wrote: > It looks like the only change done to this file is including a header. Are > there any related changes that should be added? Thank you, this is a good catch, I removed it. ================ Comment at: unittests/AST/ASTImporterTest.cpp:3845 + ASTImporterLookupTable LT(*ToTU); +} + ---------------- a_sidorin wrote: > martong wrote: > > a_sidorin wrote: > > > Could you please explain what does this test do? > > Well, it makes sure that we can build a lookup table for an empty file > > without any *assertion*. We may have an assertion in the ctor during > > traversing the AST. I consider this the most basic use case as it tests > > only the constructor. > > However, if you find it weird I can remove it. > Yes, I think it's better to remove it or check some invariants of an empty > lookup table. Ok, I removed it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53708/new/ https://reviews.llvm.org/D53708 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits