[PATCH] D43967: [ASTImporter] Add test helper Fixture

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: unittests/AST/ASTImporterTest.cpp:170
+    ASTContext &ToCtx = ToAST->getASTContext();
+    vfs::OverlayFileSystem *OFS = static_cast<vfs::OverlayFileSystem *>(
+        ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
----------------
xazax.hun wrote:
> Repeated type, use auto. Same below.
> Are we sure these cast's will not fail? Shouldn't we use dynamic casts and asserts?
Actually, we set up the static type in `buildASTFromCodeWithArgs` to `OverlayFileSystem`. Still you are right it would be better to use `dynamic_cast` but we can't since clang and the tests are build with `-fno-rtti`.


================
Comment at: unittests/AST/ASTImporterTest.cpp:184
+  // Must not call more than once within the same test.
+  std::tuple<Decl *, Decl *>
+  getImportedDecl(StringRef FromSrcCode, Language FromLang,
----------------
xazax.hun wrote:
> I wonder if pair or tuple is the better choice here. I have no strong preference just wondering.
I chose tuple here, because it is more general then pair, i.e. in the future we might be able to extend this easier. But this is a weak argument, nevertheless I cannot come up anything besides the pair.


Repository:
  rC Clang

https://reviews.llvm.org/D43967