[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

This revision was automatically updated to reflect the committed changes. Closed by commit rG94c6dfbaebbd: [clang] Implement setting crash_diagnostics_dir through env variable (authored by mizvekov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133082/new/ https://reviews.llvm.org/D133082

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added a comment. @aaron.ballman I think all of your concerns are addressed now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133082/new/ https://reviews.llvm.org/D133082 ___ cfe-commits

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov updated this revision to Diff 457818. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133082/new/ https://reviews.llvm.org/D133082 Files: clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst clang/lib/Driver/Driver.cpp

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov updated this revision to Diff 457685. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133082/new/ https://reviews.llvm.org/D133082 Files: clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst clang/lib/Driver/Driver.cpp

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov updated this revision to Diff 457468. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133082/new/ https://reviews.llvm.org/D133082 Files: clang/docs/ClangCommandLineReference.rst clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added a comment. In D133082#3764256 , @aaron.ballman wrote: > If we go this route, we definitely need user-facing documentation that > explains what's going on. I don't think we have anything corresponding to >

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov updated this revision to Diff 457462. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133082/new/ https://reviews.llvm.org/D133082 Files: clang/docs/ClangCommandLineReference.rst clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov updated this revision to Diff 457327. mizvekov marked 10 inline comments as done. mizvekov requested review of this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133082/new/ https://reviews.llvm.org/D133082 Files:

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

Mordante accepted this revision. Mordante added a comment. Ideally it would be documented these artifacts are now available. However there's no good place in libc++ to document that. I'm working on such a document so I will take care of documenting these artifacts. LGTM from libc++'s PoV.

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added inline comments. Comment at: libcxx/test/libcxx/crash.sh.cpp:15 + +#pragma clang __debug parser_crash Mordante wrote: > The libc++ build seems to be green. I assume it was intended to be red so we > can validate the artifact is available in the

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

Mordante added a comment. Thanks for working on this! It's really great to get the crash report as an artifact. Comment at: libcxx/test/libcxx/crash.sh.cpp:15 + +#pragma clang __debug parser_crash The libc++ build seems to be green. I assume it was intended

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

aaron.ballman added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5427 + ? A->getValue() + : std::getenv("CLANG_CRASH_DIAGNOSTICS_DIR"); + if (CrashDirectory) { mizvekov wrote: >

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added a comment. In D133082#3764256 , @aaron.ballman wrote: > We use environment variables in several other places within the driver, but I > am a bit skittish about adding new ones to correspond to feature flags as > that seems to be

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

aaron.ballman added subscribers: cor3ntin, tahonermann. aaron.ballman added a comment. In D133082#3763829 , @erichkeane wrote: > I don't mind the idea, but wonder if some level of RFC/project-wide decision > should be made here? WDYT @aaron.ballman ?

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory =

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

erichkeane accepted this revision. erichkeane added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); +

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory =

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory =

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

erichkeane added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory =

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory =

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

erichkeane added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory =

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

mizvekov added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory =

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

erichkeane added a comment. I don't mind the idea, but wonder if some level of RFC/project-wide decision should be made here? WDYT @aaron.ballman ? Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if