[scanner] setup the clang importer injected redirecting overlay vfs f…

[hyp]

…or correct clang dependency scanning

This makes its possible to use -explicit-module-build to correctly find 'crt' swift module on windows

[hyp]

@swift-ci please test

[cachemeifyoucan]

Can you be more specific about what problem is being fixed here. I see you added SilentClangImporterOverlayWrapperFS to avoid assertions in clang dependency scanner, is that the problem? But I don't the class being used anywhere.

Also I am unsure where is the VFS for ucrt module coming from. It would be good to elaborate the process of how the module map is found and what problem it has for dependency scanner right now.

[hyp]

Sorry, the SilentClangImporterOverlayWrapperFS is no longer needed after the rebranch (my original fix was on a pre-rebranch branch which had a strong assertion related to VFS overlay optimization count that is different now in the new branch). Now it should be more clear hopefully! Answers inline:

Also I am unsure where is the VFS for ucrt module coming from. It would be good to elaborate the process of how the module map is found and what problem it has for dependency scanner right now.

The module map for UCRT is shipped alongside swift (ucrt.modulemap). The clang importer then constructs the redirecting file system, that redirects from the virtual module map (doesn't exist on disk) located windows SDK in MSVC to the real one that's located next to Swift. This lets Swift import windows C runtime and C++ stdlib correctly in implicit module-based compilation, as clang can then actually load the injected module map file for UCRT.

Right now the Clang scanner that the Swift scanner uses for Clang modules doesn't know about the injected files that Clang requires to correctly import Clang modules that have system dependencies like UCRT on windows, and as such it is impossible to use -explicit-module-build on Windows as the Clang scanner is unable to find the injected module map for UCRT (it reports - Unable to find module dependency: 'ucrt').

This change ensures that the Swift scanner sets up the exact same injection redirect & overlay VFS for the Clang dependency scanner as it does for the Clang importer. This lets the scanner correctly find the module map and construct appropriate invocation for the build of the UCRT module, that will later go through -direct-clang-cc1-module-build.

I see you added SilentClangImporterOverlayWrapperFS to avoid assertions in clang dependency scanner, is that the problem? But I don't the class being used anywhere.

The SilentClangImporterOverlayWrapperFS is no longer needed, thanks!

[hyp]

@swift-ci please test

[hyp]

@swift-ci please test windows platform

[cachemeifyoucan]

The module map for UCRT is shipped alongside swift (ucrt.modulemap). The clang importer then constructs the redirecting file system, that redirects from the virtual module map (doesn't exist on disk) located windows SDK in MSVC to the real one that's located next to Swift. This lets Swift import windows C runtime and C++ stdlib correctly in implicit module-based compilation, as clang can then actually load the injected module map file for UCRT.

Is the process to import the ucrt from clang the same as swift or is this a swift specific module? If it is the previous, it might better to do it inside clang. The fix looks good to me as I think dependency scanner should have the file available. I just want to understand it better so it is possible to get rid of that redirecting file system if possible. It is fine to have redirecting file system during dependency scanner but we want a path forward to get rid of that after dependency scanner if scanner can build a VFS that contains all inputs.

[hyp]

The CI doesn't seem to find the overlay still, even though locally I can do it. Need to trace the overlay path in the CI:

error: Unable to find module dependency: '_complex'

[hyp]

Is the process to import the ucrt from clang the same as swift or is this a swift specific module? If it is the previous, it might better to do it inside clang. The fix looks good to me as I think dependency scanner should have the file available. I just want to understand it better so it is possible to get rid of that redirecting file system if possible. It is fine to have redirecting file system during dependency scanner but we want a path forward to get rid of that after dependency scanner if scanner can build a VFS that contains all inputs.

Standalone clang currently doesn't import anything from the WinSDK using clang modules, this is all just for Swift for now.
Can the scanner add these files into the VFS of inputs it constructs? I think that should be fine if I'm understanding what you're suggesting, so after scanning Swift wouldn't have to setup the overlay anymore if the scanner sets up the VFS with these files.

[cachemeifyoucan]

Seems fine to me if windows clang has no plan to support module in winSDK for now.

[egorzhdan]

LGTM modulo the comment from @cachemeifyoucan

[hjyamauchi]

I'd like to drive this PR to merge on behalf of @hyp
@hyp I took the liberty of pushing an improvement for this PR/branch
@cachemeifyoucan can you take another look? Your comment should be addressed.
CC @egorzhdan please take another look
@swift-ci please test

[cachemeifyoucan]

LGTM

[hjyamauchi]

Thanks @cachemeifyoucan
The CI got a lot of assertion failures:

swift-frontend: /home/build-user/llvm-project/clang/lib/Lex/HeaderSearch.cpp:162: std::vector<bool> clang::HeaderSearch::collectVFSUsageAndClear() const: Assertion `VFSUsage.size() == getHeaderSearchOpts().VFSOverlayFiles.size() && "A different number of RedirectingFileSystem's were present than " "-ivfsoverlay options passed to Clang!"' failed.

So something needs to be fixed further :)

[hjyamauchi]

Please test with following PR:
swiftlang/llvm-project#10075

@swift-ci Please test

[hjyamauchi]

Please test with following PR:
swiftlang/llvm-project#10075

@swift-ci Please test Linux Platform

[hjyamauchi]

@cachemeifyoucan @egorzhdan @compnerd I sent this PR for review as an alternative to this PR because the win-crt test in this PR doesn't pass in this PR. Can you take a look?