[PATCH] D48601: Added -fcrash-diagnostics-dir flag

[Zachary Turner via Phabricator via cfe-commits]

zturner added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:4043-4044
+    if (CCGenDiagnostics && A) {
+      SmallString<128> CrashDirectory;
+      CrashDirectory = A->getValue();
+      llvm::sys::path::append(CrashDirectory, Split.first);
----------------
Maybe you can combine these into one line with:

```
SmallString<128> CrashDirectory{ A->getValue() };
```


================
Comment at: clang/test/Driver/crash-diagnostics-dir.c:5
+// CHECK: Preprocessed source(s) and associated run script(s) are located at:
+// CHECK: diagnostic msg: {{.*}}/Output/crash-diagnostics-dir.c.tmp/{{.*}}.c
----------------
This will fail on Windows I think where path separators are backslashes.  I think you need to do something like:

```
{{.*}}Output{{/|\\}}crash-diagnostics-dir.c.tmp{{(/|\\).*}}.c
```


================
Comment at: llvm/include/llvm/Support/FileSystem.h:758-759
 
+std::error_code createUniqueFile(const Twine &Prefix, StringRef Suffix,
+                                 SmallVectorImpl<char> &ResultPath);
 /// Represents a temporary file.
----------------
Is there any reason we can't just use the existing overload?  In other words, instead of creating this overload and having the user write `createUniqueFile("foo", "bar")`, how about just `createUniqueFile("foo-%%%%%%.bar")` which seems equivalent.


Repository:
  rL LLVM

https://reviews.llvm.org/D48601