[3/5] support C++20 Modules, add deps-scanner and update toolchains by PikachuHyA · Pull Request #22429 · bazelbuild/bazel

PikachuHyA · 2024-05-17T08:13:38Z

Summary

I have splited the XXL PR #19940 into several smaller patches. This is the third patch to support C++20 Modules, which adds the deps-scanner tool and updates toolchains.

This patch includes:

New action names
File extensions
Build variables
Updated toolchains for compiling C++20 Modules

Action Names

Three action names have been added:

c++-module-deps-scanning
c++20-module-compile
c++20-module-codegen

When two-phase compilation is employed:

c++-module-deps-scanning: Scans source files and retrieves C++20 Modules dependencies, storing them in <filename>.ddi.
c++20-module-compile: Compiles the C++20 Modules Interfaces to a Built Module Interface (BMI), converting <filename>.cppm to <filename>.pcm.
c++20-module-codegen: Compiles the BMI to an object file, converting <filename>.pcm to <filename>.o.

When one-phase compilation is employed:

c++-module-deps-scanning: Operates similarly to two-phase compilation.
c++20-module-compile: Compiles the C++20 Modules Interfaces directly to an object file <filename>.o and produces a BMI <filename>.pcm as a byproduct.

File Extensions

We follow the file extensions preferred by different compilers, adding two new ArtifactCategorys: CPP_MODULE_GCM and CPP_MODULE_IFC.

Clang uses .pcm (CPP_MODULE, already exists).
GCC uses .gcm (CPP_MODULE_GCM, new).
MSVC uses .ifc (CPP_MODULE_IFC, new).

Following the CMake implementation, we added three extra ArtifactCategorys: CPP_MODULES_INFO, CPP_MODULES_DDI, and CPP_MODULES_MODMAP.

The .ddi file (CPP_MODULES_DDI) stores the dependencies information of one source file.
The .CXXModules.json file (CPP_MODULES_INFO) stores dependencies information for an entire target.
The .modmap file (CPP_MODULES_MODMAP) maps module names to BMIs, with different formats for each compiler.

Additionally, a special ArtifactCategory, CPP_MODULES_MODMAP_INPUT, is an auxiliary file used to easily obtain the requested BMI paths.

Build Variables

Two build variables, CPP_MODULE_MODMAP_FILE and CPP_MODULE_OUTPUT_FILE, have been added.

CPP_MODULE_MODMAP_FILE specifies the path to the .modmap file and is used by the cpp20_modmap_file_feature.
CPP_MODULE_OUTPUT_FILE specifies the output name of the BMI when one-phase compilation is employed and is used by the cpp20_module_compile_flags_feature.

Toolchains

Three action configs (cpp_module_scan_deps, cpp20_module_compile, and cpp20_module_codegen) have been added, corresponding to the action names section.

Two features (cpp_module_modmap_file_feature and cpp20_module_compile_flags_feature) have been added, corresponding to the build variables section.

Using C++20 Modules necessitates topological ordering for the compilation units. For more details, see the Discovering Dependencies section.

Considering the various compilers, I have added the deps-scanner tool. The default implementation is a script wrapper that uses different scanning methods depending on the compiler. The wrapper deps_scanner_wrapper is generated by a template file <compiler>_deps_scanner_wrapper.sh.tpl. Three template files have been added:

clang_deps_scanner_wrapper.sh.tpl
gcc_deps_scanner_wrapper.sh.tpl
mvsc_deps_scanner_wrapper.bat.tpl

For a demonstration of how to scan C++20 dependencies, please refer to this demo.

comius

After filtering out the other 2 PRs this change looks mostly reasonable. I posted some nits on the files unique to this change.

PikachuHyA · 2024-05-27T09:09:52Z

I just keep the main commit in this patch.
due to patch dependencies, it may build failed.

I also fix clang_deps_scanner_wrapper.sh.tpl according to #19940 (comment) and #19940 (comment)

PikachuHyA · 2024-06-14T03:17:38Z

hi @comius , the third patch is ready. please review.

comius

LGTM in general, except an a lot of renames to keep it consistent with PR[1/5].

Please replace cpp20_modules with cpp_modules.
Please replace deps_scanning with cpp_module_deps_scanning - so that there isn't a confusion what the deps are. Or maybe is should be dep_module_scanning?

