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
