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

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 8 06:44:15 PST 2018

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.

  rC Clang


More information about the cfe-commits mailing list