-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add WTF and bmalloc modulemaps #45763
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
base: main
Are you sure you want to change the base?
Conversation
EWS run on previous version of this PR (hash f4f675b) |
EWS run on previous version of this PR (hash c1e4655) |
EWS run on previous version of this PR (hash af53ea8) |
EWS run on previous version of this PR (hash 0c10219) |
EWS run on previous version of this PR (hash 236c9c8) |
EWS run on previous version of this PR (hash 1299191) |
EWS run on previous version of this PR (hash 3bae365) |
1299191
to
79acbaf
Compare
EWS run on previous version of this PR (hash 79acbaf) |
EWS run on previous version of this PR (hash bfef518) |
EWS run on previous version of this PR (hash 69e657d) |
EWS run on previous version of this PR (hash 5b90a7d) |
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 don't think having massive WTF.modulemap etc. is the right solution. We will not update it every time we add / modify WTF, that's significant development regression.
EWS run on previous version of this PR (hash 0776a4d) |
EWS run on previous version of this PR (hash 9dbf400) |
General status update here for anyone following along: we still need to land this, but it's blocked on some problems which only occur in cloud builds. https://bugs.webkit.org/show_bug.cgi?id=294673 is the main thing, but there's also the problem with inline locks which accounts for a bonus commit temporarily parked in this PR. I'll mark this as draft until it's ready for re-review. |
EWS run on previous version of this PR (hash 32345de) |
EWS run on previous version of this PR (hash d92aa76) |
EWS run on previous version of this PR (hash 37837b1) |
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.
Really appreciate the detailed message. This looks good to me, though I won't r+ while it's in draft in case you have other changes planned.
For the sake of clarity for reviewers, you might want to add me as a coauthor?
Co-authored-by: Elliott Williams <[email protected]>
at the bottom of the commit message should make GitHub show it. WebKit doesn't have official policies on coauthorship, though.
(For other reviewers: I worked on the build script that reads modulemap.toml
and the Xcode integration.)
EWS run on previous version of this PR (hash ff7d1b4) |
EWS run on previous version of this PR (hash 69885e9) |
EWS run on previous version of this PR (hash f523c25) |
EWS run on previous version of this PR (hash 96aba6f) |
EWS run on previous version of this PR (hash 00e8bd3) |
EWS run on previous version of this PR (hash 3690739) |
EWS run on previous version of this PR (hash 7e74dbc) |
https://bugs.webkit.org/show_bug.cgi?id=292791 rdar://151023888 Reviewed by NOBODY (OOPS!). This commit adds modulemap files for WTF and bmalloc. These are necessary because Swift/C++ interop interprets C++ headers using the "-fmodules" option, interpreting the C++ codebase as clang modules. Ultimately this PR results in just two new module.modulemap files alongside the headers. However, this can cause build time differences in all the transitive dependencies of WTF and bmalloc because they'll interpret header files differently. The approach taken for the two modulemaps is different. * bmalloc has a hard-coded modulemap, defining a single monolithic module. * WTF has an autogenerated modulemap, where each header typically resides in a small submodule. The approach of many small submodules was previously used in WebKit_Internal, and is used here because Swift/C++ interoperability otherwise tries to interpret the whole module, and that results in errors such as: /usr/include/c++/v1/__memory/construct_at.h:47:58: note: candidate template ignored: substitution failure [with _Tp = WTF::TextBreakIterator, _Args = <const WTF::TextBreakIterator &>]: call to deleted constructor of 'WTF::TextBreakIterator' _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp* __construct_at(_Tp* __location, _Args&&... __args) { This is presumed to be rdar://148760413 or related overinstantiation of templates. We'll likely need to solve those in future, but meanwhile splitting the module into lots of submodules limits the blast radius of such issues and allows us to make progress with Swift. Why is one manually generated and the other autogenerated? Per the comments in the bmalloc modulemap, we only want to expose a hand-curated set of headers in the bmalloc module, and therefore there's no benefit to autogenerating it. It's specifically the dependency on a certain value of OS_UNFAIR_LOCK_INLINE which forces this decision, and that's discussed in rdar://153287512. Whereas, for WTF we want to expose all the headers, and thus evolving the modulemap to add new headers and remove older headers would be error-prone and time consuming - hence we autogenerate. For the WTF mdoulemap, we use a script and a TOML file that describes the quirks of given header files (for example, whether they're interdependent or can be used in plain C contexts). See the TOML files in this PR and their comments for a description of these quirks. Thanks to Elliott Williams for writing the script. The script converts a tapi filelist-style header listing into a modulemap, supporting both monolithic and submodule layouts. This process works well for libraries like WTF which have no umbrella header, and which have no concept of public vs private API. We do not export the modulemaps to the build output on pre-OS X 15; doing so confuses the build of PALSwift. This PR is the second attempt at landing modulemaps for WTF and bmalloc. The last time didn't work because it turned out that we needed to resolve three issues first, to unbreak downstream consumers: * We needed to move away from reliance on UCHAR_TYPE in WTF. (rdar://153817245). * Downstream consumers of WebKit were previously using some WTF definitions without including WTF headers. That ceases to work when WTF becomes a module; we need to land those fixes before landing these modulemaps. (rdar://154081231) * WebKit#45625 resolves an issue with AppServerSupport which starts to appear once a modulemap is in place. There were also small fixes to the modularization of various system dependencies included within WebKit. (Because some of these fixes in WebKit's dependencies have only landed in recent SDKs, we do not export the modulemaps when older SDKs are in use.) * Source/WTF/Configurations/WTF.xcconfig: * Source/WTF/Configurations/modulemap.toml: Added. * Source/WTF/Scripts/modulemap-from-tapi-filelist.py: Added. * Source/WTF/WTF.xcodeproj/project.pbxproj: * Source/WTF/wtf/Compiler.h: * Source/WTF/wtf/SIMDUTF.cpp: simdutf_impl.cpp.h should not be treated as a library header, and it confuses the generator. It's only used to compile this file, so change it to a local quote-include. * Source/WTF/wtf/SmallMap.h: * Source/WebCore/PAL/pal/spi/cg/CoreGraphicsSPI.h: * Source/WebKit/WebKitSwift/IdentityDocumentServices/WKIdentityDocumentPresentmentDelegate.h: * Source/bmalloc/Configurations/bmalloc.xcconfig: * Source/bmalloc/Configurations/module.modulemap: Added. * Source/bmalloc/bmalloc.xcodeproj/project.pbxproj: * Source/bmalloc/libpas/src/libpas/pas_darwin_spi.h: Co-authored-by: Elliott Williams <[email protected]> * Source/bmalloc/libpas/src/libpas/pas_scavenger.h: * Source/bmalloc/libpas/src/libpas/pas_system_heap.h:
EWS run on current version of this PR (hash 2258c33) |
https://bugs.webkit.org/show_bug.cgi?id=292791
rdar://151023888
Reviewed by NOBODY (OOPS!).
This commit adds modulemap files for WTF and bmalloc. These are necessary because Swift/C++ interop interprets C++ headers using the "-fmodules" option, interpreting the C++ codebase as clang modules.
The approach taken for the two modulemaps is slightly different. bmalloc defines a single monolithic module, the simplest approach. For WTF we choose a more complex approach, as previously followed by the WebKit_Internal module, where the module is divided into many small submodules. This approach is chosen because Swift/C++ interoperability otherwise tries to interpret the whole module, and that results in errors such as:
We'll likely need to solve those in future, but meanwhile splitting the module into lots of submodules limits the blast radius of such issues and allows us to make progress with Swift.
Notes on some specific parts of the WTF modulemap:
This change also specifies UCHAR_TYPE in GCC_PREPROCESSOR_DEFINITIONS. This definition is supplied also in a prefix header injected into WebKit builds. However, prefix headers are not applied in module builds, so we need to specify this another way. In an ideal world we'd specify UCHAR_TYPE in one place instead of two, but removing its definition from the prefix header turns out to break downstream non-modularized consumers of WebKit which also need to ensure this definition is present. If and when such consumers start to enable clang modules or Swift/C++ interoperability, they'll need to tackle this problem and we'll need to work out a solution, but for now, continuing to define this within the prefix header avoids a compatibility break.
This PR is the second attempt at landing modulemaps for WTF and bmalloc (the first was #45181). The last time didn't work because it turned out that we needed to resolve three issues first, to unbreak downstream consumers:
Changes:
2258c33
🛠 playstation