mizvekov added a comment. In D133082#3764256 <https://reviews.llvm.org/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 somewhat novel. I think it'd be a good idea to get more > buy-in with an RFC on Discourse for more broad awareness. > > 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 > https://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html, but having > such a page for Clang would be a good idea (even if the page is incomplete). Okay, fair! We will have an RFC about this. ================ Comment at: clang/lib/Driver/Driver.cpp:5427 + ? A->getValue() + : std::getenv("CLANG_CRASH_DIAGNOSTICS_DIR"); + if (CrashDirectory) { ---------------- aaron.ballman wrote: > This should use `llvm::sys::Process::GetEnv()` so it properly handles text > encodings. CC @tahonermann @cor3ntin for text encoding awareness. Okay, makes sense. But be aware that we have multiple places in the llvm project where we are using std::getenv to retrieve a filesystem path variable. There is another one within clang itself as well (`CLANG_MODULE_CACHE_PATH`, which is where I took inspiration from). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133082/new/ https://reviews.llvm.org/D133082 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
