-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Don't build swiftMSVCRT with MSVC #6843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@swift-ci please smoke test |
I don't think that this is the right thing to do. This is the module responsible for bridging libc to swift. This would be tantamount to not building the glibc of Darwin modules on Linux or macOS! I think that this is something where it becomes mandatory to use clang. CC @jrose-apple to get his opinion as well. |
My logic was that this is an SDK overlay so is an optional component, I.e. Nothing in the swift project depends on it, so it's also even more optional |
I don't believe that was the case. It has been a while since I poked at this, so I may be mistaken, but I believe that disabling this is significantly reduces the usefulness of the resultant swift toolchain. |
The fact that nothing in the swift standard library/overlay depends on the C library. I suppose that if we have introduced enough shims, this may work, but we are cutting out a large surface of integration by disabling this module. |
Yeah, @compnerd is right. A user can turn off the overlays if they just want a compiler, but they shouldn't require any particular compiler (and they shouldn't need to be built with the just-built Clang either). I'm not sure what you mean by "module definitions". Do you mean that there's a module map that refers to headers on disk? Or that the C/C++ source files use |
From #5904
|
I can close this, if you'd like, following the discussion above |
That doesn't really make sense to me. We don't put module maps into the system headers on Linux; we have one that's loaded explicitly by the Clang importer to categorize system headers. (In fact, I suspect that will interact poorly with a system that actually does have module maps for its system headers, but nobody's complained yet.) If we only need modules to build the Swift part of the overlay, and not any support .cpp files, I don't think we have a problem. We just need to make the correct module map like we do for Glibc. |
What do you mean by "make the correct module map"? Does this mean explicitly loading it? Sorry, I'm not too clued up into the world of modules :D |
A module is a set of headers that can be imported independently of any other headers and any macros you set before and after the import. I'm not quite sure how that extends to MSVC, where macros seem to affect everything, but we can pick some base options and stick with them by using -D flags in the importer. A module map is a file that describes one or more modules, mostly by listing and grouping their headers. We have a generated module map for Linux headers in stdlib/public/Platform/glibc.modulemap.gyb, and we could easily do something similar for the base MSVC headers. Normally module maps are found alongside headers via include paths, but they can also be explicitly included (and glibc.modulemap is). @compnerd, where does one get module maps for MSVC headers if you want to use them with Clang? Can we just bundle that? Or have our own copy? I didn't think Clang itself provided them. |
There is a static module map for parts of ucrt and the Windows SDK that need to be placed alongside the headers in the swift repository. The problem is that both of them change on a pretty frequent basis. Right now I am building against 10.0.10586.0 though need to update to 10.0.14393.0. There is no standard location for this that you can easily substitute in gyb. We can try to pull out the build values from the environment (include paths and library search paths are specified as environment variables). |
I think it's fine for us to generate it only for whatever you're building against for now, and worry about redistributable packages later. |
This uses module definitions, which are not supported by MSVC.