[PATCH] D14320: Fix createUniqueFile() to do what it is documented to do: create the unique file in a temporary directory if the model isn't absolute. Fix the callers of createUniqueFile() which were affected by this change. Add test cases to verify patch.

Paweł Bylica via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 5 02:00:13 PST 2015


chfast added a comment.

About the example. Currently it is:
`clang-%%-%%-%%-%%-%%.s => clang-a0-b1-c2-d3-e4.s`.
With your changes it should be something like:
`clang-%%-%%-%%-%%-%%.s => /tmp/clang-a0-b1-c2-d3-e4.s`.

In general we have 3 options here:

1. Leave the implementation as it was, fix the docs. There are definitely use cases (exposed by your patch) that would benefit from that.
2. Apply your patch. I suspect it is the case you are interested in.
3. Try to support 1 and 2. It can be by adding additional param to the function or by adding similar function.

I'm not sure which solution is the best so I would like someone else to comment on that.


================
Comment at: unittests/Support/Path.cpp:509
@@ +508,3 @@
+  // path should still be absolute.
+  path::append(AbsolutePath, "unique%%%%%%.txt");
+  ASSERT_NO_ERROR(fs::createUniqueFile(Twine(AbsolutePath), UniquePath));
----------------
I was quite sure the docs of system_temp_directory() stated that it would not have the trailing /. I will look at that later, but I cannot test OSX myself.

I would say having the trailing dir separator is at least not guaranteed.


http://reviews.llvm.org/D14320





More information about the llvm-commits mailing list