Skip to content

[C++20] [Modules] Offer -fmodules-embed-all-files option #107194

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

Merged
merged 4 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions clang/docs/StandardCPlusPlusModules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,37 @@ Currently, Clang accepts the above example, though it may produce surprising
results if the debugging code depends on consistent use of ``NDEBUG`` in other
translation units.

Source Files Consistency
^^^^^^^^^^^^^^^^^^^^^^^^

Clang may open the input files\ :sup:`1`` of a BMI during the compilation. This implies that
when Clang consumes a BMI, all the input files need to be present in the original path
and with the original contents.

To overcome these requirements and simplify cases like distributed builds and sandboxed
builds, users can use the ``-fmodules-embed-all-files`` flag to embed all input files
into the BMI so that Clang does not need to open the corresponding file on disk.

When the ``-fmodules-embed-all-files`` flag are enabled, Clang explicitly emits the source
code into the BMI file, the contents of the BMI file contain a sufficiently verbose
representation to reproduce the original source file.

:sup:`1`` Input files: The source files which took part in the compilation of the BMI.
For example:

.. code-block:: c++

// M.cppm
module;
#include "foo.h"
export module M;

// foo.h
#pragma once
#include "bar.h"

The ``M.cppm``, ``foo.h`` and ``bar.h`` are input files for the BMI of ``M.cppm``.

Object definition consistency
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand All @@ -484,6 +515,13 @@ fragment is disabled by default. These checks can be enabled by specifying
and you encounter incorrect or missing diagnostics, please report them via the
`community issue tracker <https://github.com/llvm/llvm-project/issues/>`_.

Privacy Issue
-------------

BMIs are not and should not be treated as an information hiding mechanism.
They should always be assumed to contain all the information that was used to
create them, in a recoverable form.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I might expand on this a bit with something like, "While the -fmodules_embed_all_files flag explicitly emits the source code into the BMI file, the contents of the BMI file contain a sufficiently verbose(word choice?) representation to reproduce the original source file."

I'd like to make this warning as clear as possible: If you ship a BMI, you mind as well just ship the source code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done by adding the suggested text to the above section about -fmodules-embed-all-files. I feel it is odd to mention -fmodules-embed-all-files in the current section since it is mostly about clarifying that hiding information is not a goal for BMIs. It is true with and without -fmodules-embed-all-files.


ABI Impacts
-----------

Expand Down
10 changes: 6 additions & 4 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -3162,6 +3162,12 @@ def modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">,
HelpText<"Generate the reduced BMI">,
MarshallingInfoFlag<FrontendOpts<"GenReducedBMI">>;

def fmodules_embed_all_files : Joined<["-"], "fmodules-embed-all-files">,
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the position since the original position is enclosed by a group assigning the CC1Option.

Visibility<[ClangOption, CC1Option, CLOption]>,
HelpText<"Embed the contents of all files read by this compilation into "
"the produced module file.">,
MarshallingInfoFlag<FrontendOpts<"ModulesEmbedAllFiles">>;

def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, Group<i_Group>,
Visibility<[ClangOption, CC1Option]>, MetaVarName<"<seconds>">,
HelpText<"Specify the interval (in seconds) between attempts to prune the module cache">,
Expand Down Expand Up @@ -7665,10 +7671,6 @@ def fmodules_embed_file_EQ : Joined<["-"], "fmodules-embed-file=">,
HelpText<"Embed the contents of the specified file into the module file "
"being compiled.">,
MarshallingInfoStringVector<FrontendOpts<"ModulesEmbedFiles">>;
def fmodules_embed_all_files : Joined<["-"], "fmodules-embed-all-files">,
HelpText<"Embed the contents of all files read by this compilation into "
"the produced module file.">,
MarshallingInfoFlag<FrontendOpts<"ModulesEmbedAllFiles">>;
defm fimplicit_modules_use_lock : BoolOption<"f", "implicit-modules-use-lock",
FrontendOpts<"BuildingImplicitModuleUsesLock">, DefaultTrue,
NegFlag<SetFalse>,
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4214,6 +4214,9 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
Args.ClaimAllArgs(options::OPT_fmodule_output_EQ);
}

if (Args.hasArg(options::OPT_fmodules_embed_all_files))
CmdArgs.push_back("-fmodules-embed-all-files");

return HaveModules;
}

Expand Down
5 changes: 5 additions & 0 deletions clang/test/Driver/fmodules-embed-all-files.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// RUN: %clang -std=c++20 %s -fmodules-embed-all-files -### 2>&1 | FileCheck %s
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd appreciate a second RUN line that passes no option and checks that the cc1 option is not passed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

// CHECK: -fmodules-embed-all-files

// RUN: %clang -std=c++20 %s -### 2>&1 | FileCheck %s --check-prefix=NON-EMBED
// NON-EMBED-NOT: -fmodules-embed-all-files
Loading