[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Alexandre Ganea via Phabricator via llvm-commits]

aganea marked 2 inline comments as done.
aganea added inline comments.


================
Comment at: clang/include/clang/Basic/CodeGenOptions.h:320
+  /// Executable and command-line used to create a given CompilerInvocation.
+  const char *BuildTool = nullptr;
+  ArrayRef<const char *> CommandLineArgs;
----------------
hans wrote:
> The name BuildTool makes me think of things like Make or MSBuild rather than the compiler. And in this case we know the compiler is Clang :-) Also since this is for capturing the cc1 arguments, it shouldn't really matter if it's clang-cl, clang++ or just clang. So it seems unfortunate that it has to be piped through all the way like this..
> 
> Is there some way we could just grab the path to the current clang binary during the pdb writing?
There's `sys::fs::getMainExecutable(const char *argv0, void *MainExecAddr)` but that's not guaranteed to work on all platforms, you still need to provide argv[0] to fallback, see https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Unix/Path.inc#L232
I'm not well versed in Unix distros, can this only rarely occur?
Would you prefer that I used that function instead? (it is ok on Windows though)



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:835
+static std::string renderCommandLine(ArrayRef<const char *> CommandLineArgs,
+                                     StringRef MainFile) {
+  std::string FlatCmdLine;
----------------
hans wrote:
> Don't we already have code somewhere that can do this quoting? E.g. the code that prints the cc1 args for "clang -v"?
Yes, the code below was copy-pasted from `Command::printArg`. But that's in Clang. Should I make that code common somewhere in `llvm/lib/Support`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833