Include -fmodule-file flags in compile_commands.json This change updates GN to include the `-fmodule-file` flags for all module dependencies in the `compile_commands.json` file. To achieve this, the module dependency gathering logic has been refactored from `ninja_c_binary_target_writer.cc` into a new, shared utility, `ninja_module_writer_util.cc` in https://gn-review.googlesource.com/c/gn/+/19880 . The `compile_commands_writer` now uses this utility to append the correct module flags to the compilation commands, ensuring that tools have the information they need to correctly process C++ modules. Bug: 443228626 Change-Id: I74c2f7c01b3d8f0cd468594a2c8632275382549b Reviewed-on: https://gn-review.googlesource.com/c/gn/+/19840 Reviewed-by: David Turner <digit@google.com> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/compile_commands_writer.cc b/src/gn/compile_commands_writer.cc index 23d6029..c0c9ae7 100644 --- a/src/gn/compile_commands_writer.cc +++ b/src/gn/compile_commands_writer.cc
@@ -15,9 +15,12 @@ #include "gn/config_values_extractors.h" #include "gn/deps_iterator.h" #include "gn/escape.h" +#include "gn/ninja_module_writer_util.h" #include "gn/ninja_target_command_util.h" #include "gn/path_output.h" +#include "gn/resolved_target_data.h" #include "gn/string_output_buffer.h" +#include "gn/substitution_list.h" #include "gn/substitution_writer.h" // Structure of JSON output file @@ -49,6 +52,8 @@ std::string cflags_objcc; std::string framework_dirs; std::string frameworks; + std::string clang_module_deps; + std::string clang_module_deps_no_self; }; // Helper template function to call RecursiveTargetConfigToStream<std::string> @@ -71,6 +76,7 @@ void SetupCompileFlags(const Target* target, PathOutput& path_output, EscapeOptions opts, + const ResolvedTargetData& resolved, CompileFlags& flags) { bool has_precompiled_headers = target->config_values().has_precompiled_headers(); @@ -94,6 +100,28 @@ target, &ConfigValues::include_dirs, IncludeWriter(path_output)); + std::vector<ClangModuleDep> module_dep_info = + GetModuleDepsInformation(target, resolved); + if (!module_dep_info.empty()) { + std::ostringstream module_deps_out; + for (const auto& module_dep : module_dep_info) { + module_deps_out << " -fmodule-file="; + path_output.WriteFile(module_deps_out, module_dep.pcm); + } + base::EscapeJSONString(module_deps_out.str(), false, + &flags.clang_module_deps); + + std::ostringstream module_deps_no_self_out; + for (const auto& module_dep : module_dep_info) { + if (!module_dep.is_self) { + module_deps_no_self_out << " -fmodule-file="; + path_output.WriteFile(module_deps_no_self_out, module_dep.pcm); + } + } + base::EscapeJSONString(module_deps_no_self_out.str(), false, + &flags.clang_module_deps_no_self); + } + // Helper lambda to call WriteOneFlag() and return the resulting // escaped JSON string. auto one_flag = [&](RecursiveWriterConfig config, @@ -177,6 +205,10 @@ out << flags.frameworks; } else if (range.type == &CSubstitutionIncludeDirs) { out << flags.includes; + } else if (range.type == &CSubstitutionModuleDeps) { + out << flags.clang_module_deps; + } else if (range.type == &CSubstitutionModuleDepsNoSelf) { + out << flags.clang_module_deps_no_self; } else if (range.type == &CSubstitutionCFlags) { out << flags.cflags; } else if (range.type == &CSubstitutionCFlagsC) { @@ -233,6 +265,7 @@ EscapeOptions opts; opts.mode = ESCAPE_NINJA_PREFORMATTED_COMMAND; + ResolvedTargetData resolved; for (const auto* target : all_targets) { if (!target->IsBinary()) @@ -247,7 +280,7 @@ ESCAPE_NINJA_COMMAND); CompileFlags flags; - SetupCompileFlags(target, path_output, opts, flags); + SetupCompileFlags(target, path_output, opts, resolved, flags); for (const auto& source : target->sources()) { // If this source is not a C/C++/ObjC/ObjC++ source (not header) file,
diff --git a/src/gn/compile_commands_writer_unittest.cc b/src/gn/compile_commands_writer_unittest.cc index 2f0294c..badd100 100644 --- a/src/gn/compile_commands_writer_unittest.cc +++ b/src/gn/compile_commands_writer_unittest.cc
@@ -7,6 +7,7 @@ #include <memory> #include <sstream> #include <utility> +#include <vector> #include "gn/config.h" #include "gn/ninja_target_command_util.h" @@ -599,6 +600,91 @@ EXPECT_EQ(expected, out); } +TEST_F(CompileCommandsTest, ModuleMap) { + Err err; + + // This setup's toolchain does not have precompiled headers defined. + // A precompiled header toolchain. + Settings module_settings(build_settings(), "withmodules/"); + Toolchain module_toolchain(&module_settings, + Label(SourceDir("//toolchain/"), "withmodules")); + module_settings.set_toolchain_label(module_toolchain.label()); + module_settings.set_default_toolchain_label(toolchain()->label()); + + // Declare a C++ compiler that supports PCH. + std::unique_ptr<Tool> cxx = Tool::CreateTool(CTool::kCToolCxx); + CTool* cxx_tool = cxx->AsC(); + TestWithScope::SetCommandForTool( + "c++ {{source}} {{cflags}} {{cflags_cc}} {{module_deps}} " + "{{defines}} {{include_dirs}} -o {{output}}", + cxx_tool); + cxx_tool->set_outputs(SubstitutionList::MakeForTest( + "{{source_out_dir}}/{{target_output_name}}.{{source_name_part}}.o")); + module_toolchain.SetTool(std::move(cxx)); + + std::unique_ptr<Tool> cxx_module_tool = + Tool::CreateTool(CTool::kCToolCxxModule); + TestWithScope::SetCommandForTool( + "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")); + module_toolchain.SetTool(std::move(cxx_module_tool)); + + module_toolchain.ToolchainSetupComplete(); + + Target module_target(&module_settings, Label(SourceDir("//foo/"), "module")); + module_target.set_output_type(Target::SOURCE_SET); + module_target.visibility().SetPublic(); + module_target.sources().push_back(SourceFile("//foo/foo.modulemap")); + module_target.source_types_used().Set(SourceFile::SOURCE_MODULEMAP); + module_target.SetToolchain(&module_toolchain); + ASSERT_TRUE(module_target.OnResolved(&err)); + + Target dep_target(&module_settings, Label(SourceDir("//foo/"), "dep")); + dep_target.set_output_type(Target::SOURCE_SET); + dep_target.visibility().SetPublic(); + dep_target.sources().push_back(SourceFile("//foo/dep.cc")); + dep_target.source_types_used().Set(SourceFile::SOURCE_CPP); + dep_target.public_deps().push_back(LabelTargetPair(&module_target)); + dep_target.SetToolchain(&module_toolchain); + ASSERT_TRUE(dep_target.OnResolved(&err)); + + std::vector<const Target*> targets; + targets.push_back(&module_target); + targets.push_back(&dep_target); + + CompileCommandsWriter writer; + std::string out = writer.RenderJSON(build_settings(), targets); + +#if defined(OS_WIN) + const char expected[] = + "[\r\n" + " {\r\n" + " \"file\": \"../../foo/dep.cc\",\r\n" + " \"directory\": \"out/Debug\",\r\n" + " \"command\": \"c++ ../../foo/dep.cc " + "-fmodule-file=withmodules/obj/foo/module.foo.pcm -o " + "withmodules/obj/foo/dep.dep.o\"\r\n" + " }\r\n" + "]\r\n"; +#else + const char expected[] = + "[\n" + " {\n" + " \"file\": \"../../foo/dep.cc\",\n" + " \"directory\": \"out/Debug\",\n" + " \"command\": \"c++ ../../foo/dep.cc " + "-fmodule-file=withmodules/obj/foo/module.foo.pcm -o " + "withmodules/obj/foo/dep.dep.o\"\n" + " }\n" + "]\n"; +#endif + EXPECT_EQ(expected, out) << expected << "\n" << out; +} + TEST_F(CompileCommandsTest, CollectTargets) { // Contruct the dependency tree: //