@comius

This patch adds `compiler_input_flags_feature` and `compiler_output_flags_feature` to the features. follow #22717 By default, the features `compiler_input_flags_feature` and `compiler_output_flags_feature` are included through `CppActionConfigs.java` in the `getFeaturesToAppearLastInFeaturesList` method. For reference, see the relevant code here: https://github.com/bazelbuild/bazel/blob/0dbfaccaf5bee5ea7f11c01db1fc0cd1ca7f3810/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java#L1513-L1573 ## Background I modified `tools/cpp/unix_cc_toolchain_config.bzl` and found no input and output on macOS when testing #19940 with the new action names `c++20-deps-scanning` and `c++20-module-compile`. As discussed in #22429 (comment), I added these two features to `unix_cc_toolchain_config.bzl`. the Windows toolchains already have these features, so no modifications were necessary for `windows_cc_toolchain_config.bzl`. - Windows input flags: https://github.com/bazelbuild/bazel/blob/786a893ef6f69a8f77ca008a478bf67abfdcdc57/tools/cpp/windows_cc_toolchain_config.bzl#L1073-L1095 - Windows output flags: https://github.com/bazelbuild/bazel/blob/786a893ef6f69a8f77ca008a478bf67abfdcdc57/tools/cpp/windows_cc_toolchain_config.bzl#L960-L1020 cc @comius Closes #22743. PiperOrigin-RevId: 643345702 Change-Id: I5715d25e12c7a3616d1fdb484f77ef7cd0fd1bba

PikachuHyA · 2024-06-17T03:07:19Z

LGTM in general, except an a lot of renames to keep it consistent with PR[1/5].

Please replace cpp20_modules with cpp_modules. Please replace deps_scanning with cpp_module_deps_scanning - so that there isn't a confusion what the deps are. Or maybe is should be dep_module_scanning?

Complete the following renaming.

CPP20_MODULES_INFO -> CPP_MODULES_INFO
CPP20_MODULES_DDI -> CPP_MODULES_DDI
CPP20_MODULES_MODMAP -> CPP_MODULES_MODMAP
CPP20_MODULES_MODMAP_INPUT -> CPP_MODULES_MODMAP_INPUT
CPP20_MODULE_OUTPUT_FILE -> CPP_MODULE_OUTPUT_FILE
CPP20_DEPS_SCANNING -> CPP_MODULE_DEPS_SCANNING
CPP20_MODMAP_FILE -> CPP_MODULE_MODMAP_FILE
deps-scanner -> cpp-module-deps-scanner

How should I handle the singular and plural forms? Currently, CPP_MODULES_INFO, CPP_MODULES_DDI, CPP_MODULES_MODMAP, and CPP_MODULES_MODMAP_INPUT use the CPP_MODULES_XXX format, while others use the CPP_MODULE_XXX format. Should we unify them into the CPP_MODULE_XXX format?

Additionally, c++-module-compile and c++-module-codegen already exist. How should we handle c++20-module-compile and c++20-module-codegen?

PikachuHyA · 2024-07-01T07:04:39Z

@comius
ping

PikachuHyA · 2024-07-17T08:15:50Z

@trybka ping

peakschris · 2024-07-21T14:56:02Z

@trybka we are keenly waiting the modules feature, is there anything that can be done to help expedite the review of this and other MRs? cc @comius

trybka · 2024-07-21T17:59:49Z

Apologies. I had some personal stuff take my attention this past week. I will prioritize this review (and the other splits if they are ready) this week.

I appreciate your patience as I am ramping up on this work.

PikachuHyA · 2024-07-23T03:17:48Z

I'm testing #22553 and find a bug in this patch due to #22743 merged

Changes

squash old commits to 1 commit
rebase to the latest master branch
update compiler_input_flags_feature and compiler_output_flags_feature

peakschris · 2024-08-04T12:05:36Z

@comius @trybka can this go in now?

PikachuHyA · 2024-08-22T06:53:36Z

@trybka
ping

trybka

This looks fine to me. I was waiting for the PR for 2/5 to be imported, and I think there are still unresolved comments there (possibly on the internal import), but this split should be fine once that one is ready.

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

PikachuHyA force-pushed the cxx20-modules-support-patch-3 branch from 9582891 to 4f31b5a Compare May 20, 2024 07:58

