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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG40472ef14cd3: [clang][modules] Serialize VFS overlay paths into PCMs (authored by jansvoboda11). Changed prior to commit: https://reviews.llvm.org

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

benlangmuir accepted this revision. benlangmuir added a comment. Thanks for the explanation, make sense. LGTM with the documentation tweak! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135634/new/ https://reviews.llvm.org/D135634

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

jansvoboda11 added a comment. In 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 origina

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

benlangmuir added a comment. > If the original HeaderSearchOptions didn't have any VFS overlay files, adopt > the PCM ones. IIUC this is what the patch does, right? > If the original HeaderSearchOptions did have VFS overlay files of its own, > error out if the PCM has different ones. Where di

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

jansvoboda11 updated this revision to Diff 479723. jansvoboda11 added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Move header search paths into unhashed control block. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

jansvoboda11 added a comment. Ah, I now see that there are modes where `ASTUnit` gets configured with command-line arguments. I'm not very familiar with its clients, but I'd be inclined to: 1. If the original `HeaderSearchOptions` didn't have any VFS overlay files, adopt the PCM ones. 2. If th

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

bnbarham added a comment. In D135634#3866704 , @jansvoboda11 wrote: > In D135634#3866353 , @benlangmuir > wrote: > >> I think we should deduplicate the vfs overlays if the same ivfsoverlay is >> specified in bo

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

jansvoboda11 added a comment. In D135634#3866353 , @benlangmuir wrote: > I think we should deduplicate the vfs overlays if the same ivfsoverlay is > specified in both the pcm and the command-line. My understanding is that `ASTUnit` never uses command-l

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

bnbarham added a comment. In D135634#3866353 , @benlangmuir wrote: > I think we should deduplicate the vfs overlays if the same ivfsoverlay is > specified in both the pcm and the command-line. > > @bnbarham any concern about overlay vs chaining behaviou

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

benlangmuir added a subscriber: bnbarham. benlangmuir added a comment. I think we should deduplicate the vfs overlays if the same ivfsoverlay is specified in both the pcm and the command-line. @bnbarham any concern about overlay vs chaining behaviour here? I remember you looking at that a while

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

jansvoboda11 updated this revision to Diff 466911. jansvoboda11 added a comment. Use `ArrayRef`, `move` VFS smart pointer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135634/new/ https://reviews.llvm.org/D135634 Files: clang/include/clang/Basic

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

jansvoboda11 added a comment. In D135634#3850224 , @akyrtzi wrote: > This seems fine to me but note that we no longer depend on the functionality > that `test/Index/index-module-with-vfs.m` is testing (and not sure anyone > else does), so if there is an

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

akyrtzi added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4715 + return createVFSFromOverlayFiles(CI.getHeaderSearchOpts().VFSOverlayFiles, + Diags, BaseFS); +} A bit nitpick but I'd suggest changing t

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

akyrtzi added a comment. This seems fine to me but note that we no longer depend on the functionality that `test/Index/index-module-with-vfs.m` is testing (and not sure anyone else does), so if there is another change affecting it that is more complicated we could consider removing the test.

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

jansvoboda11 created this revision. jansvoboda11 added reviewers: akyrtzi, benlangmuir, Bigcheese. Herald added a subscriber: ributzka. Herald added a project: All. jansvoboda11 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. With implicitl