[PATCH] D101758: [clang][modules] Add -cc1 option to backup PCM files
Duncan P. N. Exon Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon May 3 11:18:27 PDT 2021
dexonsmith added a comment.
A few high-level comments:
- Something like this *does* seem really useful for debugging problems with implicit module builds using a fuzzy context hash. But I feel like the use case is carved out at the wrong place; I feel like we either want this to be:
- more restrictive: only supported for implicit+fuzzy modules, so it's not misused; or
- more general: supported for all output files (CompilerInstance::createOutputFile), to provide a debugging option in other volatile filesystem situations.
- Changing the file extension doesn't seem ideal; it'd be better to embed the PID (or whatever it is) before the extension.
- I'm not sure I love adding the pid to the filename at all TBH; although I'm not sure what'd be better.
- From a file-handling level, there's definite crossover with temporary files (write to temporary location, then move atomically into place). It might be cleaner to:
- Write to the temp location, copy the temp to the `.pid` location, and move the temp to the final location.
- Write to the unique/uncontended location (as the new temporary location), then copy the file (e.g., using `llvm::copy_file`) to the final location. I don't know if `llvm::copy_file` is atomic though...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101758/new/
https://reviews.llvm.org/D101758
More information about the cfe-commits
mailing list