[PATCH] D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Seems reasonable, but I have a concern about the cc1 line being replicable



================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5127
 
+#ifdef _WIN32
+  // Put (mini-)dumps in the same place as other crash diagnostics files.
----------------
I guess I would remove this ifdef as well and adjust the comment to indicate that this only controls the location of some kinds of crash dumps for some platforms.


================
Comment at: llvm/lib/Support/Signals.cpp:46
                          cl::location(DisableSymbolicationFlag), cl::Hidden);
+#ifdef _WIN32
+static std::string CrashDiagnosticsDirectory;
----------------
I guess the one question I have is, should we have these ifdefs? Thinking about it, we generally want the -cc1 line to be platform independent. You should be able to take a cc1 line from Windows and run it on Linux. That is our typical crash reproduction work flow. With that in mind, I think we need this cl::opt on all platforms, even if it does nothing on Linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99199