[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