bazel
bazel copied to clipboard
bazelbuild
[4/5] support C++20 Modules, support one phase compilation
I split the XXL PR https://github.com/bazelbuild/bazel/pull/19940 into several small patches. This is the fourth patch of Support C++20 Modules, which supports one phase compilation.
Changes of build graph
First, files are scanned, resulting in the creation of .ddi files. If the compiler being used is Clang, the tool clang-scan-deps is employed.
Next, the generated .ddi files are aggregated into a .CXXModules.json file using the aggregate-ddi tool.
Following this, the .CXXModules.json file, along with the .ddi files, is used to generate .modmap files through the generate-modmap tool.
Finally, the source files are compiled into object files. If a file is a module interface, a corresponding module file is produced as a byproduct.
The following diagram illustrates the scanning process.
┌────────┐ ┌────────┐ ┌────────┐
│foo.cppm│ │bar.cppm│ │main.cc │
└────┬───┘ └────┬───┘ └────┬───┘
│ c++-module-deps-scanning │ │
│ │ │
┌────▼───┐ ┌────▼───┐ ┌────▼───┐
│foo.ddi │ │bar.ddi │ │main.ddi│
└────┬───┘ └────┬───┘ └────┬───┘
│ │ │
└───────────────────────────┼───────────────────────────┘
│aggregate-ddi
┌────────▼───────────┐
│demo.CXXModules.json│
└────────────────────┘
The following diagram illustrates the compile process of foo.cppm.
┌────────────────────┐
│demo.CXXModules.json│
└─────┬──────────────┘
│
│
┌───────┐ │ ┌───────────┐
┌─►│foo.ddi├─────────┤ │ bar.cppm │
│ └───────┘ │ └─────┬─────┘
│ │ │
│ │ │
│ │ │
c++-module-deps-scanning │ │ │
┌────────┐ │ ┌───────┐ ┌─────▼────┐ ┌────▼─────┐
│foo.cppm├─────────────────────┴─►│foo.d │ │foo.modmap│ │ bar.pcm │
└───┬────┘ └──┬────┘ └────┬─────┘ └────┬─────┘
│ │ │ │
│ │ │ │
│◄───────────────────────────────┴─────────────┴─────────────────┘
│ c++20-module-compile
┌────────▼─────────┐
│ foo.pcm && foo.o │
└──────────────────┘
Changes of compile
I modified the CppCompileAction to include the relevant context, inputs, and outputs.
Context: Module files and .CXXModules.json files
Inputs: Module files, .modmap file, and .modmap.input file
Outputs: Module files and .CXXModules.json files
The primary difference in CppCompileAction is the addition of the computeUsedCpp20Modules function.
The restart mechanism is utilized; if the necessary C++20 Modules files are not ready during compilation, the current compilation will exit and wait for an appropriate time to recompile.
@katre @lberki I'm not sure if you got the notification, but ping... If this PR [4/5] can be merged, we can start using C++ modules with bazel... in eager anticipation :-)
Sorry, I didn't notice this because I don't know anything about C++ modules. Re-assigning to @comius, apologies for not noticing this earlier.
I'm testing this patch. If this patch ready, I will remove the [WIP] prefix.
Hey, this is quite a large PR and will take me some time to review it completely. Thanks for your patience.
Hey, this is quite a large PR and will take me some time to review it completely.
Please also review https://github.com/bazelbuild/bazel/pull/22744, which contains a small patch split from this one.
Hi @comius @trybka, is there any chance this feature could make it in soon? We are keenly awaiting it in one of the bazel 8 releases
@comius @trybka can we expect to see this before 2025?
Hey, yes. I was under a very heavy load for Bazel 8 release (removal of all the ruleset except C++). It's just winding down, so expect more going on in the following weeks.
@comius @trybka can we expect to see this before 2025?
Hey, yes. I was under a very heavy load for Bazel 8 release (removal of all the ruleset except C++). It's just winding down, so expect more going on in the following weeks.
Not a push, but it's Dec 20.
@comius @trybka It is January 2025. Many people are waiting for this PR to land.
@comius @trybka we are eagerly awaiting support for modules in Bazel. If you don't have time to review (this PR has now been waiting for 4 months), is there some other way it can progress?
I conferred with @comius and I'm going to take point. As I understand it, it doesn't merge cleanly in our repo so I anticipate some back and forth to get it working.
@trybka joining others in voicing excitement in getting c++ module support into bazel. Anything we can do to help get this merged? Any rough ETA so we can plan accordingly?
Much appreciated!
I conferred with @comius and I'm going to take point. As I understand it, it doesn't merge cleanly in our repo so I anticipate some back and forth to get it working.
ping @trybka
Internal checks mostly look good. AFAICT no adverse breakages, but I'm re-running flaky failing tests anyway.
I don't have a good solution yet for the failing test. I'm wondering if it would be possible to gate the tests that require cpp20 modules support on some other bit that signals that we have the requisite tools available? (Maybe it would be sufficient to try to configure the action once and fail gracefully).
I can imagine also wanting something in the crosstool itself (e.g. feature(name='supports_cpp20_modules')) for the non-{clang,msvc-cl,gcc} compilers or toolchains not yet willing to adopt support.
Here's the current failure:
Basically because we don't intend to support C++20 modules immediately, we had stubbed out the tools provided in semantics.bzl -- e.g. here: https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/common/cc/semantics.bzl;l=137-149;drc=f777a3c33151b5a3ae2295f1ba6ea94653840665
Ours looks like this:
semantics = struct(
...
cpp_modules_tools = lambda: {},
...
)
With this PR, tests like CppModulesConfiguredTargetTest.testSameModuleInterfacesFileInCcLibraryTwice fail because those tools are not provided, e.g. with:
java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=<somelabel>, config=BuildConfigurationKey[<somekey>]}' (requested by nodes 'ConfiguredTargetKey{label=<someotherlabel>, config=BuildConfigurationKey[<somekey>]}')
...
Caused by: net.starlark.java.eval.Starlark$UncheckedEvalException: NullPointerException thrown during Starlark evaluation (<somelabel>)
at <starlark>.compile(<builtin>:0)
at <starlark>._compile(/virtual_builtins_bzl/common/cc/cc_common.bzl:755)
at <starlark>._cc_library_impl(/virtual_builtins_bzl/common/cc/cc_library.bzl:56)
Caused by: java.lang.NullPointerException: the tool 'aggregate-ddi' cannot be null
at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:988)
at com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.createAggDdiAction(CcCompilationHelper.java:1852)
at com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.createCcCompileActionsWithCpp20ModuleHelper(CcCompilationHelper.java:1057)
at com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.createCcCompileActionsWithCpp20Module(CcCompilationHelper.java:987)
at com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.compile(CcCompilationHelper.java:835)
at com.google.devtools.build.lib.rules.cpp.CcModule.compile(CcModule.java:2255)
With this PR, tests like
CppModulesConfiguredTargetTest.testSameModuleInterfacesFileInCcLibraryTwicefail because those tools are not provided
hi @trybka , thank you for sharing the detailed failure messages.
Would it be possible for us to temporarily disable the failing tests in order to quickly merge this patch?
Yes that seems reasonable. If you want to do that I can merge your changes, otherwise I'll do something tomorrow.
Yes that seems reasonable. If you want to do that I can merge your changes, otherwise I'll do something tomorrow.
hi @trybka , I have removed the CppModulesConfiguredTargetTest . please merge the patch.
Can you try rebasing? There are some non-trivial changes to input discovery here that I think you should probably take into consideration. Notably, the most recent change to CppCompileAction was to fix "old" (i.e. Clang Header) modules w.r.t. discoverInputs for the purpose of aspects.
Ack, thanks. I've re-imported, and re-running our Global Presubmits. I should have results tomorrow (need to run off-peak hours as it invalidates all the caches). Will post an update then.
Mailed for review internally -- local presubmits all green. Global presubmit hit a snag and didn't run. Scheduled another one for this evening, will try to check it early in the AM.