-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[WinSDK] Modularize <guiddef.h>
#83914
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 |
|
Ugh, looks like this header is under |
|
Yeah, the GUID definition is shared between um and km. We currently get it through transitive includes. |
6388790 to
862b799
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still support older SDKs? This might be a bit simpler if we don't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we do support older SDKs, which is why this gets tricky, but only from 10.0.17763 (Windows 10 RS5).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, so we should be able to remove this check then.
862b799 to
d154f8f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should rename this to winsdk_um.modulemap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable! Renamed it.
d154f8f to
014a0ec
Compare
This fixes modularization errors that arise when importing a C++ header that contains `#include <guiddef.h>`, which might hijack this header from the WinSDK module where it belongs.
014a0ec to
28f9db7
Compare
This is a companion to swiftlang/swift#83914.
|
swiftlang/swift-installer-scripts#458 @swift-ci please test Windows |
|
@swift-ci please smoke test macOS |
|
@swift-ci please smoke test Linux |
|
swiftlang/swift-installer-scripts#458 @swift-ci please build toolchain Windows ARM64 |
|
swiftlang/swift-installer-scripts#458 @swift-ci please build toolchain Windows |
|
The toolchain seems to work for me locally – @compnerd would you be OK with landing this? |
compnerd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builds, works. I think that it's a bit unfortunate that we have to split it up, but so be it.
Thanks for doing this!
This PR changes the name of the clang module introduced in #83914 from `_GUID` to `_GUIDDef`. This change is necessary because the Windows SDK declares a type named `_GUID` and Swift gets confused when a module and a type have the same name. This confusion is the motivation for [SE-0491](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0491-module-selectors.md), but we can't just wait for a fix because the new module is a source-breaking change and clients won't be able to adopt the proposed syntax for some time.
This PR changes the name of the clang module introduced in #83914 from `_GUID` to `_GUIDDef`. This change is necessary because the Windows SDK declares a type named `_GUID` and Swift gets confused when a module and a type have the same name. This confusion is the motivation for [SE-0491](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0491-module-selectors.md), but we can't just wait for a fix because the new module is a source-breaking change and clients won't be able to adopt the proposed syntax for some time.
This fixes modularization errors that arise when importing a C++ header that contains
#include <guiddef.h>, which might hijack this header from the WinSDK module where it belongs.