Generate two modulemaps per target. This is required for layering check, otherwise we would not be able to: * Verify that public headers don't rely on anything in nonpublic deps. * Verify that non-public headers are not accessed The public modulemap: * Includes public headers * Declares that it uses public_deps The private modulemap: * Inculdes private headers * Declares that it uses deps + public_deps + public module Bug: b:500845363 Change-Id: I5909c81a024d3928f38176383ee884966a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/21802 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc index 03a92b5..7355920 100644 --- a/src/gn/ninja_binary_target_writer.cc +++ b/src/gn/ninja_binary_target_writer.cc
@@ -114,8 +114,8 @@ writer.Run(); } -void NinjaBinaryTargetWriter::WriteModuleMap(std::ostream& out, - const SourceDir& out_dir) { +void NinjaBinaryTargetWriter::WritePublicModuleMap(std::ostream& out, + const SourceDir& out_dir) { out << "module \"" << target_->module_name() << "\" {\n"; if (target_->all_headers_public()) { WriteModuleMapHeaders(out, out_dir, target_->sources(), settings_); @@ -131,6 +131,30 @@ out << " export *\n}\n"; } +void NinjaBinaryTargetWriter::WritePrivateModuleMap(std::ostream& out, + const SourceDir& out_dir) { + auto base = target_->modulemap_file()->GetDir(); + auto module_name = target_->module_name(); + out << "module \"impl:" << module_name << "\" {\n"; + if (!target_->all_headers_public()) { + WriteModuleMapHeaders(out, out_dir, target_->sources(), settings_); + } + out << " extern module \"" << module_name << "\" \"" + << target_->modulemap_file()->GetName() << "\"\n"; + out << " use \"" << module_name << "\"\n"; + + auto deps = ExpandModules(target_->private_deps()); + auto pub_deps = ExpandModules(target_->public_deps()); + deps.insert(deps.end(), pub_deps.begin(), pub_deps.end()); + std::ranges::sort(deps, [](const Target* lhs, const Target* rhs) { + return lhs->module_name() < rhs->module_name(); + }); + auto dirty = std::ranges::unique(deps); + deps.erase(dirty.begin(), dirty.end()); + WriteModuleDeps(out, deps, base); + out << "}\n"; +} + std::vector<OutputFile> NinjaBinaryTargetWriter::WriteInputsStampOrPhonyAndGetDep( size_t num_output_uses) const {
diff --git a/src/gn/ninja_binary_target_writer.h b/src/gn/ninja_binary_target_writer.h index 75448c0..ef91127 100644 --- a/src/gn/ninja_binary_target_writer.h +++ b/src/gn/ninja_binary_target_writer.h
@@ -21,7 +21,8 @@ ~NinjaBinaryTargetWriter() override; void Run() override; - void WriteModuleMap(std::ostream& out, const SourceDir& out_dir); + void WritePublicModuleMap(std::ostream& out, const SourceDir& out_dir); + void WritePrivateModuleMap(std::ostream& out, const SourceDir& out_dir); protected: // Structure used to return the classified deps from |GetDeps| method.
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc index 6b36f91..dce8dfd 100644 --- a/src/gn/ninja_c_binary_target_writer_unittest.cc +++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -2948,7 +2948,7 @@ std::ostringstream modulemap_out; SourceDir out_dir("//out/Debug/gen/"); - writer.WriteModuleMap(modulemap_out, out_dir); + writer.WritePublicModuleMap(modulemap_out, out_dir); const char expected_modulemap[] = "module \"//foo:bar\" {\n" @@ -3000,7 +3000,7 @@ std::ostringstream modulemap_out_no_public; - writer_no_public.WriteModuleMap(modulemap_out_no_public, out_dir); + writer_no_public.WritePublicModuleMap(modulemap_out_no_public, out_dir); const char expected_modulemap_no_public[] = "module \"//foo:no_public\" {\n" @@ -3076,7 +3076,7 @@ std::ostringstream public_modulemap; NinjaCBinaryTargetWriter(&root, ninja_out) - .WriteModuleMap(public_modulemap, out_dir); + .WritePublicModuleMap(public_modulemap, out_dir); const char expected_public[] = "module \"//foo:root\" {\n" @@ -3087,4 +3087,24 @@ "}\n"; EXPECT_EQ(expected_public, public_modulemap.str()) << expected_public << "\n" << public_modulemap.str(); + + std::ostringstream private_modulemap; + NinjaCBinaryTargetWriter(&root, ninja_out) + .WritePrivateModuleMap(private_modulemap, out_dir); + + const char expected_private[] = + "module \"impl://foo:root\" {\n" + " textual header \"../../../foo/private_header.h\"\n" + " extern module \"//foo:root\" \"root.modulemap\"\n" + " use \"//foo:root\"\n" + " extern module \"//foo:public_dep\" \"public_dep.modulemap\"\n" + " use \"//foo:public_dep\"\n" + " extern module \"//private:private_dep\" " + "\"../private/private_dep.modulemap\"\n" + " use \"//private:private_dep\"\n" + "}\n"; + + EXPECT_EQ(expected_private, private_modulemap.str()) + << expected_private << "\n" + << private_modulemap.str(); }
diff --git a/src/gn/ninja_target_writer.cc b/src/gn/ninja_target_writer.cc index 46964ee..e9c4bf4 100644 --- a/src/gn/ninja_target_writer.cc +++ b/src/gn/ninja_target_writer.cc
@@ -169,13 +169,22 @@ if (target->module_type() == Target::GENERATED_TEXTUAL_MODULEMAP) { const SourceFile* modulemap = target->modulemap_file(); CHECK(modulemap); - StringOutputBuffer modulemap_storage; - std::ostream os(&modulemap_storage); - writer.WriteModuleMap(os, modulemap->GetDir()); - base::FilePath file_path = + // Write public module map + StringOutputBuffer public_storage; + std::ostream public_os(&public_storage); + writer.WritePublicModuleMap(public_os, modulemap->GetDir()); + base::FilePath public_path = settings->build_settings()->GetFullPath(*modulemap); - modulemap_storage.WriteToFileIfChanged(file_path, nullptr); + public_storage.WriteToFileIfChanged(public_path, nullptr); + + // Write private module map adjacent to the public one + StringOutputBuffer private_storage; + std::ostream private_os(&private_storage); + writer.WritePrivateModuleMap(private_os, modulemap->GetDir()); + base::FilePath private_path = settings->build_settings()->GetFullPath( + *target->private_modulemap_file()); + private_storage.WriteToFileIfChanged(private_path, nullptr); } writer.Run(); } else {
diff --git a/src/gn/target.cc b/src/gn/target.cc index 54281b3..956d66c 100644 --- a/src/gn/target.cc +++ b/src/gn/target.cc
@@ -25,6 +25,8 @@ namespace { +constexpr std::string_view kModuleMapExt = ".modulemap"; + using ConfigSet = std::set<const Config*>; // Used to optimize the search for a target generating a given output file. @@ -1427,3 +1429,17 @@ return nullptr; } } + +const SourceFile* Target::private_modulemap_file() const { + if (private_modulemap_file_.is_null() && + module_type_ == GENERATED_TEXTUAL_MODULEMAP) { + auto public_modulemap = modulemap_file(); + std::string private_name = public_modulemap->GetName(); + // foo.modulemap -> foo.private.modulemap + private_name.insert(private_name.length() - kModuleMapExt.size(), + ".private"); + private_modulemap_file_ = public_modulemap->GetDir().ResolveRelativeFile( + Value(nullptr, private_name), nullptr); + } + return &private_modulemap_file_; +}
diff --git a/src/gn/target.h b/src/gn/target.h index ea1c452..210df0c 100644 --- a/src/gn/target.h +++ b/src/gn/target.h
@@ -361,6 +361,7 @@ ModuleType module_type() const { return module_type_; } void set_module_type(ModuleType type); const SourceFile* modulemap_file() const; + const SourceFile* private_modulemap_file() const; // The toolchain is only known once this target is resolved (all if its // dependencies are known). They will be null until then. Generally, this can @@ -540,6 +541,10 @@ ModuleType module_type_ = NO_MODULEMAP; // Only filled if the module type is GENERATED_* SourceFile generated_modulemap_file_; + // For performance reasons we cache private_modulemap_file. + // Modulemap files are passed as pointers, so to keep them as pointers instead + // of values we need to store them somewhere. + mutable SourceFile private_modulemap_file_; FileList sources_; SourceFileTypeSet source_types_used_;