Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

PikachuHyA
Copy link

@PikachuHyA PikachuHyA commented May 17, 2024

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.

  • the c++20-deps-scanning scan source files and get the C++20 Modules dependencies information. all are stored in <filename>.ddi file
  • the c++20-module-compile compile the C++20 Modules Interfaces to Built Module Interface (or BMI). <filename>.cppm -> <filename>.pcm
  • the 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

agg-ddi -m <cpp20modules-info-file1> -m <cpp20modules-info-file2> ... -d <ddi-file1> <path/to/pcm1> -d <ddi-file2> <path/to/pcm2> ... -o <output-file> [--verbose]

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

agg-ddi -m module-info1.json -m module-info2.json -d ddi1.json /path/to/pcm1 -d ddi2.json /path/to/pcm2 -o output.json --verbose

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

gen-modmap <ddi-file> <cpp20modules-info-file> <output-file>

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

gen-modmap ddi.json cpp20modules-info.json modmap

This command will generate two files:

  • modmap: containing the module map.
  • modmap.input: containing the module paths.

@PikachuHyA PikachuHyA requested a review from lberki as a code owner May 17, 2024 07:38
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels May 17, 2024
@fmeum fmeum mentioned this pull request May 17, 2024
}
return moduleDep;
}
public static boolean isCpp20ModuleCompilationAction(String actionName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unused.

Copy link
Author

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();
  }

@@ -0,0 +1,222 @@
// Copyright 2014 The Bazel Authors. All rights reserved.
Copy link
Contributor

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.

Copy link
Author

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?

Copy link
Contributor

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.

@comius comius removed the request for review from lberki May 22, 2024 14:10
@comius comius self-assigned this May 22, 2024
Copy link
Contributor

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

@PikachuHyA PikachuHyA changed the title support C++20 Modules, add C++20 related actions [WIP] support C++20 Modules, add C++20 related actions May 27, 2024
@PikachuHyA PikachuHyA changed the title [WIP] support C++20 Modules, add C++20 related actions [WIP] [2/5] support C++20 Modules, add C++20 related actions May 27, 2024
@sgowroji sgowroji added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels May 29, 2024
@PikachuHyA PikachuHyA force-pushed the cxx20-modules-support-patch-2 branch from 9cc6122 to c878c7a Compare June 7, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-user-response Awaiting a response from the author team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants