[PATCH] D35271: Fix printing policy for AST context loaded from file

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Thanks for adding the test! This is looking good.



================
Comment at: unittests/Frontend/ASTUnitTest.cpp:32
+    llvm::SmallString<256> Dir;
+    ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("astunit-test", Dir));
+    TestDir = Dir.str();
----------------
If a directory isn't necessary to exercise the desired functionality, it'd be better to use createTemporaryFile() to open a temporary file descriptor, and then to use tool_output_file() to manage the descriptor's lifetime. Tool_output_file has logic to clean up files when a signal is received, etc.


================
Comment at: unittests/Frontend/ASTUnitTest.cpp:48
+    TemporaryFiles.insert(Filename);
+    llvm::sys::fs::create_directories(llvm::sys::path::parent_path(Filename));
+  }
----------------
It doesn't look like all of the directories which could be created here would be deleted? With any luck it'll be possible to use tool_output_file() to side step the issue.


https://reviews.llvm.org/D35271