jfb marked 2 inline comments as done. jfb added inline comments.
================ Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:107-111 TemporaryFiles::~TemporaryFiles() { llvm::MutexGuard Guard(Mutex); for (const auto &File : Files) - llvm::sys::fs::remove(File.getKey()); + if (std::error_code EC = llvm::sys::fs::remove(File.getKey())) + llvm::report_fatal_error("failed removing file \"" + File.getKey() + "\": " + EC.message()); ---------------- bruno wrote: > jfb wrote: > > jkorous wrote: > > > vsapsai wrote: > > > > Clangd uses `PrecompiledPreamble` but not `TemporaryFiles` as far as I > > > > can tell. `report_fatal_error` can be really disruptive for clangd, so > > > > it would be good to get an opinion from somebody working on it if this > > > > change would impact them. > > > Since client code in general might have different error > > > handling/reporting strategy and can't do much about failure in destructor > > > here, I'd consider just some kind of logging or assert here. > > > > > I changed it to `dbgs()`. > This probably means we won't see the message in a non-debug build, right? Yeah, so it'll be diagnosable but won't affect release builds (which I understand to be the concern expressed w.r.t. clangd). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65545/new/ https://reviews.llvm.org/D65545 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits