Skip to content

partial c++20 module support#2516

Merged
sylvestre merged 13 commits intomozilla:mainfrom
TroyKomodo:troy/cxx20-modules
Feb 27, 2026
Merged

partial c++20 module support#2516
sylvestre merged 13 commits intomozilla:mainfrom
TroyKomodo:troy/cxx20-modules

Conversation

@TroyKomodo
Copy link
Contributor

@TroyKomodo TroyKomodo commented Dec 18, 2025

Adds partial support for c++20 modules when using clang.

Also detects when modules are used in MSVC / GCC and disable cache for those calls.

fixes #2216

partially addresses #2095

the clang c++20 module implementation (as well as msvc) is rather easy to parse and understand so adding support for it is fairly trivial. GCC however is a mess. An entirely different approach will be needed to get GCC module support which is far out side of the scope for this initial PR.

I will submit future PRs to get MSVC / GCC.

Test with nix

(pkgs.sccache.overrideAttrs (final: old: {
  version = "troy/cxx20-modules";
  cargoHash = "sha256-tCQrZ6cPGJrxHgeSJ4ZqZs6otWvVu6Ys3svAiwvmSDc=";
  src = fetchFromGitHub {
    owner = "troykomodo";
    repo = "sccache";
    rev = "troy/cxx20-modules";
    sha256 = "sha256-A0n/nsNPTHRC0Z6ERwCpoR1w3O/xl6d1yCw/5GeqNd4=";
  };
  cargoDeps = pkgs.rustPlatform.fetchCargoVendor {
    inherit (final) pname src version;
    hash = final.cargoHash;
  };
}))
@codecov-commenter
Copy link

codecov-commenter commented Dec 18, 2025

Codecov Report

❌ Patch coverage is 87.70764% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.39%. Comparing base (81954de) to head (7cba31c).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/compiler/clang.rs 84.45% 23 Missing ⚠️
src/compiler/gcc.rs 85.54% 12 Missing ⚠️
src/compiler/compiler.rs 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2516      +/-   ##
==========================================
+ Coverage   73.27%   73.39%   +0.12%     
==========================================
  Files          68       68              
  Lines       36907    37219     +312     
==========================================
+ Hits        27042    27316     +274     
- Misses       9865     9903      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@ChuanqiXu9
Copy link

Copy link
Contributor

@mathstuf mathstuf left a comment

Choose a reason for hiding this comment

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

The code looks good to me. I'll probably try to test it some time next week.

I can help out with GCC module map file parsing too (it will need to actually be parsed due to the $root meta-descriptor). However, the output path is also given by it…that may prove more difficult to "know".

@TroyKomodo
Copy link
Contributor Author

The code looks good to me. I'll probably try to test it some time next week.

I can help out with GCC module map file parsing too (it will need to actually be parsed due to the $root meta-descriptor). However, the output path is also given by it…that may prove more difficult to "know".

Thanks! It was my first time contributing to sccache so I'm not too familiar with the process.

I will fix up the clippy issues + windows test issues :p

@mathstuf
Copy link
Contributor

It looks like BMI-only compilations are not properly considered:

