[PATCH] D106876: Remove non-affecting module maps from PCM files.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.
Herald added a project: All.

Hi @ilyakuteev, this patch is causing some issues downstream. (See `clang/test/Modules/non-affecting-module-maps-source-locations.m` here <https://github.com/apple/llvm-project/pull/5451>.)

I think they all boil down to the fact that a `SourceLocation` is a simple offset into `SourceManager`'s buffer of concatenated input files. By not serializing some input files, we make that buffer shorter by leaving out some of its parts. This means that a `SourceLocation` that used to point at the end of the last file might now be pointing outside the new, shorter buffer. More generally, any `SourceLocation` pointing after any of the removed input files is now bogus. It seems that we should be computing the set of non-affecting input files at the start of serialization, and then any `SourceLocation` we serialize, we need to adjust to account for the changes in the input file buffer. I'm not sure how efficiently we can implement this, given that we might need to adjust every `SourceLocation`. And we have lots of them.

We rely on this feature for "implicitly discovered, explicitly built modules". For our purpose, the (simpler) downstream patch changes the PCM file so that it still contains all input files, preventing the issue I just described, but includes a bit that says whether a file was affecting or not. My question is: would that approach work for your use-case? You mentioned you want to avoid rebuilds in distributed compilation when some non-affecting input files are missing. You could avoid that by not validating non-affecting input files in `ASTReader`. One potential issue I see with this approach is that two PCMs for module M produced on two machines with different sets of non-affecting input files are not interchangeable. They will have different contents and therefore different signature. This could lead to rebuilds of importers that expect one signature for PCM M but (after some distributed re-shuffling), the other PCM ends up on the machine, creating a mismatch between the expected and actual PCM signature.

Please, let me know your thoughts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106876