This approach looks overall much better! Unless @sammccall has any extra comments, it LGTM.

Hi Michael, thanks for working on this!

Ping!

Ping!

Did you try xxHash64?

Ping!

Update the patch to use two ::Fixed, 32 in abbrev to encode hash.

Remove pasto from one of the testcases

Update testcase to use a more portable version of touch

Nice! Are you planning to address the FIXME's in a later update of this patch?

LGTM!

Nice! Is this something that can be tested for in unittests/Basic/FileManagerTest.cpp?

Thanks for fixing this, LGTM!

I'm curious to know if you measured the impact on the PCM size after this change (example: PCMs from building macOS public SDK)? For instance, PPD_SKIPPED_RANGES currently explodes for some specific inputs, just wonder what's the story here.

LGTM with one minor change.

LGTM

Internal compiler error: LFS error for "/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx"failed to create unique file /Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx.lock-0ae18733: No such file or directory

Thanks for working on this JF!

It seems conceptually a little strange to have the working directory be part of a serialized "FS", as it's fundamentally a property of a process and only transiently a property of the VFS.

Nice, LGTM!

Thanks for working on this. Can you add unit tests for those?

I am currently working on the next part of clang interface stubs that will take the interface stubs per compilation unit and merge them into one text stub (which will be used by something like llvm-elfabi to generate a stubbed out ELF .so file). I was using llvm-nm, but there are cases where the symbol is present in the .o file but it is marked as HIDDEN and llvm-readelf was what I was using to determine if the symbol was in fact marked as hidden. In these cases, the linker will not emit the symbol in the final .so file but it still needs the symbol as part of linking.

LGTM

Hi JF. Thanks for working on this, nice improvement to error handling!

ninja check-clang passes... is there anything else I should be testing?

Thanks for working on this, the approach & patch LGTM.

InMemoryModuleCache seems like a way more appropriate name here. Also thanks for improving some of the comments.

Nice, note that a subset of this information can be achieved with -Wauto-import, but this is way more general and solid, since it will handles @imports and pragma imports too. LGTM with a minor nitpick, see comments.

LGTM!

I missed that the DependencyCollector already marks them virtual, you are just making it obvious here. I think you can omit the ones that are already virtual and only add to the ones that are on the intend of this patch.

Not really. Would making only the attachTo* methods virtual enough though?

How much of the ModuleDependencyCollector will be reused as is by LLDB? I wonder about the tradeoff versus inheriting from DependencyCollector directly.

Thanks for working on this! LGTM

Ping

Ping!

This fixes the same problem previously addressed in https://reviews.llvm.org/D54923

Abandon this one in favor of https://reviews.llvm.org/D55676

Ping

Hi Eric,

Address @aaron.ballman and @erik.pilkington reviews.

Ping!

Update patch after @erik.pilkington review!

Address @rsmith comments

@rsmith this is on the non-determinism issue we discussed offline. Do you have any concerns with this approach?

Looks like adding a test in test/Driver/clang_f_opts.c would be good here.

Thanks for working on this @andrewjcg

Out of curiosity, why? If it makes you happy though, go for it! LGTM

LGTM

Although we can't synthesize a testcase for inclusion in the patch. Here is how one should attempt to reproduce the problem in macOS 10.14.

LGTM

LGTM

Hi Volodymyr. Does this behavior changed after your VFS fallback change?

LGTM with some minor changes.

LGTM.

Hi Erik, thanks for improving this. Comments below.

Hi Volodymyr,

LGTM with one more small change.

Hi Erik,

Update patch after review.

Addressed Adrian's review. Added support to consider the environment and well the combination platform-environment. @aprantl also handled multiple variants of simulator combinations.

Thanks for working on this Adrian, LGTM.

Thanks for the clean up! LGTM

LGTM