467:    actual-stdout> [2-1->6/8@76.9] Building CXX object 'CMakeFiles/CXXModules__export_from_ninja@synth_cf4e03cd8b1e.dir/35a7889ff75f.bmi'          
467:    actual-stdout> FAILED: [code=254] CMakeFiles/CXXModules__export_from_ninja@synth_cf4e03cd8b1e.dir/35a7889ff75f.bmi 
467:    actual-stdout> /home/boeckb/code/depot/group-devel/sccache/src/target/debug/sccache  /usr/lib64/ccache/clang++   -g -std=gnu++20 --precompile -MD -MT 'CMakeFiles/CXXModules__
export_from_ninja@synth_cf4e03cd8b1e.dir/35a7889ff75f.bmi' -MF CMakeFiles/CXXModules__export_from_ninja@synth_cf4e03cd8b1e.dir/35a7889ff75f.bmi.d @CMakeFiles/CXXModules__export_from_
ninja@synth_cf4e03cd8b1e.dir/35a7889ff75f.bmi.modmap -o 'CMakeFiles/CXXModules__export_from_ninja@synth_cf4e03cd8b1e.dir/35a7889ff75f.bmi' -c /home/boeckb/misc/builds/cmake/build-cla
ng/Tests/RunCMake/CXXModulesCompile/exp-mods-from-ninja-ninja-install/lib/cxx/miu/importable.cxx
467:    actual-stdout> sccache: encountered fatal error
467:    actual-stdout> sccache: error: Failed to generate compile commands
467:    actual-stdout> sccache: caused by: Failed to generate compile commands
467:    actual-stdout> sccache: caused by: Missing object file output

This is from running CMake's CXXModules tests:

export SCCACHE_SERVER_PORT=23448 # unique to avoid ambient re-usage
export CXX="$(which sccache) $(which clang++)"
ctest -R CXXModules
@mathstuf
Copy link
Contributor

You can ignore the tests failing about JSON argument list mismatches; it is not robust against mutli-argument compilers like here. If you do the "symlink trick" (ln -s $(which sccache) clang++ && export CXX="$PWD/clang++", that probably avoids the issue).

@TroyKomodo
Copy link
Contributor Author

Hmm, i think this is just me forgetting to change it somewhere. I will fix

@TroyKomodo
Copy link
Contributor Author

Okay so it seems i think we should always treat it as an object even if it really isnt...

@TroyKomodo

This comment was marked as outdated.

@sylvestre
Copy link
Collaborator

could you please add an integration test ? ie high level which integrates into cmake and uses C++ module.
see
tests/cmake/ tests/sccache_args.rs tests/sccache_cargo.rs tests/sccache_rustc.rs

@mathstuf
Copy link
Contributor

I tried the cmake tests and they ran fine for me but there were only 2...

There are dozens of test cases within each CTest-level test. These are the relevant ones, thanks!

Can you inspect the cache state (--show-stats) on an initial run and a subsequent run to see how well caching itself is working?

@TroyKomodo
Copy link
Contributor Author

could you please add an integration test ? ie high level which integrates into cmake and uses C++ module.

see

tests/cmake/ tests/sccache_args.rs tests/sccache_cargo.rs tests/sccache_rustc.rs

Yes!

@TroyKomodo

This comment was marked as outdated.

@TroyKomodo
Copy link
Contributor Author

@sylvestre
Copy link
Collaborator

are you sure ?

Run ${SCCACHE_PATH} --show-stats
Compile requests                      4
Compile requests executed             4
Cache hits                            2
Cache hits (C/C++)                    2
Cache misses                          2
Cache misses (C/C++)                  2
Cache hits rate                   50.00 %
Cache hits rate (C/C++)           50.00 %

@TroyKomodo
Copy link
Contributor Author

That seems right no? the first time we invoke the compiler not cached yet then we invoke again and its cached.

There are only 2 files to compile, so i expected it to be 4 requests 2 hit 2 misses.

@sylvestre
Copy link
Collaborator

oh yeah, my bad, i thought we were doing a reset

@TroyKomodo
Copy link
Contributor Author

oh yeah, my bad, i thought we were doing a reset

I just copied the other cmake test example, but I can reset the stats if that makes it easier to follow

@mathstuf
Copy link
Contributor

Gah, module compilation in the tests doesn't really happen without a setting. For clang, this should work:

-DCMake_TEST_MODULE_COMPILATION=named,compile_commands,collation,partitions,internal_partitions,export_bmi,install_bmi,shared,bmionly,build_database,import_std23

}

