[PATCH] D43967: [ASTImporter] Add test helper Fixture
Gábor Horváth via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 22 08:26:41 PDT 2018
xazax.hun accepted this revision.
xazax.hun added a comment.
Overall looks good, some minor comments inline.
================
Comment at: unittests/AST/ASTImporterTest.cpp:276
+ // This will not create the file more than once.
+ createVirtualFile(ToAST.get(), It->FileName, It->Code);
+
----------------
Maybe an IfNeeded suffix or something like that rather than a comment?
================
Comment at: unittests/AST/ASTImporterTest.cpp:289
+ for (auto &Tu : FromTUs) {
+ if (Tu.Unit) {
+ llvm::errs() << "FromAST:\n";
----------------
When can the TU.Unit be nullptr here? Should this be an assert instead?
================
Comment at: unittests/AST/ASTImporterTest.cpp:1045
+
+ assert(Check(From));
+ Check(To);
----------------
I wonder why we only assert on the first one and if we should use GTEST macros here. Same questions below.
Repository:
rC Clang
https://reviews.llvm.org/D43967
More information about the cfe-commits
mailing list