clang: Fixes to module support
1. -fmodules-embed-all-files is a cc1 flag, and should have been passed
to clang++ via -Xclang.
2. Don't require a .cc in the target to emit the module_deps variables
(this is to be able to compile a target that contains only a
.modulemap, e.g. the C++ std module).
3. Add {{module_deps_no_self}}, which is the same as {{module_deps}}
except that it excludes the flags to include the pcm for the current
target.
That is, for A depending on B, and both using modules, when building
for A, {{module_deps}} will reference both A.pcm and B.pcm, and so
can be used to build .cc files in A. By contrast,
{{module_deps_no_self}} for A will only reference B.pcm, and so can
be used to build A.pcm.
Bug: fuchsia:27276
Change-Id: I239d1c5cd4d5d167251801b1347b02e8d89caeee
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/9680
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
diff --git a/docs/reference.md b/docs/reference.md
index 257f474..f8c2bd4 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -3718,6 +3718,13 @@
prefixed by "-I" (these work with Posix tools as well as Microsoft
ones).
+ {{module_deps}}
+ {{module_deps_no_self}}
+ Strings that correspond to the flags necessary to depend upon the Clang
+ modules referenced by the current target. The "_no_self" version doesn't
+ include the module for the current target, and can be used to compile
+ the pcm itself.
+
{{source}}
The relative path and name of the current input file.
Example: "../../base/my_file.cc"
diff --git a/src/gn/c_substitution_type.cc b/src/gn/c_substitution_type.cc
index 618e846..ea47ce9 100644
--- a/src/gn/c_substitution_type.cc
+++ b/src/gn/c_substitution_type.cc
@@ -10,22 +10,33 @@
#include "gn/err.h"
const SubstitutionTypes CSubstitutions = {
- &CSubstitutionAsmFlags, &CSubstitutionCFlags,
- &CSubstitutionCFlagsC, &CSubstitutionCFlagsCc,
- &CSubstitutionCFlagsObjC, &CSubstitutionCFlagsObjCc,
- &CSubstitutionDefines, &CSubstitutionFrameworkDirs,
- &CSubstitutionIncludeDirs, &CSubstitutionModuleDeps,
+ &CSubstitutionAsmFlags,
+ &CSubstitutionCFlags,
+ &CSubstitutionCFlagsC,
+ &CSubstitutionCFlagsCc,
+ &CSubstitutionCFlagsObjC,
+ &CSubstitutionCFlagsObjCc,
+ &CSubstitutionDefines,
+ &CSubstitutionFrameworkDirs,
+ &CSubstitutionIncludeDirs,
+ &CSubstitutionModuleDeps,
+ &CSubstitutionModuleDepsNoSelf,
&CSubstitutionSwiftModules,
- &CSubstitutionLinkerInputs, &CSubstitutionLinkerInputsNewline,
- &CSubstitutionLdFlags, &CSubstitutionLibs,
- &CSubstitutionSoLibs, &CSubstitutionFrameworks,
+ &CSubstitutionLinkerInputs,
+ &CSubstitutionLinkerInputsNewline,
+ &CSubstitutionLdFlags,
+ &CSubstitutionLibs,
+ &CSubstitutionSoLibs,
+ &CSubstitutionFrameworks,
&CSubstitutionRlibs,
&CSubstitutionArFlags,
- &CSubstitutionSwiftModuleName, &CSubstitutionSwiftBridgeHeader,
- &CSubstitutionSwiftModuleDirs, &CSubstitutionSwiftFlags,
+ &CSubstitutionSwiftModuleName,
+ &CSubstitutionSwiftBridgeHeader,
+ &CSubstitutionSwiftModuleDirs,
+ &CSubstitutionSwiftFlags,
};
// Valid for compiler tools.
@@ -42,6 +53,8 @@
const Substitution CSubstitutionIncludeDirs = {"{{include_dirs}}",
"include_dirs"};
const Substitution CSubstitutionModuleDeps = {"{{module_deps}}", "module_deps"};
+const Substitution CSubstitutionModuleDepsNoSelf = {"{{module_deps_no_self}}",
+ "module_deps_no_self"};
// Valid for linker tools.
const Substitution CSubstitutionLinkerInputs = {"{{inputs}}", "in"};
@@ -74,7 +87,9 @@
type == &CSubstitutionCFlagsCc || type == &CSubstitutionCFlagsObjC ||
type == &CSubstitutionCFlagsObjCc || type == &CSubstitutionDefines ||
type == &CSubstitutionFrameworkDirs ||
- type == &CSubstitutionIncludeDirs || type == &CSubstitutionModuleDeps;
+ type == &CSubstitutionIncludeDirs ||
+ type == &CSubstitutionModuleDeps ||
+ type == &CSubstitutionModuleDepsNoSelf;
}
bool IsValidCompilerOutputsSubstitution(const Substitution* type) {
diff --git a/src/gn/c_substitution_type.h b/src/gn/c_substitution_type.h
index e4255fd..1f4e90e 100644
--- a/src/gn/c_substitution_type.h
+++ b/src/gn/c_substitution_type.h
@@ -24,6 +24,7 @@
extern const Substitution CSubstitutionFrameworkDirs;
extern const Substitution CSubstitutionIncludeDirs;
extern const Substitution CSubstitutionModuleDeps;
+extern const Substitution CSubstitutionModuleDepsNoSelf;
// Valid for linker tools.
extern const Substitution CSubstitutionLinkerInputs;
diff --git a/src/gn/function_toolchain.cc b/src/gn/function_toolchain.cc
index 52ac8f7..267198e 100644
--- a/src/gn/function_toolchain.cc
+++ b/src/gn/function_toolchain.cc
@@ -647,6 +647,13 @@
prefixed by "-I" (these work with Posix tools as well as Microsoft
ones).
+ {{module_deps}}
+ {{module_deps_no_self}}
+ Strings that correspond to the flags necessary to depend upon the Clang
+ modules referenced by the current target. The "_no_self" version doesn't
+ include the module for the current target, and can be used to compile
+ the pcm itself.
+
{{source}}
The relative path and name of the current input file.
Example: "../../base/my_file.cc"
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index 5f4a14b..17f9c08 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -31,8 +31,12 @@
struct ModuleDep {
ModuleDep(const SourceFile* modulemap,
const std::string& module_name,
- const OutputFile& pcm)
- : modulemap(modulemap), module_name(module_name), pcm(pcm) {}
+ const OutputFile& pcm,
+ bool is_self)
+ : modulemap(modulemap),
+ module_name(module_name),
+ pcm(pcm),
+ is_self(is_self) {}
// The input module.modulemap source file.
const SourceFile* modulemap;
@@ -42,6 +46,9 @@
// The compiled version of the module.
OutputFile pcm;
+
+ // Is this the module for the current target.
+ bool is_self;
};
namespace {
@@ -80,7 +87,7 @@
std::vector<ModuleDep> GetModuleDepsInformation(const Target* target) {
std::vector<ModuleDep> ret;
- auto add = [&ret](const Target* t) {
+ auto add = [&ret](const Target* t, bool is_self) {
const SourceFile* modulemap = GetModuleMapFromTargetSources(t);
CHECK(modulemap);
@@ -94,17 +101,17 @@
t->GetOutputFilesForSource(*modulemap, &tool_type, &modulemap_outputs));
// Must be only one .pcm from .modulemap.
CHECK(modulemap_outputs.size() == 1u);
- ret.emplace_back(modulemap, label, modulemap_outputs[0]);
+ ret.emplace_back(modulemap, label, modulemap_outputs[0], is_self);
};
if (target->source_types_used().Get(SourceFile::SOURCE_MODULEMAP)) {
- add(target);
+ add(target, true);
}
for (const auto& pair: target->GetDeps(Target::DEPS_LINKED)) {
// Having a .modulemap source means that the dependency is modularized.
if (pair.ptr->source_types_used().Get(SourceFile::SOURCE_MODULEMAP)) {
- add(pair.ptr);
+ add(pair.ptr, false);
}
}
@@ -256,25 +263,13 @@
}
if (!module_dep_info.empty()) {
- // TODO(scottmg): Currently clang modules only supported for C++.
- if (target_->source_types_used().Get(SourceFile::SOURCE_CPP)) {
- if (target_->toolchain()->substitution_bits().used.count(
- &CSubstitutionModuleDeps)) {
- EscapeOptions options;
- options.mode = ESCAPE_NINJA_COMMAND;
-
- out_ << CSubstitutionModuleDeps.ninja_name << " = ";
- EscapeStringToStream(out_, "-fmodules-embed-all-files", options);
-
- for (const auto& module_dep : module_dep_info) {
- out_ << " ";
- EscapeStringToStream(
- out_, "-fmodule-file=" + module_dep.module_name + "=", options);
- path_output_.WriteFile(out_, module_dep.pcm);
- }
-
- out_ << std::endl;
- }
+ // TODO(scottmg): Currently clang modules only working for C++.
+ if (target_->source_types_used().Get(SourceFile::SOURCE_CPP) ||
+ target_->source_types_used().Get(SourceFile::SOURCE_MODULEMAP)) {
+ WriteModuleDepsSubstitution(&CSubstitutionModuleDeps, module_dep_info,
+ true);
+ WriteModuleDepsSubstitution(&CSubstitutionModuleDepsNoSelf,
+ module_dep_info, false);
}
}
@@ -290,7 +285,8 @@
if (target_->source_types_used().Get(SourceFile::SOURCE_C) ||
target_->source_types_used().Get(SourceFile::SOURCE_CPP) ||
target_->source_types_used().Get(SourceFile::SOURCE_M) ||
- target_->source_types_used().Get(SourceFile::SOURCE_MM)) {
+ target_->source_types_used().Get(SourceFile::SOURCE_MM) ||
+ target_->source_types_used().Get(SourceFile::SOURCE_MODULEMAP)) {
WriteOneFlag(target_, &CSubstitutionCFlags, false, Tool::kToolNone,
&ConfigValues::cflags, opts, path_output_, out_);
}
@@ -299,7 +295,8 @@
CTool::kCToolCc, &ConfigValues::cflags_c, opts, path_output_,
out_);
}
- if (target_->source_types_used().Get(SourceFile::SOURCE_CPP)) {
+ if (target_->source_types_used().Get(SourceFile::SOURCE_CPP) ||
+ target_->source_types_used().Get(SourceFile::SOURCE_MODULEMAP)) {
WriteOneFlag(target_, &CSubstitutionCFlagsCc, has_precompiled_headers,
CTool::kCToolCxx, &ConfigValues::cflags_cc, opts, path_output_,
out_);
@@ -356,6 +353,30 @@
WriteSharedVars(subst);
}
+void NinjaCBinaryTargetWriter::WriteModuleDepsSubstitution(
+ const Substitution* substitution,
+ const std::vector<ModuleDep>& module_dep_info,
+ bool include_self) {
+ if (target_->toolchain()->substitution_bits().used.count(
+ substitution)) {
+ EscapeOptions options;
+ options.mode = ESCAPE_NINJA_COMMAND;
+
+ out_ << substitution->ninja_name << " = -Xclang ";
+ EscapeStringToStream(out_, "-fmodules-embed-all-files", options);
+
+ for (const auto& module_dep : module_dep_info) {
+ if (!module_dep.is_self || include_self) {
+ out_ << " ";
+ EscapeStringToStream(out_, "-fmodule-file=", options);
+ path_output_.WriteFile(out_, module_dep.pcm);
+ }
+ }
+
+ out_ << std::endl;
+ }
+}
+
void NinjaCBinaryTargetWriter::WritePCHCommands(
const std::vector<OutputFile>& input_deps,
const std::vector<OutputFile>& order_only_deps,
diff --git a/src/gn/ninja_c_binary_target_writer.h b/src/gn/ninja_c_binary_target_writer.h
index c2e63e4..bbc71c5 100644
--- a/src/gn/ninja_c_binary_target_writer.h
+++ b/src/gn/ninja_c_binary_target_writer.h
@@ -29,6 +29,12 @@
// Writes all flags for the compiler: includes, defines, cflags, etc.
void WriteCompilerVars(const std::vector<ModuleDep>& module_dep_info);
+ // Write module_deps or module_deps_no_self flags for clang modulemaps.
+ void WriteModuleDepsSubstitution(
+ const Substitution* substitution,
+ const std::vector<ModuleDep>& module_dep_info,
+ bool include_self);
+
// Writes build lines required for precompiled headers. Any generated
// object files will be appended to the |object_files|. Any generated
// non-object files (for instance, .gch files from a GCC toolchain, are
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc
index 542221a..6290301 100644
--- a/src/gn/ninja_c_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -1724,8 +1724,9 @@
std::unique_ptr<Tool> cxx_module_tool =
Tool::CreateTool(CTool::kCToolCxxModule);
TestWithScope::SetCommandForTool(
- "c++ {{source}} {{cflags}} {{cflags_cc}} {{defines}} {{include_dirs}} "
- "-fmodule-name={{label}} -c -x c++ -Xclang -emit-module -o {{output}}",
+ "c++ {{source}} {{cflags}} {{cflags_cc}} {{module_deps_no_self}} "
+ "{{defines}} {{include_dirs}} -fmodule-name={{label}} -c -x c++ "
+ "-Xclang -emit-module -o {{output}}",
cxx_module_tool.get());
cxx_module_tool->set_outputs(SubstitutionList::MakeForTest(
"{{source_out_dir}}/{{target_output_name}}.{{source_name_part}}.pcm"));
@@ -1774,7 +1775,8 @@
const char expected[] = R"(defines =
include_dirs =
-module_deps = -fmodules-embed-all-files -fmodule-file=//blah$:a=obj/blah/liba.a.pcm
+module_deps = -Xclang -fmodules-embed-all-files -fmodule-file=obj/blah/liba.a.pcm
+module_deps_no_self = -Xclang -fmodules-embed-all-files
cflags =
cflags_cc =
label = //blah$:a
@@ -1814,7 +1816,8 @@
const char expected[] = R"(defines =
include_dirs =
-module_deps = -fmodules-embed-all-files -fmodule-file=//stuff$:b=obj/stuff/libb.b.pcm
+module_deps = -Xclang -fmodules-embed-all-files -fmodule-file=obj/stuff/libb.b.pcm
+module_deps_no_self = -Xclang -fmodules-embed-all-files
cflags =
cflags_cc =
label = //stuff$:b
@@ -1835,6 +1838,45 @@
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
}
+ Target target3(&module_settings, Label(SourceDir("//things/"), "c"));
+ target3.set_output_type(Target::STATIC_LIBRARY);
+ target3.visibility().SetPublic();
+ target3.sources().push_back(SourceFile("//stuff/c.modulemap"));
+ target3.source_types_used().Set(SourceFile::SOURCE_MODULEMAP);
+ target3.private_deps().push_back(LabelTargetPair(&target));
+ target3.SetToolchain(&module_toolchain);
+ ASSERT_TRUE(target3.OnResolved(&err));
+
+ // A third library that depends on one of the previous static libraries, to
+ // check module_deps_no_self.
+ {
+ std::ostringstream out;
+ NinjaCBinaryTargetWriter writer(&target3, out);
+ writer.Run();
+
+ const char expected[] = R"(defines =
+include_dirs =
+module_deps = -Xclang -fmodules-embed-all-files -fmodule-file=obj/stuff/libc.c.pcm -fmodule-file=obj/blah/liba.a.pcm
+module_deps_no_self = -Xclang -fmodules-embed-all-files -fmodule-file=obj/blah/liba.a.pcm
+cflags =
+cflags_cc =
+label = //things$:c
+root_out_dir = withmodules
+target_out_dir = obj/things
+target_output_name = libc
+
+build obj/stuff/libc.c.pcm: cxx_module ../../stuff/c.modulemap | obj/blah/liba.a.pcm
+
+build obj/things/libc.a: alink || obj/blah/liba.a
+ arflags =
+ output_extension =
+ output_dir =
+)";
+
+ std::string out_str = out.str();
+ EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
+ }
+
Target depender(&module_settings, Label(SourceDir("//zap/"), "c"));
depender.set_output_type(Target::EXECUTABLE);
depender.sources().push_back(SourceFile("//zap/x.cc"));
@@ -1853,7 +1895,8 @@
const char expected[] = R"(defines =
include_dirs =
-module_deps = -fmodules-embed-all-files -fmodule-file=//blah$:a=obj/blah/liba.a.pcm -fmodule-file=//stuff$:b=obj/stuff/libb.b.pcm
+module_deps = -Xclang -fmodules-embed-all-files -fmodule-file=obj/blah/liba.a.pcm -fmodule-file=obj/stuff/libb.b.pcm
+module_deps_no_self = -Xclang -fmodules-embed-all-files -fmodule-file=obj/blah/liba.a.pcm -fmodule-file=obj/stuff/libb.b.pcm
cflags =
cflags_cc =
label = //zap$:c