if !module_only_flag {
if let Some(module_output_path) = module_output_path {
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you please move this into a function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can, however this function is ~550 lines, I think it would be better if in a followup PR we break up this function rather than doing a half job here by breaking up just this one conditional.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i still want this to be a new function, sorry

@TroyKomodo
Copy link
Contributor Author

Gah, module compilation in the tests doesn't really happen without a setting. For clang, this should work:

-DCMake_TEST_MODULE_COMPILATION=named,compile_commands,collation,partitions,internal_partitions,export_bmi,install_bmi,shared,bmionly,build_database,import_std23

Ah, I see. Okay I will try again i think i need to setup my env to have support for import std.

@TroyKomodo
Copy link
Contributor Author

Gah, module compilation in the tests doesn't really happen without a setting. For clang, this should work:
-DCMake_TEST_MODULE_COMPILATION=named,compile_commands,collation,partitions,internal_partitions,export_bmi,install_bmi,shared,bmionly,build_database,import_std23

Ah, I see. Okay I will try again i think i need to setup my env to have support for import std.

let
  pkgs = import <nixpkgs> {
    overlays = [
      (final: prev: {
        llvmPackages_21 = prev.llvmPackages_21 // {
          libcxx = prev.llvmPackages_21.libcxx.overrideAttrs (old: {
            postInstall = (old.postInstall or "") + ''
              substituteInPlace $out/lib/libc++.modules.json \
                --replace-fail '"../share' "\"$out/share"
            '';
          });
        };
      })
    ];
  };

  llvm = pkgs.llvmPackages_21;

  wrappedClangScanDeps = pkgs.writeShellScriptBin "clang-scan-deps" ''
    newargs=()
    inject_next=false
    for arg in "$@"; do
      if $inject_next; then
        newargs+=("$arg")
        newargs+=("-isystem" "${llvm.libcxx.dev}/include/c++/v1")
        inject_next=false
      elif [[ "$arg" == "--" ]]; then
        newargs+=("$arg")
        inject_next=true
      else
        newargs+=("$arg")
      fi
    done
    exec ${llvm.clang-tools}/bin/clang-scan-deps "''${newargs[@]}"
  '';

  sccache = "/home/troy/github/troy/sccache/result/bin/sccache";

  clangWrapper = pkgs.writeShellScriptBin "clang" ''
    exec ${sccache} "${llvm.libcxxClang}/bin/clang" "''${@}"
  '';
  clangxxWrapper = pkgs.writeShellScriptBin "clang++" ''
    exec ${sccache} "${llvm.libcxxClang}/bin/clang++" "''${@}"
  '';
in
pkgs.mkShell {
  hardeningDisable = [ "all" ];

  buildInputs = [
    pkgs.cmake
    pkgs.mold
    pkgs.ninja
    pkgs.openssl
    llvm.libcxxClang
    llvm.clang-tools
    wrappedClangScanDeps
  ];

  shellHook = ''
    export CC="${clangWrapper}/bin/clang"
    export CXX="${clangxxWrapper}/bin/clang++"
    export LD_LIBRARY_PATH="${llvm.libcxx.out}/lib:${pkgs.openssl.out}/lib:''${LD_LIBRARY_PATH:-}"
    export NIX_CFLAGS_COMPILE="-B${llvm.libcxx.out}/lib -isystem ${llvm.libcxx.dev}/include/c++/v1"
    export PATH="${wrappedClangScanDeps}/bin:''${PATH:-}"
  '';
}

A huge pain...

troy@troy-thinkpad ~/g/t/CMake (master) [SIGINT]> ctest -R CXXModules -j$(nproc) --test-dir=build   nix-shell 
Test project /home/troy/github/troy/CMake/build
Guessing configuration NoConfig
    Start 536: RunCMake.CXXModulesCompile
    Start 535: RunCMake.CXXModules
1/2 Test #535: RunCMake.CXXModules ..............   Passed   18.90 sec
2/2 Test #536: RunCMake.CXXModulesCompile .......   Passed  116.80 sec

100% tests passed, 0 tests failed out of 2

Label Time Summary:
CXXModules    = 135.70 sec*proc (2 tests)

Total Test time (real) = 116.81 sec
troy@troy-thinkpad ~/g/t/CMake (master)> ctest -R CXXModules -j$(nproc) --test-dir=build            nix-shell 
Test project /home/troy/github/troy/CMake/build
Guessing configuration NoConfig
    Start 536: RunCMake.CXXModulesCompile
    Start 535: RunCMake.CXXModules
1/2 Test #535: RunCMake.CXXModules ..............   Passed   19.94 sec
2/2 Test #536: RunCMake.CXXModulesCompile .......   Passed  104.62 sec

100% tests passed, 0 tests failed out of 2

Label Time Summary:
CXXModules    = 124.55 sec*proc (2 tests)

Total Test time (real) = 104.63 sec
troy@troy-thinkpad ~/g/t/CMake (master)>                                                            nix-shell 
Every 2.0s: /home/troy/github/troy/sccache/result/bin/sccache -s       troy-thinkpad: Mon Dec 22 12:47:11 2025

Compile requests                   1976
Compile requests executed           830
Cache hits                          756
Cache hits (C/C++)                  756
Cache misses                         72
Cache misses (C/C++)                 72
Cache hits rate                   91.30 %
Cache hits rate (C/C++)           91.30 %
Cache timeouts                        0
Cache read errors                     0
Forced recaches                       0
Cache write errors                    0
Cache errors                          0
Compilations                         72
Compilation failures                  2
Non-cacheable compilations            0
Non-cacheable calls                 244
Non-compilation calls               902
Unsupported compiler calls            0
Average cache write               0.001 s
Average compiler                  0.217 s
Average cache read hit            0.000 s
Failed distributed compilations       0

Non-cacheable reasons:
-E                                  244

Cache location                  Local disk: "/home/troy/github/troy/CMake/build/Tests/CMakeFiles/TestHome/.cac
he/sccache"
Use direct/preprocessor mode?   yes
Version (client)                0.12.0
Cache size                          155 MiB
Max cache size                       10 GiB



@mathstuf
Copy link
Contributor

There's no rush on my account at least; take your time.

@AJIOB
Copy link
Contributor

AJIOB commented Feb 4, 2026

Hi @TroyKomodo,

Do you have any updates?

Looks like we have conflicts.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 15, 2026

Merging this PR will degrade performance by 3.49%

❌ 1 regressed benchmark
✅ 61 untouched benchmarks
⏩ 2 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation normalize_win_path_typical 1.6 µs 1.7 µs -3.49%

Comparing TroyKomodo:troy/cxx20-modules (7cba31c) with main (81a6c0a)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@TroyKomodo
Copy link
Contributor Author

Okay! So i finally have gotten the time to look through this PR again. I think, this automatically works with dist mode since we add the PCMs to the extra_hash_files which are sent to the dist server automatically?

Could someone please provide me with a way to test this or someone test this for me, using the integration test in the PR.

Integration tests fail because this branch is from a fork.

@sylvestre
Copy link
Collaborator

Integration tests fail because this branch is from a fork.

why do you think that ?

Copy link
Contributor

@mathstuf mathstuf left a comment

Choose a reason for hiding this comment

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

Seems fine as a stepping stone towards actual support. Just one minor request.

@TroyKomodo
Copy link
Contributor Author

Integration tests fail because this branch is from a fork.

why do you think that ?

Initially it spoke about missing an auth token when uploading. I think that was fixed in some other PR, after a rebase i think you (or me) resolved a merge incorrectly and it included additional ci jobs which were removed/changed which is why CI was failing before my last commit.

@sylvestre sylvestre merged commit 3ad5944 into mozilla:main Feb 27, 2026
49 of 50 checks passed
@TroyKomodo TroyKomodo deleted the troy/cxx20-modules branch February 27, 2026 13:29
@avikivity
Copy link
Contributor

Thanks a lot for implementing this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

7 participants