Skip to content

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

adetaylor
Copy link
Contributor

@adetaylor adetaylor commented May 22, 2025

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:

  /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) {

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:

  • HashMap and RobinHoodHashTable.h are interdependent so need to be colocated in a module.
  • A few headers (e.g. Platform) are used in contexts without C++11 support, unlike other parts of WTF.

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:

  • Define UCHAR_TYPE in two places, as mentioned above.
  • 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.
  • [Modules] Move <AppServerSupport/OSLaunchdJob.h> out of WTF headers #45625 resolves an issue with AppServerSupport which starts to appear once a modulemap is in place.

Changes:

  • Source/WTF/WTF.xcodeproj/project.pbxproj:
  • Source/WTF/wtf/module.modulemap: Added.
  • Source/bmalloc/bmalloc.xcodeproj/project.pbxproj:
  • Source/bmalloc/module.modulemap: Added.

00e8bd3

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ❌ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 22, 2025
@adetaylor adetaylor force-pushed the modulemap branch 2 times, most recently from 1299191 to 79acbaf Compare May 27, 2025 10:22
@adetaylor adetaylor marked this pull request as ready for review May 28, 2025 11:40
@adetaylor adetaylor requested review from emw-apple and a team as code owners May 28, 2025 11:41
Copy link
Member

@Constellation Constellation left a 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.

@emw-apple
Copy link
Contributor

If it is necessary, it must be generated during the build. It is not acceptable for us to maintain these non-autogenerated modulemap.

I worked with @adetaylor on this, and contributed a script that generates the modulemap for each project using a small config file. The end result should be that contributors can add WTF and bmalloc headers without having to think about the modulemap — the config only needs to be tweaked for headers that have special grouping requirements or attributes, such as needing to be accessible from C.

@Constellation, does that seem reasonable to you?

@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 9dbf400. Live statuses available at the PR page, #45763

@adetaylor
Copy link
Contributor Author

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.

@adetaylor adetaylor marked this pull request as draft June 19, 2025 13:33
Copy link
Contributor

@emw-apple emw-apple left a 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.)

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 adapting 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.

* 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/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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merging-blocked Applied to prevent a change from being merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants