[PATCH] D83228: [llvm] [unittests] Remove some temporary files after they're not needed

Sam McCall via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 31 05:23:22 PDT 2020


sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Sorry, this fell off my radar :-(



================
Comment at: llvm/include/llvm/Testing/Support/SupportHelpers.h:122
+  ///               llvm::sys::fs::createUniqueDirectory.
+  explicit TempDir(StringRef Name, bool Unique = false) {
+    std::error_code EC;
----------------
the bool param that changes the meaning of the previous param is a little cryptic (including at callsites).
It also makes the common case of "unique temporary directory that I don't care about the name of

You could consider `TempDir(Name)` and `TempDir::unique()`. (Specifying the model doesn't seem all that useful?).

Up to you.


================
Comment at: llvm/include/llvm/Testing/Support/SupportHelpers.h:158
+  template <typename... T>
+  SmallString<128> path(StringRef component, T &&... rest) const {
+    SmallString<128> Result(Path);
----------------
to avoid a bit of template magic, you can just accept a path separated by `/` and call native on it before append. I tend to find this more readable at the callsites that multiple params, up to you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83228/new/

https://reviews.llvm.org/D83228



More information about the llvm-commits mailing list