-
Notifications
You must be signed in to change notification settings - Fork 4k
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
[WIP] [2/5] support C++20 Modules, add C++20 related actions #22427
base: master
Are you sure you want to change the base?
[WIP] [2/5] support C++20 Modules, add C++20 related actions #22427
Conversation
src/main/java/com/google/devtools/build/lib/rules/cpp/ArtifactCategory.java
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/rules/cpp/Cpp20ModuleDepMapAction.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/rules/cpp/Cpp20ModuleDepMapAction.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/rules/cpp/Cpp20ModuleHelper.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/rules/cpp/Cpp20ModuleDepMapAction.java
Outdated
Show resolved
Hide resolved
} | ||
return moduleDep; | ||
} | ||
public static boolean isCpp20ModuleCompilationAction(String actionName) { |
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.
This looks unused.
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.
the helper function isCpp20ModuleCompilationAction
is used in CppCompileAction
@Override
public boolean discoversInputs() {
return Cpp20ModuleHelper.isCpp20ModuleCompilationAction(actionName) ||
shouldScanIncludes || getDotdFile() != null || shouldParseShowIncludes();
}
src/main/java/com/google/devtools/build/lib/rules/cpp/Cpp20ModuleHelper.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/rules/cpp/Cpp20ModuleDepMapAction.java
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,222 @@ | |||
// Copyright 2014 The Bazel Authors. All rights reserved. |
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.
Examining the class, I don't see any justification, why this needs to be a new native Bazel action. There are no fields exposed from the action. All the inputs and the output file are created like in any other regular SpawnAction.
Please move the whole implementation of this action into a tool, i.e. a cc_binary, and use a regular SpawnAction (without creating a new native action).
PS Feel free to ignore other comments on the file. They are irrelevant if the file isn't part of Bazel binary.
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 you mean write starlark in builtins_bzl, i.e main/starlark/builtins_bzl/common/cc/cc_binary.bzl
?
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 think if you try doing it in the builtins_bzl
it would become more clear what I mean.
I think this whole action can be constructed in Starlark by calling: ctx.actions.run
. The execute
method seems complex. But imagine you have a shell tools, a cc_binary, that accepts arguments with input files and does exactly what execute
method does, writing given output files. Then you can use that in ctx.actions.run(tools = ...)
.
The code for the tool can go into /tools/cc
.
You need to bring in an implicit dependency that points to such a tool. You can do that on cc_toolchain
. Follow the example of _grep_include
.
If you can do the ctx.actions.run
in builtins_bzl
, that's perfect.
If you can't, then you can still natively construct a SpawnAction with everything that ctx.actions.run
does (but without creating a dedicated class for it.
src/main/java/com/google/devtools/build/lib/rules/cpp/Cpp20ModulesInfoAction.java
Outdated
Show resolved
Hide resolved
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.
Also please add tests in the same PR for the newly introduced tools.
9cc6122
to
c878c7a
Compare
I split the XXL PR #19940 into several small patches.
This is the second patch of Support C++20 Modules, I add C++20 related actions
A two-phase compilation is employed in this patch.
c++20-deps-scanning
scan source files and get the C++20 Modules dependencies information. all are stored in<filename>.ddi
filec++20-module-compile
compile the C++20 Modules Interfaces to Built Module Interface (or BMI).<filename>.cppm -> <filename>.pcm
c++20-module-codegen
compile the BMI to object file.<filename>.pcm -> <filename>.o
some helper tools are also added.
agg-ddi
Description
agg-ddi
is a tool that aggregates C++20 module information from multiple sources and processes DDI files to generate a consolidated output containing module paths and their dependencies.Usage
Command Line Arguments
-m <cpp20modules-info-file>
: Path to a JSON file containing C++20 module information.-d <ddi-file> <pcm-path>
: Path to a DDI file and its associated PCM path.-o <output-file>
: Path to the output file where the aggregated information will be stored.--verbose
: Optional flag to enable verbose output.Example
gen-modmap
Description
gen-modmap
is a tool that generates a module map from a DDI file and C++20 modules information file. It creates two output files: one for the module map and one for the input module paths.Usage
Command Line Arguments
<ddi-file>
: Path to the DDI file containing module dependencies.<cpp20modules-info-file>
: Path to the JSON file containing C++20 modules information.<output-file>
: Path to the output file where the module map will be stored.Example
This command will generate two files:
modmap
: containing the module map.modmap.input
: containing the module paths.