From 18f87c87f86473dd2106b6d8ab7acc9def99b9b1 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Sun, 14 May 2023 17:24:02 -0400 Subject: cmCxxModuleMapper: track whether modules are private or not This allows collation to give a useful error message when it finds usage of a private module rather than collation just not informing the compilation and the compiler erroring out about not being able to import unknown modules (which exists, but it was not told about due to visibility). Fixes: #24652 --- Source/cmCxxModuleMapper.cxx | 65 +++++++++++++++++++++++++-------------- Source/cmCxxModuleMapper.h | 5 ++- Source/cmGlobalNinjaGenerator.cxx | 44 ++++++++++++++++++++------ 3 files changed, 78 insertions(+), 36 deletions(-) diff --git a/Source/cmCxxModuleMapper.cxx b/Source/cmCxxModuleMapper.cxx index e320f54..e836a2a 100644 --- a/Source/cmCxxModuleMapper.cxx +++ b/Source/cmCxxModuleMapper.cxx @@ -61,13 +61,15 @@ std::string const& CxxBmiLocation::Location() const return empty; } -cm::optional CxxModuleLocations::BmiGeneratorPathForModule( +CxxBmiLocation CxxModuleLocations::BmiGeneratorPathForModule( std::string const& logical_name) const { - if (auto l = this->BmiLocationForModule(logical_name)) { - return this->PathForGenerator(std::move(*l)); + auto bmi_loc = this->BmiLocationForModule(logical_name); + if (bmi_loc.IsKnown() && !bmi_loc.IsPrivate()) { + bmi_loc = + CxxBmiLocation::Known(this->PathForGenerator(bmi_loc.Location())); } - return {}; + return bmi_loc; } namespace { @@ -86,18 +88,21 @@ std::string CxxModuleMapContentClang(CxxModuleLocations const& loc, // A series of flags which tell the compiler where to look for modules. for (auto const& p : obj.Provides) { - if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) { + auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName); + if (bmi_loc.IsKnown()) { // Force the TU to be considered a C++ module source file regardless of // extension. mm << "-x c++-module\n"; - mm << "-fmodule-output=" << *bmi_loc << '\n'; + mm << "-fmodule-output=" << bmi_loc.Location() << '\n'; break; } } for (auto const& r : obj.Requires) { - if (auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName)) { - mm << "-fmodule-file=" << r.LogicalName << "=" << *bmi_loc << '\n'; + auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName); + if (bmi_loc.IsKnown()) { + mm << "-fmodule-file=" << r.LogicalName << "=" << bmi_loc.Location() + << '\n'; } } @@ -120,13 +125,15 @@ std::string CxxModuleMapContentGcc(CxxModuleLocations const& loc, mm << "$root " << loc.RootDirectory << "\n"; for (auto const& p : obj.Provides) { - if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) { - mm << p.LogicalName << ' ' << *bmi_loc << '\n'; + auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName); + if (bmi_loc.IsKnown()) { + mm << p.LogicalName << ' ' << bmi_loc.Location() << '\n'; } } for (auto const& r : obj.Requires) { - if (auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName)) { - mm << r.LogicalName << ' ' << *bmi_loc << '\n'; + auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName); + if (bmi_loc.IsKnown()) { + mm << r.LogicalName << ' ' << bmi_loc.Location() << '\n'; } } @@ -167,8 +174,9 @@ std::string CxxModuleMapContentMsvc(CxxModuleLocations const& loc, mm << "-internalPartition\n"; } - if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) { - mm << "-ifcOutput " << *bmi_loc << '\n'; + auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName); + if (bmi_loc.IsKnown()) { + mm << "-ifcOutput " << bmi_loc.Location() << '\n'; } } @@ -176,10 +184,11 @@ std::string CxxModuleMapContentMsvc(CxxModuleLocations const& loc, std::set transitive_usage_names; for (auto const& r : obj.Requires) { - if (auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName)) { + auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName); + if (bmi_loc.IsKnown()) { auto flag = flag_for_method(r.Method); - mm << flag << ' ' << r.LogicalName << '=' << *bmi_loc << "\n"; + mm << flag << ' ' << r.LogicalName << '=' << bmi_loc.Location() << "\n"; transitive_usage_directs.insert(r.LogicalName); // Insert transitive usages. @@ -281,18 +290,28 @@ std::set CxxModuleUsageSeed( for (cmScanDepInfo const& object : objects) { // Add references for each of the provided modules. for (auto const& p : object.Provides) { - if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) { + auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName); + if (bmi_loc.IsKnown()) { // XXX(cxx-modules): How to support header units? - usages.AddReference(p.LogicalName, *bmi_loc, LookupMethod::ByName); + usages.AddReference(p.LogicalName, bmi_loc.Location(), + LookupMethod::ByName); } } // For each requires, pull in what is required. for (auto const& r : object.Requires) { - // Find transitive usages. - auto transitive_usages = usages.Usage.find(r.LogicalName); // Find the required name in the current target. auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName); + if (bmi_loc.IsPrivate()) { + cmSystemTools::Error( + cmStrCat("Unable to use module '", r.LogicalName, + "' as it is 'PRIVATE' and therefore not accessible outside " + "of its owning target.")); + continue; + } + + // Find transitive usages. + auto transitive_usages = usages.Usage.find(r.LogicalName); for (auto const& p : object.Provides) { auto& this_usages = usages.Usage[p.LogicalName]; @@ -304,14 +323,14 @@ std::set CxxModuleUsageSeed( if (transitive_usages != usages.Usage.end()) { this_usages.insert(transitive_usages->second.begin(), transitive_usages->second.end()); - } else if (bmi_loc) { + } else if (bmi_loc.IsKnown()) { // Mark that we need to update transitive usages later. internal_usages[p.LogicalName].insert(r.LogicalName); } } - if (bmi_loc) { - usages.AddReference(r.LogicalName, *bmi_loc, r.Method); + if (bmi_loc.IsKnown()) { + usages.AddReference(r.LogicalName, bmi_loc.Location(), r.Method); } } } diff --git a/Source/cmCxxModuleMapper.h b/Source/cmCxxModuleMapper.h index f405054..ef01e48 100644 --- a/Source/cmCxxModuleMapper.h +++ b/Source/cmCxxModuleMapper.h @@ -50,12 +50,11 @@ struct CxxModuleLocations std::function PathForGenerator; // Lookup the BMI location of a logical module name. - std::function(std::string const&)> - BmiLocationForModule; + std::function BmiLocationForModule; // Returns the generator path (if known) for the BMI given a // logical module name. - cm::optional BmiGeneratorPathForModule( + CxxBmiLocation BmiGeneratorPathForModule( std::string const& logical_name) const; }; diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 7626fa3..21d1e6d 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -2526,7 +2526,12 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( CxxModuleUsage usages; // Map from module name to module file path, if known. - std::map mod_files; + struct AvailableModuleInfo + { + std::string BmiPath; + bool IsPrivate; + }; + std::map mod_files; // Populate the module map with those provided by linked targets first. for (std::string const& linked_target_dir : linked_target_dirs) { @@ -2550,7 +2555,15 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( Json::Value const& target_modules = ltm["modules"]; if (target_modules.isObject()) { for (auto i = target_modules.begin(); i != target_modules.end(); ++i) { - mod_files[i.key().asString()] = i->asString(); + Json::Value const& visible_module = *i; + if (visible_module.isObject()) { + Json::Value const& bmi_path = visible_module["bmi"]; + Json::Value const& is_private = visible_module["is-private"]; + mod_files[i.key().asString()] = AvailableModuleInfo{ + bmi_path.asString(), + is_private.asBool(), + }; + } } } Json::Value const& target_modules_references = ltm["references"]; @@ -2631,8 +2644,15 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( cmSystemTools::ReplaceString(safe_logical_name, ":", "-"); mod = cmStrCat(module_dir, safe_logical_name, module_ext); } - mod_files[p.LogicalName] = mod; - target_modules[p.LogicalName] = mod; + mod_files[p.LogicalName] = AvailableModuleInfo{ + mod, + false, // Always visible within our own target. + }; + Json::Value& module_info = target_modules[p.LogicalName] = + Json::objectValue; + module_info["bmi"] = mod; + module_info["is-private"] = + cmDyndepCollation::IsObjectPrivate(object.PrimaryOutput, export_info); } } @@ -2652,12 +2672,15 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( return path; }; locs.BmiLocationForModule = - [&mod_files](std::string const& logical) -> cm::optional { + [&mod_files](std::string const& logical) -> CxxBmiLocation { auto m = mod_files.find(logical); if (m != mod_files.end()) { - return m->second; + if (m->second.IsPrivate) { + return CxxBmiLocation::Private(); + } + return CxxBmiLocation::Known(m->second.BmiPath); } - return {}; + return CxxBmiLocation::Unknown(); }; // Insert information about the current target's modules. @@ -2679,13 +2702,14 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( build.ImplicitOuts.clear(); for (auto const& p : object.Provides) { build.ImplicitOuts.push_back( - this->ConvertToNinjaPath(mod_files[p.LogicalName])); + this->ConvertToNinjaPath(mod_files[p.LogicalName].BmiPath)); } build.ImplicitDeps.clear(); for (auto const& r : object.Requires) { auto mit = mod_files.find(r.LogicalName); if (mit != mod_files.end()) { - build.ImplicitDeps.push_back(this->ConvertToNinjaPath(mit->second)); + build.ImplicitDeps.push_back( + this->ConvertToNinjaPath(mit->second.BmiPath)); } } build.Variables.clear(); @@ -2751,7 +2775,7 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( [mod_files](std::string const& name) -> cm::optional { auto m = mod_files.find(name); if (m != mod_files.end()) { - return m->second; + return m->second.BmiPath; } return {}; }; -- cgit v0.12