[PATCH] D135634: [clang][modules] Serialize VFS overlay paths into PCMs

Fri Dec 2 14:51:15 PST 2022

jansvoboda11 added a comment.

In D135634#3967718 <https://reviews.llvm.org/D135634#3967718>, @benlangmuir wrote:

>> If the original HeaderSearchOptions didn't have any VFS overlay files, adopt the PCM ones.
>
> IIUC this is what the patch does, right?

Yes, that's correct.

>> If the original HeaderSearchOptions did have VFS overlay files of its own, error out if the PCM has different ones.
>
> Where did we land on this on?

We didn't. I looked into the current behavior once again and noticed that for language options and the target, we adopt whatever the PCM reports for the first time the ASTReader callbacks get called and **ignore** everything else. I didn't want to complicate things beyond that, since @akyrtzi mentioned this mode of `ASTUnit` (adopting stuff from PCMs) isn't being actively used anyways, so I did the same thing.

================
Comment at: clang/include/clang/Serialization/ASTReader.h:179
+  /// otherwise.
+  virtual bool ReadHeaderSearchPaths(const HeaderSearchOptions &HSOpts,
+                                     bool Complain) {
----------------
benlangmuir wrote:
> We should document what the expectations are for `ReadHeaderSearchOptions` and `ReadHeaderSearchPaths` callbacks: will paths be set when `ReadHeaderSearchOptions` is called? Will non-paths be set when `ReadHeaderSearchPaths` is called? I assume we want to say "no" so that we're not tied to callback order, but we should document it.
Sounds good.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135634