-
Notifications
You must be signed in to change notification settings - Fork 4.4k
[4/5] support C++20 Modules, support one phase compilation #22553
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
[4/5] support C++20 Modules, support one phase compilation #22553
Conversation
cba3e03 to
dde1f8e
Compare
7228856 to
93f963c
Compare
|
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 |
93f963c to
97cd7b3
Compare
|
@PikachuHyA rules_cc v0.2.9 is out now |
d024b2f to
fa6672b
Compare
bf4d91f to
2c915f1
Compare
@trybka ping |
|
Hi there, Began the import earlier this week, just re-pulled based on your commits from yesterday. Currently fixing up some internal tests and then sending out for final review. 🤞 |
|
Just a quick status update: as you can see there are now a number of merge conflicts. I'm resolving those in my local branch to ease the back and forth. Had to fix a few more tests and the automatic merge resolution had some fun errors. |
|
I've done my best to resolve merge conflicts but I have a few test failures, @PikachuHyA @fmeum can you take a look? https://buildkite.com/bazel/google-bazel-presubmit/builds/97272 (should be able to view the current state in https://bazel-review.googlesource.com/c/bazel/+/294090 ? ) |
|
@trybka With this diff --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java
@@ -695,6 +695,9 @@ public class CcStarlarkInternal implements StarlarkValue {
boolean needsIncludeValidation,
String toolchainType)
throws EvalException, TypeException {
+ if (modmapInputFile == null || modmapInputFile == Starlark.NONE) {
+ throw new EvalException("modmap_input_file parameter is required");
+ }
CoptsFilter coptsFilter =
createCoptsFilter(
Starlark.isNullOrNone(coptsFilterObject) ? null : (String) coptsFilterObject);I get a stack trace in rules_cc. Is it possible that the same patch also has to be applied to and released with rules_cc? I have to admit that I don't fully understand the split between Bazel builtins and rules_cc anymore. Edit: The stack trace: |
|
Yes, I think you're right. For now I will disable the test until this rolls out into rules_cc. |
@trybka Thanks for your work. I have a basic question. I’m not familiar with Gerrit. How can I check out the change at https://bazel-review.googlesource.com/c/bazel/+/294090 on my development machine, including fetching your updated patchsets? Also, regarding the merge conflicts and failing tests: should I resolve the GitHub PR conflicts locally and push the updates to this PR? |
|
From a bazelbuild/bazel git checkout, I think you can just do this: (I found the commit hash on the CI status page) |
No, I have a pile of merge conflicts already in my local copy. FWICT the current test failure is just because it's not in rules_cc yet, so we're going to disable it for now. I think that's the only failing test at the moment. |
There is a "Download" button at the top of the file list that provides you with with various git commands you can run to get the change.
It's also the only one :-) But we can easily reenable it and if this PR doesn't break any other tests, we can merge now and do that later. |
|
For those of us anxiously following from the side lines, does this mean C++ modules are now supported? I'm unsure where the 5/5 of bug resides, or whether this is actually all that's needed (or if this is being rolled out in a downstream bazel release). Thank you all for your hard work! |
|
Yes, C++20 modules are supported with
@PikachuHyA please correct me if I missed anything. And thanks again for the contribution! |
@jwhpryor Thanks for following this work. With this PR merged, Bazel now offers limited support for C++20 Modules. The current limitation is that each The work in #22555 will not block anyone who wants to use C++20 Modules with Bazel. I will update that PR later. Currently only Clang supports two-phase compilation; GCC/MSVC do not yet. Our next priorities include broader module features (e.g. support for |
|
I have to rain on your parade here, I'm afraid: in my capacity as the person whose job this week is to keep our internal builds running, http://www.github.com/bazelbuid/bazel/pull/22553 breaks a few things. I asked @comius to investigate and I hope it's just some tiny little snag, but for now, that's just a hope and I can't exclude the possibility that a significant amount of work will still need to be done (and I don't know yet where, either) |
|
@PikachuHyA is there a hello_world example available showing use of C++20 modules and shared libraries? I found the disabled cc_integration_test and the attribute docstrings but couldn't figure it out from there. |
@peakschris |
I split the XXL PR #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
.ddifiles. If the compiler being used is Clang, the toolclang-scan-depsis employed.Next, the generated
.ddifiles are aggregated into a.CXXModules.jsonfile using theaggregate-dditool.Following this, the
.CXXModules.jsonfile, along with the.ddifiles, is used to generate.modmapfiles through thegenerate-modmaptool.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.
The following diagram illustrates the compile process of
foo.cppm.Changes of compile
I modified the CppCompileAction to include the relevant context, inputs, and outputs.
Context: Module files and
.CXXModules.jsonfilesInputs: Module files,
.modmapfile, and.modmap.inputfileOutputs: Module files and
.CXXModules.jsonfilesThe primary difference in CppCompileAction is the addition of the
computeUsedCpp20Modulesfunction.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.