[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