PikachuHyA requested a review from lberki as a code owner May 20, 2024 07:58

PikachuHyA force-pushed the cxx20-modules-support-patch-3 branch from 4f31b5a to c6a1120 Compare May 20, 2024 08:26

PikachuHyA requested review from a team, ahumesky, oquenchil and ted-xie as code owners May 20, 2024 08:26

PikachuHyA requested review from gregestren and removed request for a team May 20, 2024 08:26

oquenchil removed their request for review May 20, 2024 09:30

comius requested review from comius and removed request for ahumesky, gregestren, lberki and ted-xie May 22, 2024 11:54

comius self-assigned this May 22, 2024

comius reviewed May 22, 2024

View reviewed changes

Comment thread src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java Outdated

Comment thread tools/cpp/clang_deps_scanner_wrapper.sh.tpl Outdated

PikachuHyA mentioned this pull request May 23, 2024

support C++20 Modules #19940

Closed

PikachuHyA changed the title ~~support C++20 Modules, add deps-scanner~~ [WIP] support C++20 Modules, add deps-scanner May 27, 2024

PikachuHyA force-pushed the cxx20-modules-support-patch-3 branch from c6a1120 to 546b0b7 Compare May 27, 2024 09:06

PikachuHyA changed the title ~~[WIP] support C++20 Modules, add deps-scanner~~ [WIP] [3/5] support C++20 Modules, add deps-scanner May 27, 2024

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 force-pushed the cxx20-modules-support-patch-3 branch from fd16e2e to 4b3d7f3 Compare June 13, 2024 09:12

PikachuHyA changed the title ~~[WIP] [3/5] support C++20 Modules, add deps-scanner~~ [3/5] support C++20 Modules, add deps-scanner and update toolchains Jun 14, 2024

PikachuHyA mentioned this pull request Jun 14, 2024

add compiler_input_flags_feature and compiler_output_flags_feature #22743

Closed

comius requested changes Jun 14, 2024

View reviewed changes

PikachuHyA force-pushed the cxx20-modules-support-patch-3 branch from 1871243 to 282423a Compare June 17, 2024 02:53

PikachuHyA force-pushed the cxx20-modules-support-patch-3 branch from 282423a to c0d43c1 Compare June 17, 2024 03:15

comius approved these changes Jul 5, 2024

View reviewed changes

comius requested a review from trybka July 5, 2024 12:25

comius added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Jul 5, 2024

PikachuHyA mentioned this pull request Jul 22, 2024

[2/5] support C++20 Modules, add C++20 modules tools #22427

Closed

PikachuHyA force-pushed the cxx20-modules-support-patch-3 branch from c0d43c1 to 86d1494 Compare July 23, 2024 03:13

PikachuHyA added 2 commits August 21, 2024 15:04

support C++20 Modules, update toolchain config

c385530

update compiler_input_flags_feature and compiler_output_flags_feature

ae05dee

PikachuHyA force-pushed the cxx20-modules-support-patch-3 branch from 86d1494 to ae05dee Compare August 21, 2024 07:08

trybka approved these changes Aug 23, 2024

View reviewed changes

copybara-service Bot closed this in fc057d2 Aug 30, 2024

github-actions Bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Aug 30, 2024

gracicot mentioned this pull request Jan 13, 2025

Proposal for C++20 module support with gmake generator (GCC) premake/premake-core#1735

Closed

Conversation

PikachuHyA commented May 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Action Names

File Extensions

Build Variables

Toolchains

Uh oh!

comius left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PikachuHyA commented May 27, 2024

Uh oh!

PikachuHyA commented Jun 14, 2024

Uh oh!

comius left a comment

Choose a reason for hiding this comment

Uh oh!

PikachuHyA commented Jun 17, 2024

Uh oh!

PikachuHyA commented Jul 1, 2024

Uh oh!

PikachuHyA commented Jul 17, 2024

Uh oh!

peakschris commented Jul 21, 2024

Uh oh!

trybka commented Jul 21, 2024

Uh oh!

PikachuHyA commented Jul 23, 2024

Uh oh!

peakschris commented Aug 4, 2024

Uh oh!

PikachuHyA commented Aug 22, 2024

Uh oh!

trybka left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

PikachuHyA commented May 17, 2024 •

edited

Loading