[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D139168#4036850 <https://reviews.llvm.org/D139168#4036850>, @tahonermann wrote:

>> Header units have even more need to be involved in the build graph than named units. ODR violations and cache invalidation problems await anyone just winging it on header units (at least that's the understanding I've gotten from SG15 meetings).
>
> I think that latter claim applies equally to all module units. The ODR violation and cache invalidation concerns sometimes associated with header units occur in implicit module build systems in which a header unit might be built multiple times with different sets of options that result in an ODR violation. The same problem can occur with other kinds of module units if they are built multiple times with different options and then imported by distinct TUs that are then linked together. The general rule is, given a set of TUs that will be linked together, all imported module units should be built exactly one time.

I think the option set will be naturally a unique set for a single module-unit since a module-unit is an actual translation unit. But the header units are synthesized units so it is not the same case.

I am pretty interested about the topic indeed but I guess we'd better to find another place to discuss this.



================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:775
+            static llvm::StringMap<llvm::raw_fd_ostream> OSs;
+            std::unique_lock<std::mutex> LockGuard(Lock);
+
----------------
jansvoboda11 wrote:
> How will this work when a different process tries to write the same file? Could we write into a temporary file and then do atomic rename?
It may be problematic if another process tries to write the same file at the same time. But I feel it is an overkill to defend such cases otherwise many existing codes need to be refactored.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139168/new/

https://reviews.llvm.org/D139168