hans marked an inline comment as done.
hans added a comment.
I'm basically happy with this, just some minor comments.
================
Comment at: clang/lib/Driver/Job.cpp:376
+ CrashReportInfo *CrashInfo) const {
+ OS << " Running the following in ExecuteCC1Tool():\n";
+ Command::Print(OS, Terminator, Quote, CrashInfo);
----------------
I think this gets printed by "clang -###" so maybe just print something short
here. Perhaps just an "(in-process)" prefix? Then we don't need to change so
many test expectations also.
================
Comment at: clang/lib/Driver/Job.cpp:394
+
+ llvm::CrashRecoveryContext::Enable();
+
----------------
Do we need to disable afterwards?
================
Comment at: clang/tools/driver/driver.cpp:313
+ llvm::cl::ResetAllOptionOccurrences();
+ StringRef Tool = argv[1] + 4;
void *GetExecutablePathVP = (void *)(intptr_t) GetExecutablePath;
----------------
aganea wrote:
> hans wrote:
> > This feels a little risky to me. Do we still need to rely on argv[1], now
> > that we have a nice CC1Command abstraction?
> Risky in what way? What would you suggest? The contents of `argv` are exactly
> the same in both cases: when called from the CC1Command, or explicitly from
> the command-line.
You're right, I suppose the code was already doing this.
================
Comment at: clang/tools/driver/driver.cpp:267
+
+ StringRef SpawnCC1Str = ::getenv("CLANG_SPAWN_CC1");
+ if (!SpawnCC1Str.empty()) {
----------------
Maybe just do the "!!" thing like for the environment variables above? It's not
pretty, but at least that would be consistent, and it avoids the problem of
deciding what values mean "on" and what mean "off".
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69825/new/
https://reviews.llvm.org/D69825
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
