[ClangImporter] Swift needs to pass -Xclang -fbuiltin-headers-in-system-modules for its module maps that group cstd headers

[ian-twilightcoder]

Swift has some module maps it overlays on Linux and Windows that groups all of the C standard library headers into a single module. This doesn’t allow clang and C++ headers to layer properly with the OS/SDK modules. clang will set -fbuiltin-headers-in-system-modules as necessary for Apple SDKs, but Swift will need to pass that flag itself when required by its module maps.

rdar://118361742

[al45tair]

Backtracing is currently just absorbing them all into its OS module

Indeed. Sadly that was necessary because of when this code gets built (it's before the Darwin or Glibc modules are available).

[ian-twilightcoder]

I'm still testing this with swiftlang/llvm-project#7619, looks like it's not working yet.

[ian-twilightcoder]

This turned out to be a bigger change, I'll clean up the PR tomorrow, I just pushed the branch to run the CI overnight.

The PR has changed completely since I originally looked.

[ian-twilightcoder]

Running one more test in swiftlang/llvm-project#7619 and then I'll put this through CI

[ian-twilightcoder]

@compnerd any chance you could help me with the Windows failures? I'm pretty sure the issue is that -fbuiltin-headers-in-system-modules isn't making it down to ModuleMapParser::parseHeaderDecl in llvm-project/clang/lib/Lex/ModuleMap.cpp, but I don't have a way to actually debug on Windows.

I don't think that is the case at all if you look at the error logs.

:0: error: cannot open file 'C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.34.31933\include\vcruntime.apinotes' (no such file or directory)

I think that the changes might not be honouring the VFS overlay as modifying the VS was undesirable, so this injection is now done through an injected VFS overlay by the frontend.

🤔 I only see that error in a few of the failed tests, is -code-completion eating that error in the other tests? Which way is the vfs supposed to be seen in those tests? Via the -Xcc -ivfsoverlay argument or injected by swift::getClangInvocationFileMapping? Why is -Xcc -ivfsoverlay passed instead of -vfsoverlay? I wonder if that was making the command line overlay be ignored until I started passing -strict-implicit-module-context which was needed for -Xcc -fbuiltin-headers-in-system-modules to make it through (otherwise InterfaceSubContextDelegateImpl in ModuleInterfaceLoader.cpp drops ClangImporterOptions.ExtraArgs). Maybe I should try taking out -Xcc -ivfsoverlay from the lit config since I think it was probably being ignored anyway, but keep -strict-implicit-module-context where we use -vfsoverlay.

[ian-twilightcoder]

@compnerd any chance you could help me with the Windows failures? I'm pretty sure the issue is that -fbuiltin-headers-in-system-modules isn't making it down to ModuleMapParser::parseHeaderDecl in llvm-project/clang/lib/Lex/ModuleMap.cpp, but I don't have a way to actually debug on Windows.

I don't think that is the case at all if you look at the error logs.

:0: error: cannot open file 'C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.34.31933\include\vcruntime.apinotes' (no such file or directory)

I think that the changes might not be honouring the VFS overlay as modifying the VS was undesirable, so this injection is now done through an injected VFS overlay by the frontend.

🤔 I only see that error in a few of the failed tests, is -code-completion eating that error in the other tests? Which way is the vfs supposed to be seen in those tests? Via the -Xcc -ivfsoverlay argument or injected by swift::getClangInvocationFileMapping? Why is -Xcc -ivfsoverlay passed instead of -vfsoverlay? I wonder if that was making the command line overlay be ignored until I started passing -strict-implicit-module-context which was needed for -Xcc -fbuiltin-headers-in-system-modules to make it through (otherwise InterfaceSubContextDelegateImpl in ModuleInterfaceLoader.cpp drops ClangImporterOptions.ExtraArgs). Maybe I should try taking out -Xcc -ivfsoverlay from the lit config since I think it was probably being ignored anyway, but keep -strict-implicit-module-context where we use -vfsoverlay.

Awesome that made the "can't find file" errors all go away and the missing string one too. There's just one last "rebuilt a thing we didn't think we'd need to rebuild" error which isn't nearly so bad. I'll rerun the tests and see if it goes away.

[ian-twilightcoder]

@compnerd any chance you could help me with the Windows failures? I'm pretty sure the issue is that -fbuiltin-headers-in-system-modules isn't making it down to ModuleMapParser::parseHeaderDecl in llvm-project/clang/lib/Lex/ModuleMap.cpp, but I don't have a way to actually debug on Windows.

I don't think that is the case at all if you look at the error logs.

:0: error: cannot open file 'C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.34.31933\include\vcruntime.apinotes' (no such file or directory)

I think that the changes might not be honouring the VFS overlay as modifying the VS was undesirable, so this injection is now done through an injected VFS overlay by the frontend.

🤔 I only see that error in a few of the failed tests, is -code-completion eating that error in the other tests? Which way is the vfs supposed to be seen in those tests? Via the -Xcc -ivfsoverlay argument or injected by swift::getClangInvocationFileMapping? Why is -Xcc -ivfsoverlay passed instead of -vfsoverlay? I wonder if that was making the command line overlay be ignored until I started passing -strict-implicit-module-context which was needed for -Xcc -fbuiltin-headers-in-system-modules to make it through (otherwise InterfaceSubContextDelegateImpl in ModuleInterfaceLoader.cpp drops ClangImporterOptions.ExtraArgs). Maybe I should try taking out -Xcc -ivfsoverlay from the lit config since I think it was probably being ignored anyway, but keep -strict-implicit-module-context where we use -vfsoverlay.

Awesome that made the "can't find file" errors all go away and the missing string one too. There's just one last "rebuilt a thing we didn't think we'd need to rebuild" error which isn't nearly so bad. I'll rerun the tests and see if it goes away.

I'm suspicious of the error I'm getting from -emit-pcm/-verify where it looks like the pcm is being emitted without -fbuiltin-headers-in-system-modules.

$ "t:\\1\\bin\\swift-frontend.exe" "-target" "x86_64-unknown-windows-msvc" "-module-cache-path" "T:\1\swift-test-results\x86_64-unknown-windows-msvc\clang-module-cache" "-vfsoverlay" "T:/1/tools/swift\stdlib\windows-vfs-overlay.yaml" "-strict-implicit-module-context" "-Xcc" "-Xclang" "-Xcc" "-fbuiltin-headers-in-system-modules" "-swift-version" "4" "-define-availability" "SwiftStdlib 9999:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999" "-define-availability" "SwiftStdlib 5.0:macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2" "-define-availability" "SwiftStdlib 5.1:macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0" "-define-availability" "SwiftStdlib 5.2:macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4" "-define-availability" "SwiftStdlib 5.3:macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0" "-define-availability" "SwiftStdlib 5.4:macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5" "-define-availability" "SwiftStdlib 5.5:macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0" "-define-availability" "SwiftStdlib 5.6:macOS 12.3, iOS 15.4, watchOS 8.5, tvOS 15.4" "-define-availability" "SwiftStdlib 5.7:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0" "-define-availability" "SwiftStdlib 5.8:macOS 13.3, iOS 16.4, watchOS 9.4, tvOS 16.4" "-define-availability" "SwiftStdlib 5.9:macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0" "-define-availability" "SwiftStdlib 5.10:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999" "-define-availability" "SwiftStdlib 5.11:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999" "-typo-correction-limit" "10" "-typecheck" "-verify" "-Xcc" "-Xclang" "-Xcc" "-fallow-pcm-with-compiler-errors" "-Xcc" "-fmodule-file=T:\1\tools\swift\test-windows-x86_64\ClangImporter\AllowErrors\Output\invalid-pcm.swift.tmp/m.pcm" "T:\1\tools\swift\test-windows-x86_64\ClangImporter\AllowErrors\Output\invalid-pcm.swift.tmp/use.swift"
# command stderr:
<unknown>:0: error: builtin headers belong to system modules, and _Builtin_ modules are ignored for cstdlib headers was disabled in PCH file but is currently enabled

I'm also having too much trouble trying to debug this without Windows, so I think what I'm going to do is try setting that flag in the MSVC driver/toolchain in clang for now. When I have a little breathing room, we'll either have removed that flag from Darwin and I'll have a better change at debugging how -fbuiltin-headers-in-system-modules flows through Swift, or we can fix the ucrt module to not require -fbuiltin-headers-in-system-modules, which we should do for all of our module maps anyway. Testing with the workaround.

[ian-twilightcoder]

@swift-ci smoke test

[ian-twilightcoder]

@swift-ci test

[al45tair]

LGTM.