[clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL #98761

sharadhr · 2024-07-13T18:36:51Z

This PR is the first step in improving the situation for clang-cl detailed in this LLVM Discourse thread. There has been some work done in #89772. I believe this is somewhat orthogonal.

This is a work-in-progress; the functionality has only been tested with the basic 'Hello World' example, and proper test cases need to be written. I'd like some thoughts on this, thanks!

Partially resolves #64118.

github-actions · 2024-07-13T18:37:10Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-07-13T18:37:40Z

@llvm/pr-subscribers-clang-modules
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Sharadh Rajaraman (sharadhr)

Changes

This PR is the first step in improving the situation for clang-cl detailed in this LLVM Discourse thread. There has been some work done in #89772. I believe this is somewhat orthogonal.

This is a work-in-progress; the functionality has only been tested with the basic 'Hello World' example, and proper test cases need to be written. Additionally, the /std:c++20 command-line argument is considered unused (although it is very much required, and compilation fails without it) with the following warning:

clang: warning: argument unused during compilation: '/std:c++20' [-Wunused-command-line-argument]

I suspect the issue is somewhere around

llvm-project/clang/lib/Driver/ToolChains/Clang.cpp

Lines 6285 to 6300 in 8802c9f

    
           // FIXME: Clang doesn't correctly handle -std= when the input language 
        
           // doesn't match. For the time being just ignore this for C++ inputs; 
        
           // eventually we want to do all the standard defaulting here instead of 
        
           // splitting it between the driver and clang -cc1. 
        
           if (!types::isCXX(InputType)) { 
        
             if (!Args.hasArg(options::OPT__SLASH_std)) { 
        
               Args.AddAllArgsTranslated(CmdArgs, options::OPT_std_default_EQ, "-std=", 
        
                                         /*Joined=*/true); 
        
             } else 
        
               ImplyVCPPCVer = true; 
        
           } 
        
           else if (IsWindowsMSVC) 
        
             ImplyVCPPCXXVer = true; 
        
           Args.AddLastArg(CmdArgs, options::OPT_ftrigraphs, 
        
                           options::OPT_fno_trigraphs);

I'd like some thoughts on this, thanks!

Full diff: https://github.com/llvm/llvm-project/pull/98761.diff

1 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+2-1)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 4ab8638175dd3..ca7cfef8453a0 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3085,7 +3085,7 @@ def fmodules_user_build_path : Separate<["-"], "fmodules-user-build-path">, Grou
   HelpText<"Specify the module user build path">,
   MarshallingInfoString<HeaderSearchOpts<"ModuleUserBuildPath">>;
 def fprebuilt_module_path : Joined<["-"], "fprebuilt-module-path=">, Group<i_Group>,
-  Flags<[]>, Visibility<[ClangOption, CC1Option]>,
+  Flags<[]>, Visibility<[ClangOption, CLOption, CC1Option]>,
   MetaVarName<"<directory>">,
   HelpText<"Specify the prebuilt module path">;
 defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules",
@@ -5874,6 +5874,7 @@ def _output : Separate<["--"], "output">, Alias<o>;
 def _param : Separate<["--"], "param">, Group<CompileOnly_Group>;
 def _param_EQ : Joined<["--"], "param=">, Alias<_param>;
 def _precompile : Flag<["--"], "precompile">, Flags<[NoXarchOption]>,
+  Visibility<[ClangOption, CLOption]>,
   Group<Action_Group>, HelpText<"Only precompile the input">;
 def _prefix_EQ : Joined<["--"], "prefix=">, Alias<B>;
 def _prefix : Separate<["--"], "prefix">, Alias<B>;

ChuanqiXu9

Thanks. Can you add a test for this under (clang/test/Driver) and add new subsection for it in StandardCPlusPlusModules.rst ?

github-actions · 2024-07-16T02:49:47Z

✅ With the latest revision this PR passed the C/C++ code formatter.

clang/lib/Driver/Types.cpp

clang/test/Driver/cl-cxx20-modules.cppm

clang/lib/Driver/Types.cpp

