[PATCH] D14959: [Support] Add path::temp_directory() function.

[Aaron Ballman via llvm-commits]

aaron.ballman added inline comments.

================
Comment at: include/llvm/Support/Path.h:336
@@ +335,3 @@
+/// @param Result Holds the resulting path name.
+void temp_directory(SmallVectorImpl<char> &Result, const Twine &Path1,
+                    const Twine &Path2 = "", const Twine &Path3 = "");
----------------
Why is Path1 required to pass to the function?

================
Comment at: lib/Support/Unix/Path.inc:563
@@ -562,3 +562,3 @@
 
-static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {
-  #if defined(_CS_DARWIN_USER_TEMP_DIR) && defined(_CS_DARWIN_USER_CACHE_DIR)
+enum class DarwinConfDir : bool { Temp, Cache };
+
----------------
Is there a reason this is a bool instead of an int?

================
Comment at: lib/Support/Unix/Path.inc:646
@@ +645,3 @@
+  const char *RequestedDir = getDefaultTempDir();
+  Result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
+}
----------------
Do you have to call Result.clear() here in the event getDarwinConfDir() fails but modifies Result?

================
Comment at: lib/Support/Windows/Path.inc:816
@@ -816,3 +815,3 @@
   // Fall back to a system default.
   const char *DefaultResult = "C:\\Temp";
   Result.append(DefaultResult, DefaultResult + strlen(DefaultResult));
----------------
I know this isn't yours, but this is a terrible idea just the same. We should not be hard coding paths on Windows. I have a slight preference for this API to return a bool indicating failure, but at the same time, I struggle to imagine what error recovery would look like. I think I could be persuaded that failures here should assert and fail.

================
Comment at: lib/Support/Windows/Path.inc:824
@@ +823,3 @@
+
+  temp_directory(Result, "");
+}
----------------
Do you need to clear Result if user_cache_directory() fails?


http://reviews.llvm.org/D14959