martong added inline comments.
================
Comment at: unittests/AST/ASTImporterTest.cpp:276
+ // This will not create the file more than once.
+ createVirtualFile(ToAST.get(), It->FileName, It->Code);
+
----------------
xazax.hun wrote:
> Maybe an IfNeeded suffix or something like that rather than a comment?
Added the suffix.
================
Comment at: unittests/AST/ASTImporterTest.cpp:289
+ for (auto &Tu : FromTUs) {
+ if (Tu.Unit) {
+ llvm::errs() << "FromAST:\n";
----------------
xazax.hun wrote:
> When can the TU.Unit be nullptr here? Should this be an assert instead?
Good catch, changed it.
================
Comment at: unittests/AST/ASTImporterTest.cpp:1045
+
+ assert(Check(From));
+ Check(To);
----------------
xazax.hun wrote:
> I wonder why we only assert on the first one and if we should use GTEST
> macros here. Same questions below.
Changed to use GTEST macros.
The reason why we assert only on the first is this: In the first check we
verify the original "from" AST, if the assumption we made on the original AST
is false then there is no point to continue. However, in the second check we do
verify the logic of the importer which may fail. But that is not a fatal
failure, because if there were any other checks after that we would like to
know if those checks pass or not.
The main difference between `EXPECT_*` and `ASSERT_*` is that `ASSERT_*` will
terminate the given test if it fails, but `EXPECT_*` keep on going and
subsequent checks will be executed in the same test.
Repository:
rC Clang
https://reviews.llvm.org/D43967
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