clang/docs/StandardCPlusPlusModules.rst

sharadhr · 2024-08-05T10:34:50Z

@ChuanqiXu9, do make a re-review when you can, thanks!

ChuanqiXu9

LGTM. I'll merge this after the CI finished.

ChuanqiXu9 · 2024-08-06T02:03:31Z

It looks like the CI failure is not related.

ChuanqiXu9 · 2024-08-06T02:04:51Z

@sharadhr it looks like the commit author is not correct. By default it is 3754080+sharadhr@users.noreply.github.com, but we don't like such fake mail account. Please update it with your real mail.

…ly into CL driver without `/clang:` prefix

- Also add a note for build system authors to use `clang++` options

sharadhr · 2024-08-06T10:03:40Z

I've done a git rebase with --reset-author. Hopefully this is correct.

github-actions · 2024-08-06T15:06:14Z

@sharadhr Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

ChuanqiXu9 · 2024-08-06T15:11:20Z

I think this worth to be in 19.x and I don't it is a risk.

ChuanqiXu9 · 2024-08-06T15:15:01Z

/cherry-pick bd576fe

…ions in Clang-CL (llvm#98761) This PR is the first step in improving the situation for `clang-cl` detailed in [this LLVM Discourse thread](https://discourse.llvm.org/t/clang-cl-exe-support-for-c-modules/72257/28). There has been some work done in llvm#89772. I believe this is somewhat orthogonal. This is a work-in-progress; the functionality has only been tested with the [basic 'Hello World' example](https://clang.llvm.org/docs/StandardCPlusPlusModules.html#quick-start), and proper test cases need to be written. I'd like some thoughts on this, thanks! Partially resolves llvm#64118. (cherry picked from commit bd576fe)

llvmbot · 2024-08-06T15:20:07Z

/pull-request #102159

…` for precompiled module inputs to `clang-cl` (#99300) Relates to #98761: `-fmodule-file` requires `/std:c++20` or greater, but passing that option results in an unused argument warning. This resolves that.

…ions in Clang-CL (llvm#98761) This PR is the first step in improving the situation for `clang-cl` detailed in [this LLVM Discourse thread](https://discourse.llvm.org/t/clang-cl-exe-support-for-c-modules/72257/28). There has been some work done in llvm#89772. I believe this is somewhat orthogonal. This is a work-in-progress; the functionality has only been tested with the [basic 'Hello World' example](https://clang.llvm.org/docs/StandardCPlusPlusModules.html#quick-start), and proper test cases need to be written. I'd like some thoughts on this, thanks! Partially resolves llvm#64118. (cherry picked from commit bd576fe)

1. __cpp_lib_modules now is defined. llvm/llvm-project#90091 2. clang-cl supports -x option for all languages. llvm/llvm-project#89772 3. Header units allow non-inline external static members definitions. llvm/llvm-project#98309 4. winsysroot alias to the GNU driver: -Xmicrosoft-windows-sys-root. llvm/llvm-project#94731 5. clang-cl gained options for compiling C++ modules. llvm/llvm-project#98761 6. fix unused argument warning for /std:c++20 when compiling c++ modules. llvm/llvm-project#99300

llvmbot added the clang Clang issues not falling into any other category label Jul 13, 2024

llvmbot added the clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' label Jul 14, 2024

sharadhr changed the title ~~[clang][driver] Improve Clang-CL support for C++20 standard modules~~ [clang][driver] Support --precompile and -fmodule-* options in Clang-CL Jul 14, 2024

ChuanqiXu9 reviewed Jul 16, 2024

View reviewed changes

ChuanqiXu9 added the clang:modules C++20 modules and Clang Header Modules label Jul 16, 2024

ChuanqiXu9 reviewed Jul 16, 2024

View reviewed changes

clang/lib/Driver/Types.cpp Outdated Show resolved Hide resolved

sharadhr commented Jul 16, 2024

View reviewed changes

clang/test/Driver/cl-cxx20-modules.cppm Outdated Show resolved Hide resolved

ChuanqiXu9 reviewed Jul 17, 2024

View reviewed changes

clang/test/Driver/cl-cxx20-modules.cppm Outdated Show resolved Hide resolved

clang/lib/Driver/Types.cpp Outdated Show resolved Hide resolved

clang/docs/StandardCPlusPlusModules.rst Outdated Show resolved Hide resolved

clang/docs/StandardCPlusPlusModules.rst Outdated Show resolved Hide resolved

sharadhr force-pushed the clang-cl-modules branch from 2d4067f to e6386de Compare July 17, 2024 10:07

sharadhr mentioned this pull request Jul 17, 2024

[clang][driver][clang-cl] Fix unused argument warning for /std:c++20 for precompiled module inputs to clang-cl #99300

Merged

sharadhr changed the title ~~[clang][driver] Support --precompile and -fmodule-* options in Clang-CL~~ [clang][driver][clang-cl] Support --precompile and -fmodule-* options in Clang-CL Jul 17, 2024

ChuanqiXu9 approved these changes Aug 6, 2024

View reviewed changes

sharadhr added 7 commits August 6, 2024 11:00

Allow --precompile and -fprebuilt-module-path to be passed direct…

2f499e7

…ly into CL driver without `/clang:` prefix

Support more fmodule-* options from CL driver

9bf377f

clang-cl C++20 modules compile test

d0491d3

Document the new options

8404e4f

Remove commented-out docs

09d7caa

- Also add a note for build system authors to use `clang++` options

Attempt to rewrite tests for -fmodule* args to clang-cl

eab339c

Use --check-prefix to split tests

4e8ca2a

sharadhr force-pushed the clang-cl-modules branch from ef6a97d to 4e8ca2a Compare August 6, 2024 10:02

ChuanqiXu9 merged commit bd576fe into llvm:main Aug 6, 2024
4 of 6 checks passed

ChuanqiXu9 added this to the LLVM 19.X Release milestone Aug 6, 2024

mgabka mentioned this pull request Aug 22, 2024

Add release note about ABI mgabka/llvm-project#6

Open

thewtex mentioned this pull request Feb 10, 2025

llvmorg 19.1.5 libcxxabi pthread lib name #126605

Closed

kassane mentioned this pull request Apr 28, 2025

zig c++: Doesn't support c++ modules ziglang/zig#15496

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL #98761

[clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL #98761

sharadhr commented Jul 13, 2024 •

edited

Loading

github-actions bot commented Jul 13, 2024

llvmbot commented Jul 13, 2024 •

edited

Loading

ChuanqiXu9 left a comment

github-actions bot commented Jul 16, 2024 •

edited

Loading

sharadhr commented Aug 5, 2024

ChuanqiXu9 left a comment

ChuanqiXu9 commented Aug 6, 2024

ChuanqiXu9 commented Aug 6, 2024

sharadhr commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

ChuanqiXu9 commented Aug 6, 2024

ChuanqiXu9 commented Aug 6, 2024

llvmbot commented Aug 6, 2024

[clang][driver][clang-cl] Support --precompile and -fmodule-* options in Clang-CL #98761

[clang][driver][clang-cl] Support --precompile and -fmodule-* options in Clang-CL #98761

Conversation

sharadhr commented Jul 13, 2024 • edited Loading

github-actions bot commented Jul 13, 2024

llvmbot commented Jul 13, 2024 • edited Loading

ChuanqiXu9 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 16, 2024 • edited Loading

sharadhr commented Aug 5, 2024

ChuanqiXu9 left a comment

Choose a reason for hiding this comment

ChuanqiXu9 commented Aug 6, 2024

ChuanqiXu9 commented Aug 6, 2024

sharadhr commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

ChuanqiXu9 commented Aug 6, 2024

ChuanqiXu9 commented Aug 6, 2024

llvmbot commented Aug 6, 2024

[clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL #98761

[clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL #98761

sharadhr commented Jul 13, 2024 •

edited

Loading

llvmbot commented Jul 13, 2024 •

edited

Loading

github-actions bot commented Jul 16, 2024 •

edited

Loading