Merge pull request 'Track what a primary module interface imports' (#29) from claude/issue-26 into master
Some checks failed
CI / build-test-release (push) Failing after 6m3s
Some checks failed
CI / build-test-release (push) Failing after 6m3s
This commit is contained in:
commit
ded60bb8c3
12 changed files with 437 additions and 22 deletions
|
|
@ -125,7 +125,7 @@ Per-import precise tracking for both within-project and cross-project module dep
|
||||||
|
|
||||||
Diamond deps (`A → {B, C}; B → X; C → X`) build `X` exactly once via a `std::shared_future<BuildResult>` cache.
|
Diamond deps (`A → {B, C}; B → X; C → X`) build `X` exactly once via a `std::shared_future<BuildResult>` cache.
|
||||||
|
|
||||||
Tracking is derived from each translation unit's `import` statements, which are scanned when the sources are declared. `cfg.dependencies` is often assigned *afterwards* — `AddTest` works that way — so `Build()` re-resolves any import that matched nothing at scan time before it compares mtimes. Without that, a consumer of a dependency's module carried no edge to it at all: adding a data member to that dependency's interface rebuilt the library, relinked the consumer, and left the consumer's object compiled against the old class layout. Nothing fails to link when a member is added, so the result was a working build and a crash later.
|
Tracking is derived from each translation unit's `import` statements, which are scanned when the sources are declared. Every kind of unit is scanned — primary module interfaces, partitions and implementation units alike — and an interface that imports a sibling module in the same `Configuration` also gets its compile ordered behind it. `cfg.dependencies` is often assigned *afterwards* — `AddTest` works that way — so `Build()` re-resolves any import that matched nothing at scan time before it compares mtimes. Without all of that, a consumer of a dependency's module carried no edge to it at all: adding a data member to that dependency's interface rebuilt the library, relinked the consumer, and left the consumer's object compiled against the old class layout. Nothing fails to link when a member is added, so the result was a working build and a crash later.
|
||||||
|
|
||||||
Everything that changes what gets built belongs in the variant id, since it names the `bin/` and `build/` directory. That includes project args crafter-build itself doesn't interpret: `crafter-build` and `crafter-build -- --no-webgpu` get separate directories rather than interleaving their outputs in one. The cached host PCMs under `<cache>/crafter.build/<target>-<march>/` are shared by every crafter-build on the machine, so they are invalidated by a hash of the module sources rather than by mtime — an mtime can't tell a newer PCM from one built by a different install.
|
Everything that changes what gets built belongs in the variant id, since it names the `bin/` and `build/` directory. That includes project args crafter-build itself doesn't interpret: `crafter-build` and `crafter-build -- --no-webgpu` get separate directories rather than interleaving their outputs in one. The cached host PCMs under `<cache>/crafter.build/<target>-<march>/` are shared by every crafter-build on the machine, so they are invalidated by a hash of the module sources rather than by mtime — an mtime can't tell a newer PCM from one built by a different install.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -73,6 +73,7 @@ void Configuration::ResolvePendingImports() {
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
for(const std::unique_ptr<Module>& interface : interfaces) {
|
for(const std::unique_ptr<Module>& interface : interfaces) {
|
||||||
|
sweep(interface->pendingImports, interface->moduleDependencies, interface->externalModuleDependencies);
|
||||||
for(const std::unique_ptr<ModulePartition>& partition : interface->partitions) {
|
for(const std::unique_ptr<ModulePartition>& partition : interface->partitions) {
|
||||||
sweep(partition->pendingImports, partition->moduleDependencies, partition->externalModuleDependencies);
|
sweep(partition->pendingImports, partition->moduleDependencies, partition->externalModuleDependencies);
|
||||||
}
|
}
|
||||||
|
|
@ -104,19 +105,26 @@ void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfac
|
||||||
tempModulePaths[i] = {file, fileContent, nullptr, nullptr};
|
tempModulePaths[i] = {file, fileContent, nullptr, nullptr};
|
||||||
}
|
}
|
||||||
|
|
||||||
std::erase_if(tempModulePaths, [this](std::tuple<fs::path, std::string, ModulePartition*, Module*>& file) {
|
// Primary interface units first, so the partition pass below can find their
|
||||||
|
// parent Module. They stay in tempModulePaths — a primary unit's own
|
||||||
|
// `import X;` lines are layout dependencies exactly like a partition's, and
|
||||||
|
// dropping the entry here is what left them unrecorded (issue #26). Marked
|
||||||
|
// by a null partition slot with the Module slot filled in.
|
||||||
|
for(std::tuple<fs::path, std::string, ModulePartition*, Module*>& file : tempModulePaths) {
|
||||||
std::smatch match;
|
std::smatch match;
|
||||||
if (std::regex_search(std::get<1>(file), match, std::regex(R"(export module ([a-zA-Z0-9_\.\-]+);)"))) {
|
if (std::regex_search(std::get<1>(file), match, std::regex(R"(export module ([a-zA-Z0-9_\.\-]+);)"))) {
|
||||||
std::get<0>(file).replace_extension("");
|
fs::path pthCpy = std::get<0>(file);
|
||||||
this->interfaces.push_back(std::make_unique<Module>(std::move(match[1].str()), std::move(std::get<0>(file))));
|
pthCpy.replace_extension("");
|
||||||
return true;
|
this->interfaces.push_back(std::make_unique<Module>(std::move(match[1].str()), std::move(pthCpy)));
|
||||||
} else {
|
std::get<3>(file) = this->interfaces.back().get();
|
||||||
return false;
|
}
|
||||||
}
|
}
|
||||||
});
|
|
||||||
|
|
||||||
for(std::uint16_t i = 0; i < tempModulePaths.size(); i++) {
|
for(std::uint16_t i = 0; i < tempModulePaths.size(); i++) {
|
||||||
std::smatch match;
|
std::smatch match;
|
||||||
|
if (std::get<3>(tempModulePaths[i]) != nullptr) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
if (std::regex_search(std::get<1>(tempModulePaths[i]), match, std::regex(R"(export module ([a-zA-Z_0-9\.\-]+):([a-zA-Z_0-9\.\-]+);)"))) {
|
if (std::regex_search(std::get<1>(tempModulePaths[i]), match, std::regex(R"(export module ([a-zA-Z_0-9\.\-]+):([a-zA-Z_0-9\.\-]+);)"))) {
|
||||||
for(const std::unique_ptr<Module>& modulee : this->interfaces) {
|
for(const std::unique_ptr<Module>& modulee : this->interfaces) {
|
||||||
if(modulee->name == match[1]) {
|
if(modulee->name == match[1]) {
|
||||||
|
|
@ -142,6 +150,22 @@ void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfac
|
||||||
Module* parentModule = std::get<3>(file);
|
Module* parentModule = std::get<3>(file);
|
||||||
const std::string& fileContent = std::get<1>(file);
|
const std::string& fileContent = std::get<1>(file);
|
||||||
|
|
||||||
|
// Primary interface unit: record only its module imports. Its
|
||||||
|
// `[export] import :Part;` lines need no edge — Module::Check walks
|
||||||
|
// every partition it owns and Module::Compile builds them all before
|
||||||
|
// precompiling itself, so partitions are covered wholesale.
|
||||||
|
if (partition == nullptr) {
|
||||||
|
std::regex primaryPattern(R"(import ([a-zA-Z_0-9\.\-]+);)");
|
||||||
|
std::sregex_iterator primaryCurrent(fileContent.begin(), fileContent.end(), primaryPattern);
|
||||||
|
std::sregex_iterator primaryLast;
|
||||||
|
while (primaryCurrent != primaryLast) {
|
||||||
|
std::smatch match = *primaryCurrent;
|
||||||
|
resolveImport(match[1].str(), parentModule->moduleDependencies, parentModule->externalModuleDependencies, parentModule->pendingImports);
|
||||||
|
++primaryCurrent;
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
std::regex partitionPattern(R"(import :([a-zA-Z_\-0-9\.]+);)");
|
std::regex partitionPattern(R"(import :([a-zA-Z_\-0-9\.]+);)");
|
||||||
std::sregex_iterator currentMatch(fileContent.begin(), fileContent.end(), partitionPattern);
|
std::sregex_iterator currentMatch(fileContent.begin(), fileContent.end(), partitionPattern);
|
||||||
std::sregex_iterator lastMatch;
|
std::sregex_iterator lastMatch;
|
||||||
|
|
@ -882,9 +906,24 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
|
||||||
if (ext.latestArtifact > externalFloor) externalFloor = ext.latestArtifact;
|
if (ext.latestArtifact > externalFloor) externalFloor = ext.latestArtifact;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check every interface before compiling any of them. Module::Compile waits
|
||||||
|
// on the `compiled` flag of each sibling module it imports, and that flag is
|
||||||
|
// set either by a Compile that runs or by the Check that decides none is
|
||||||
|
// needed — so a Check still pending while another module's thread is already
|
||||||
|
// waiting would block on a flag nothing goes on to raise.
|
||||||
|
std::vector<Module*> staleInterfaces;
|
||||||
for(std::unique_ptr<Module>& interface : config.interfaces) {
|
for(std::unique_ptr<Module>& interface : config.interfaces) {
|
||||||
if(interface->Check(pcmDir, externalFloor)) {
|
if(interface->Check(pcmDir, externalFloor)) {
|
||||||
Module* mod = interface.get();
|
staleInterfaces.push_back(interface.get());
|
||||||
|
buildResult.repack = true;
|
||||||
|
}
|
||||||
|
files += std::format(" {}/{}.o", buildDir.string(), interface->path.filename().string());
|
||||||
|
for(std::unique_ptr<ModulePartition>& part : interface->partitions) {
|
||||||
|
files += std::format(" {}/{}.o", buildDir.string(), part->path.filename().string());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for(Module* mod : staleInterfaces) {
|
||||||
threads.emplace_back([mod, &command, &pcmDir, &buildDir, &buildCancelled, &buildError]() {
|
threads.emplace_back([mod, &command, &pcmDir, &buildDir, &buildCancelled, &buildError]() {
|
||||||
Progress::Task task(std::format("Compiling interface {}", mod->path.filename().string()));
|
Progress::Task task(std::format("Compiling interface {}", mod->path.filename().string()));
|
||||||
try {
|
try {
|
||||||
|
|
@ -896,12 +935,6 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
buildResult.repack = true;
|
|
||||||
}
|
|
||||||
files += std::format(" {}/{}.o", buildDir.string(), interface->path.filename().string());
|
|
||||||
for(std::unique_ptr<ModulePartition>& part : interface->partitions) {
|
|
||||||
files += std::format(" {}/{}.o", buildDir.string(), part->path.filename().string());
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
for(Implementation& implementation : config.implementations) {
|
for(Implementation& implementation : config.implementations) {
|
||||||
|
|
|
||||||
|
|
@ -39,6 +39,10 @@ namespace Crafter {
|
||||||
}
|
}
|
||||||
needsRecompiling = false;
|
needsRecompiling = false;
|
||||||
compiled.store(true);
|
compiled.store(true);
|
||||||
|
// Nothing waits on `compiled` until the compile threads start,
|
||||||
|
// which is after every Check has run — but notify anyway so the
|
||||||
|
// flag is never left set without a wake-up behind it.
|
||||||
|
compiled.notify_all();
|
||||||
return false;
|
return false;
|
||||||
} else {
|
} else {
|
||||||
needsRecompiling = true;
|
needsRecompiling = true;
|
||||||
|
|
@ -100,24 +104,55 @@ namespace Crafter {
|
||||||
std::string pcmPath = std::format("{}.pcm", (pcmDir/path.filename()).generic_string());
|
std::string pcmPath = std::format("{}.pcm", (pcmDir/path.filename()).generic_string());
|
||||||
std::string cppmPath = std::format("{}.cppm", path.generic_string());
|
std::string cppmPath = std::format("{}.cppm", path.generic_string());
|
||||||
if(fs::exists(pcmPath) && std::max(fs::last_write_time(cppmPath), sourceFloor) < fs::last_write_time(pcmPath)) {
|
if(fs::exists(pcmPath) && std::max(fs::last_write_time(cppmPath), sourceFloor) < fs::last_write_time(pcmPath)) {
|
||||||
|
fs::file_time_type pcmTime = fs::last_write_time(pcmPath);
|
||||||
|
// Every partition gets Check()ed even once one is known stale:
|
||||||
|
// Compile() drives partition rebuilds off their own
|
||||||
|
// needsRecompiling flags, so short-circuiting here would leave
|
||||||
|
// the later ones unevaluated and silently unbuilt.
|
||||||
bool depCheck = false;
|
bool depCheck = false;
|
||||||
for(std::unique_ptr<ModulePartition>& partition : partitions) {
|
for(std::unique_ptr<ModulePartition>& partition : partitions) {
|
||||||
if(partition->Check(pcmDir, sourceFloor)) {
|
if(partition->Check(pcmDir, sourceFloor)) {
|
||||||
depCheck = true;
|
depCheck = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Modules this interface unit imports directly. Local ones share
|
||||||
|
// our pcmDir and are resolved recursively; external ones are
|
||||||
|
// compared by PCM mtime, their owning Configuration having
|
||||||
|
// already finished building by the time we run.
|
||||||
|
for(Module* dependency : moduleDependencies) {
|
||||||
|
if(dependency->Check(pcmDir, sourceFloor)) {
|
||||||
|
depCheck = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if(!depCheck) {
|
||||||
|
for(const auto& [externalMod, externalPcmPath] : externalModuleDependencies) {
|
||||||
|
std::error_code ec;
|
||||||
|
fs::file_time_type t = fs::last_write_time(externalPcmPath, ec);
|
||||||
|
if (!ec && t >= pcmTime) {
|
||||||
|
depCheck = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
if(depCheck) {
|
if(depCheck) {
|
||||||
needsRecompiling = true;
|
needsRecompiling = true;
|
||||||
return true;
|
return true;
|
||||||
} else {
|
} else {
|
||||||
needsRecompiling = false;
|
needsRecompiling = false;
|
||||||
compiled.store(true);
|
compiled.store(true);
|
||||||
|
compiled.notify_all();
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
// Already known stale, but the dependencies still need
|
||||||
|
// evaluating: an unchecked local module would never get built,
|
||||||
|
// and Compile() below waits on its `compiled` flag.
|
||||||
for(std::unique_ptr<ModulePartition>& partition : partitions) {
|
for(std::unique_ptr<ModulePartition>& partition : partitions) {
|
||||||
partition->Check(pcmDir, sourceFloor);
|
partition->Check(pcmDir, sourceFloor);
|
||||||
}
|
}
|
||||||
|
for(Module* dependency : moduleDependencies) {
|
||||||
|
dependency->Check(pcmDir, sourceFloor);
|
||||||
|
}
|
||||||
needsRecompiling = true;
|
needsRecompiling = true;
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
@ -127,6 +162,17 @@ namespace Crafter {
|
||||||
}
|
}
|
||||||
|
|
||||||
void Module::Compile(const std::string_view clang, const fs::path& pcmDir, const fs::path& buildDir, std::atomic<bool>& buildCancelled, std::string& buildError) {
|
void Module::Compile(const std::string_view clang, const fs::path& pcmDir, const fs::path& buildDir, std::atomic<bool>& buildCancelled, std::string& buildError) {
|
||||||
|
// A sibling module in the same Configuration that this interface unit
|
||||||
|
// imports has to have its PCM on disk before we precompile against it.
|
||||||
|
// Safe to block here: Build() Checks every interface before it spawns
|
||||||
|
// any compile thread, so a dependency that needs no rebuild already has
|
||||||
|
// `compiled` set and this returns immediately.
|
||||||
|
for(Module* dependency : moduleDependencies) {
|
||||||
|
if(!dependency->compiled.load()) {
|
||||||
|
dependency->compiled.wait(false);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
std::vector<std::thread> threads;
|
std::vector<std::thread> threads;
|
||||||
threads.reserve(partitions.size());
|
threads.reserve(partitions.size());
|
||||||
for(std::unique_ptr<ModulePartition>& part : partitions) {
|
for(std::unique_ptr<ModulePartition>& part : partitions) {
|
||||||
|
|
|
||||||
|
|
@ -22,7 +22,7 @@ namespace Crafter {
|
||||||
// Names that never resolve (`std`, module-mapped externals) stay here.
|
// Names that never resolve (`std`, module-mapped externals) stay here.
|
||||||
std::vector<std::string> pendingImports;
|
std::vector<std::string> pendingImports;
|
||||||
std::atomic<bool> compiled;
|
std::atomic<bool> compiled;
|
||||||
bool needsRecompiling;
|
bool needsRecompiling = false;
|
||||||
bool checked = false;
|
bool checked = false;
|
||||||
std::string name;
|
std::string name;
|
||||||
fs::path path;
|
fs::path path;
|
||||||
|
|
@ -33,8 +33,17 @@ namespace Crafter {
|
||||||
|
|
||||||
export class Module {
|
export class Module {
|
||||||
public:
|
public:
|
||||||
|
// A primary module interface unit imports things too — `import Base;`
|
||||||
|
// in `export module Widget;` is as much a layout dependency as the
|
||||||
|
// same line in a partition. Recorded on the same three vectors, with
|
||||||
|
// the same meanings, so Check() can see through them and Compile()
|
||||||
|
// can order itself behind a sibling module in the same Configuration
|
||||||
|
// (issue #26).
|
||||||
|
std::vector<Module*> moduleDependencies;
|
||||||
|
std::vector<std::pair<Module*, fs::path>> externalModuleDependencies;
|
||||||
|
std::vector<std::string> pendingImports;
|
||||||
std::atomic<bool> compiled;
|
std::atomic<bool> compiled;
|
||||||
bool needsRecompiling;
|
bool needsRecompiling = false;
|
||||||
bool checked = false;
|
bool checked = false;
|
||||||
std::vector<std::unique_ptr<ModulePartition>> partitions;
|
std::vector<std::unique_ptr<ModulePartition>> partitions;
|
||||||
std::string name;
|
std::string name;
|
||||||
|
|
|
||||||
|
|
@ -108,6 +108,7 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
|
||||||
cfg.AddTest("ModuleInterface").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("ModuleInterface").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("DependencyLink").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("DependencyLink").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("IncrementalInterfaceChange").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("IncrementalInterfaceChange").Dependencies({ CrafterBuildLib.get() });
|
||||||
|
cfg.AddTest("TransitiveInterfaceChange").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("CleanProject").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("CleanProject").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("ShaderCompile").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("ShaderCompile").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("StandardArgs").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("StandardArgs").Dependencies({ CrafterBuildLib.get() });
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
export module Base;
|
||||||
|
import std;
|
||||||
|
|
||||||
|
export namespace Demo {
|
||||||
|
class Payload {
|
||||||
|
public:
|
||||||
|
// The whole point of the fixture: `first` moves, and nothing about any
|
||||||
|
// signature or mangled name changes with it.
|
||||||
|
std::array<char, 4096> padding{};
|
||||||
|
std::int32_t first = 1;
|
||||||
|
void Stamp();
|
||||||
|
};
|
||||||
|
}
|
||||||
11
tests/TransitiveInterfaceChange/fixture/base/Base.cpp
Normal file
11
tests/TransitiveInterfaceChange/fixture/base/Base.cpp
Normal file
|
|
@ -0,0 +1,11 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
module Base;
|
||||||
|
import std;
|
||||||
|
|
||||||
|
namespace Demo {
|
||||||
|
void Payload::Stamp() {
|
||||||
|
first = 11;
|
||||||
|
}
|
||||||
|
}
|
||||||
13
tests/TransitiveInterfaceChange/fixture/base/Base.cppm
Normal file
13
tests/TransitiveInterfaceChange/fixture/base/Base.cppm
Normal file
|
|
@ -0,0 +1,13 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
export module Base;
|
||||||
|
import std;
|
||||||
|
|
||||||
|
export namespace Demo {
|
||||||
|
class Payload {
|
||||||
|
public:
|
||||||
|
std::int32_t first = 1;
|
||||||
|
void Stamp();
|
||||||
|
};
|
||||||
|
}
|
||||||
19
tests/TransitiveInterfaceChange/fixture/main.cpp
Normal file
19
tests/TransitiveInterfaceChange/fixture/main.cpp
Normal file
|
|
@ -0,0 +1,19 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
// Consumer of the layout under test. `Thing` lives on the stack here, so a
|
||||||
|
// stale object file that still believes the old `sizeof(Payload)` lets the
|
||||||
|
// library's Fill() write past what this frame reserved. Reports the disagreement
|
||||||
|
// as a non-zero exit rather than relying on the corruption to fault.
|
||||||
|
import Widget;
|
||||||
|
import std;
|
||||||
|
|
||||||
|
int main() {
|
||||||
|
Demo::Thing thing;
|
||||||
|
thing.Fill();
|
||||||
|
std::println("first={} tail={} sizeof={}", thing.payload.first, thing.tail, sizeof(Demo::Thing));
|
||||||
|
if (thing.payload.first != 11 || thing.tail != 7) {
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
12
tests/TransitiveInterfaceChange/fixture/widget/Widget.cpp
Normal file
12
tests/TransitiveInterfaceChange/fixture/widget/Widget.cpp
Normal file
|
|
@ -0,0 +1,12 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
module Widget;
|
||||||
|
import std;
|
||||||
|
|
||||||
|
namespace Demo {
|
||||||
|
void Thing::Fill() {
|
||||||
|
payload.Stamp();
|
||||||
|
tail = 7;
|
||||||
|
}
|
||||||
|
}
|
||||||
18
tests/TransitiveInterfaceChange/fixture/widget/Widget.cppm
Normal file
18
tests/TransitiveInterfaceChange/fixture/widget/Widget.cppm
Normal file
|
|
@ -0,0 +1,18 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
// A *primary* module interface unit — no partitions — that imports another
|
||||||
|
// module and embeds one of its types by value. The import lives here rather
|
||||||
|
// than in a partition on purpose: that is the edge issue #26 was missing.
|
||||||
|
export module Widget;
|
||||||
|
import std;
|
||||||
|
export import Base;
|
||||||
|
|
||||||
|
export namespace Demo {
|
||||||
|
class Thing {
|
||||||
|
public:
|
||||||
|
Payload payload;
|
||||||
|
std::int32_t tail = 0;
|
||||||
|
void Fill();
|
||||||
|
};
|
||||||
|
}
|
||||||
237
tests/TransitiveInterfaceChange/main.cpp
Normal file
237
tests/TransitiveInterfaceChange/main.cpp
Normal file
|
|
@ -0,0 +1,237 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
import std;
|
||||||
|
import Crafter.Build;
|
||||||
|
namespace fs = std::filesystem;
|
||||||
|
using namespace Crafter;
|
||||||
|
|
||||||
|
// Issue #26. A *primary* module interface unit — `export module Widget;`, no
|
||||||
|
// partitions — records nothing about what it imports. The scanner registered
|
||||||
|
// the Module and then dropped the file before the import pass ran, and Module
|
||||||
|
// itself had nowhere to put the edge, so:
|
||||||
|
//
|
||||||
|
// * `Module::Check` looked only at its own .cppm and its partitions. A layout
|
||||||
|
// change in an imported module left Widget.pcm and Widget.o untouched, the
|
||||||
|
// consumer's object untouched, and the executable relinked — mixing two
|
||||||
|
// layouts with no diagnostic. That is the reported failure: rebuild the
|
||||||
|
// dependency, relink the test, crash somewhere unrelated.
|
||||||
|
//
|
||||||
|
// * `Module::Compile` waited on nothing. Two modules in one Configuration
|
||||||
|
// compile on concurrent threads, so importing a sibling was a coin flip
|
||||||
|
// between working and "module 'Base' not found".
|
||||||
|
//
|
||||||
|
// Partitions never had either problem — they carry the same three vectors and
|
||||||
|
// Check/Compile honour them — which is why the gap only shows up on a module
|
||||||
|
// whose interface is one flat unit. tests/IncrementalInterfaceChange covers the
|
||||||
|
// neighbouring consumer-side edge (issue #27).
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
std::int32_t Failures = 0;
|
||||||
|
|
||||||
|
void Check(bool cond, std::string_view msg) {
|
||||||
|
if (!cond) {
|
||||||
|
std::println(std::cerr, "FAIL: {}", msg);
|
||||||
|
++Failures;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The fixture is mutated during the run, so work on a copy outside the repo.
|
||||||
|
fs::path StageFixture() {
|
||||||
|
fs::path source = fs::current_path() / "tests" / "TransitiveInterfaceChange" / "fixture";
|
||||||
|
fs::path staged = fs::temp_directory_path() / "crafter-build-transitive-interface-change";
|
||||||
|
fs::remove_all(staged);
|
||||||
|
fs::copy(source, staged, fs::copy_options::recursive);
|
||||||
|
return staged;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Swap in the Base variant carrying the extra member. From a file rather
|
||||||
|
// than a string literal: the module scanner reads raw source, so a module
|
||||||
|
// declaration spelled inside this test would make the test itself look like
|
||||||
|
// an implementation unit of Base.
|
||||||
|
void GrowBase(const fs::path& staged) {
|
||||||
|
fs::copy_file(staged / "base" / "Base-grown.cppm.in", staged / "base" / "Base.cppm", fs::copy_options::overwrite_existing);
|
||||||
|
// copy_file carries the source's mtime across, which would leave the
|
||||||
|
// rewritten interface looking older than the BMI built from it.
|
||||||
|
fs::last_write_time(staged / "base" / "Base.cppm", fs::file_time_type::clock::now());
|
||||||
|
}
|
||||||
|
|
||||||
|
std::unique_ptr<Configuration> MakeBaseLib(const fs::path& staged) {
|
||||||
|
auto base = std::make_unique<Configuration>();
|
||||||
|
base->path = staged / "base";
|
||||||
|
base->name = "base";
|
||||||
|
base->outputName = "base";
|
||||||
|
base->target = HostTarget();
|
||||||
|
base->type = ConfigurationType::LibraryStatic;
|
||||||
|
std::array<fs::path, 1> ifaces = { "Base" };
|
||||||
|
std::array<fs::path, 1> impls = { "Base" };
|
||||||
|
base->GetInterfacesAndImplementations(ifaces, impls);
|
||||||
|
return base;
|
||||||
|
}
|
||||||
|
|
||||||
|
std::unique_ptr<Configuration> MakeWidgetLib(const fs::path& staged) {
|
||||||
|
auto widget = std::make_unique<Configuration>();
|
||||||
|
widget->path = staged / "widget";
|
||||||
|
widget->name = "widget";
|
||||||
|
widget->outputName = "widget";
|
||||||
|
widget->target = HostTarget();
|
||||||
|
widget->type = ConfigurationType::LibraryStatic;
|
||||||
|
std::array<fs::path, 1> ifaces = { "Widget" };
|
||||||
|
std::array<fs::path, 1> impls = { "Widget" };
|
||||||
|
widget->GetInterfacesAndImplementations(ifaces, impls);
|
||||||
|
return widget;
|
||||||
|
}
|
||||||
|
|
||||||
|
Module* FindModule(const Configuration& cfg, std::string_view name) {
|
||||||
|
for (const std::unique_ptr<Module>& mod : cfg.interfaces) {
|
||||||
|
if (mod->name == name) return mod.get();
|
||||||
|
}
|
||||||
|
return nullptr;
|
||||||
|
}
|
||||||
|
|
||||||
|
bool BuildOk(Configuration& cfg, std::string_view label) {
|
||||||
|
// A fresh depResults per pass: the map memoizes each Configuration's
|
||||||
|
// build for the duration of one pass, so reusing it would skip the
|
||||||
|
// dependencies' second build entirely.
|
||||||
|
std::unordered_map<fs::path, std::shared_future<BuildResult>> depResults;
|
||||||
|
std::mutex depMutex;
|
||||||
|
BuildResult r = Build(cfg, depResults, depMutex);
|
||||||
|
if (!r.result.empty()) {
|
||||||
|
std::println(std::cerr, "FAIL: {} build failed: {}", label, r.result);
|
||||||
|
++Failures;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Non-zero exit here means the two ends of the link disagree about the
|
||||||
|
// layout — see the fixture's main.cpp.
|
||||||
|
void CheckAgrees(const fs::path& binary, std::string_view label) {
|
||||||
|
auto run = RunCommandWithTimeout(binary.string(), std::chrono::seconds(30));
|
||||||
|
Check(run.exitCode == 0 && !run.crashed && !run.timedOut, std::format("{}: consumer and library agree on the layout (exit={} output='{}')", label, run.exitCode, run.output));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
int main() {
|
||||||
|
{
|
||||||
|
// The scan records a primary interface unit's imports, and the
|
||||||
|
// pending/resolve machinery reaches them the same way it reaches a
|
||||||
|
// partition's.
|
||||||
|
fs::path staged = StageFixture();
|
||||||
|
std::unique_ptr<Configuration> base = MakeBaseLib(staged);
|
||||||
|
std::unique_ptr<Configuration> widget = MakeWidgetLib(staged);
|
||||||
|
|
||||||
|
Module* mod = FindModule(*widget, "Widget");
|
||||||
|
Check(mod != nullptr, "the primary interface unit registered a module named Widget");
|
||||||
|
if (mod != nullptr) {
|
||||||
|
Check(mod->partitions.empty(), "Widget has no partitions, so nothing else could carry the edge");
|
||||||
|
Check(mod->externalModuleDependencies.empty(), "nothing resolvable before dependencies are assigned");
|
||||||
|
Check(std::ranges::find(mod->pendingImports, "Base") != mod->pendingImports.end(), "the unresolved import of Base is remembered as pending");
|
||||||
|
Check(std::ranges::find(mod->pendingImports, "std") != mod->pendingImports.end(), "std stays pending — it is supplied from outside the graph");
|
||||||
|
|
||||||
|
widget->dependencies = { base.get() };
|
||||||
|
widget->ResolvePendingImports();
|
||||||
|
|
||||||
|
Check(mod->externalModuleDependencies.size() == 1, "ResolvePendingImports places the external module edge");
|
||||||
|
if (mod->externalModuleDependencies.size() == 1) {
|
||||||
|
Check(mod->externalModuleDependencies[0].first->name == "Base", "the edge names the Base module");
|
||||||
|
Check(mod->externalModuleDependencies[0].second == base->PcmDir() / "Base.pcm", "the edge points at the dependency's BMI");
|
||||||
|
}
|
||||||
|
Check(std::ranges::find(mod->pendingImports, "Base") == mod->pendingImports.end(), "the resolved name is no longer pending");
|
||||||
|
Check(std::ranges::find(mod->pendingImports, "std") != mod->pendingImports.end(), "std is still pending after the sweep");
|
||||||
|
|
||||||
|
// Build() runs a sweep unconditionally on top of whatever callers
|
||||||
|
// already did, so a repeat must not duplicate the edge.
|
||||||
|
widget->ResolvePendingImports();
|
||||||
|
Check(mod->externalModuleDependencies.size() == 1, "ResolvePendingImports is idempotent");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
{
|
||||||
|
// Two modules in one Configuration, declared in the order that used to
|
||||||
|
// hide the problem: Widget — which imports Base — is registered first.
|
||||||
|
// Every primary unit is registered before any import is resolved, so
|
||||||
|
// declaration order must not matter.
|
||||||
|
fs::path staged = StageFixture();
|
||||||
|
Configuration cfg;
|
||||||
|
cfg.path = staged;
|
||||||
|
cfg.name = "sibling";
|
||||||
|
cfg.outputName = "sibling";
|
||||||
|
cfg.target = HostTarget();
|
||||||
|
cfg.type = ConfigurationType::Executable;
|
||||||
|
std::array<fs::path, 2> ifaces = { "widget/Widget", "base/Base" };
|
||||||
|
std::array<fs::path, 3> impls = { "widget/Widget", "base/Base", "main" };
|
||||||
|
cfg.GetInterfacesAndImplementations(ifaces, impls);
|
||||||
|
|
||||||
|
Module* widgetMod = FindModule(cfg, "Widget");
|
||||||
|
Module* baseMod = FindModule(cfg, "Base");
|
||||||
|
Check(widgetMod != nullptr && baseMod != nullptr, "both sibling modules registered");
|
||||||
|
if (widgetMod != nullptr && baseMod != nullptr) {
|
||||||
|
Check(std::ranges::find(widgetMod->moduleDependencies, baseMod) != widgetMod->moduleDependencies.end(), "Widget's import of its sibling resolved locally despite being declared first");
|
||||||
|
Check(widgetMod->externalModuleDependencies.empty(), "a sibling in the same Configuration is not an external edge");
|
||||||
|
Check(baseMod->moduleDependencies.empty(), "Base imports no sibling");
|
||||||
|
}
|
||||||
|
|
||||||
|
// That edge is also the compile-ordering guarantee: Widget's precompile
|
||||||
|
// needs Base.pcm on disk. Repeated from scratch because the failure was
|
||||||
|
// a thread race, and one lucky pass proves nothing.
|
||||||
|
for (std::int32_t attempt = 0; attempt < 4; ++attempt) {
|
||||||
|
fs::remove_all(cfg.BuildDir());
|
||||||
|
fs::remove_all(cfg.BinDir());
|
||||||
|
if (!BuildOk(cfg, std::format("sibling pass {}", attempt))) break;
|
||||||
|
CheckAgrees(cfg.BinDir() / "sibling", std::format("sibling pass {}", attempt));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
{
|
||||||
|
// End to end, the reported shape: a consumer executable, a library whose
|
||||||
|
// primary interface unit imports a second library's module, and a member
|
||||||
|
// added to a class in that second module.
|
||||||
|
fs::path staged = StageFixture();
|
||||||
|
std::unique_ptr<Configuration> base = MakeBaseLib(staged);
|
||||||
|
std::unique_ptr<Configuration> widget = MakeWidgetLib(staged);
|
||||||
|
widget->dependencies = { base.get() };
|
||||||
|
|
||||||
|
Configuration app;
|
||||||
|
app.path = staged;
|
||||||
|
app.name = "consumer";
|
||||||
|
app.outputName = "consumer";
|
||||||
|
app.target = HostTarget();
|
||||||
|
app.type = ConfigurationType::Executable;
|
||||||
|
std::array<fs::path, 0> appIfaces = {};
|
||||||
|
std::array<fs::path, 1> appImpls = { "main" };
|
||||||
|
app.GetInterfacesAndImplementations(appIfaces, appImpls);
|
||||||
|
app.dependencies = { widget.get() };
|
||||||
|
|
||||||
|
fs::path binary = app.BinDir() / "consumer";
|
||||||
|
fs::path widgetInterfaceObject = widget->BuildDir() / "Widget.o";
|
||||||
|
fs::path widgetImplObject = widget->BuildDir() / "Widget_impl.o";
|
||||||
|
fs::path consumerObject = app.BuildDir() / "main_impl.o";
|
||||||
|
|
||||||
|
if (BuildOk(app, "first pass")) {
|
||||||
|
CheckAgrees(binary, "first pass");
|
||||||
|
|
||||||
|
fs::file_time_type interfaceBefore = fs::last_write_time(widgetInterfaceObject);
|
||||||
|
fs::file_time_type implBefore = fs::last_write_time(widgetImplObject);
|
||||||
|
fs::file_time_type consumerBefore = fs::last_write_time(consumerObject);
|
||||||
|
|
||||||
|
GrowBase(staged);
|
||||||
|
|
||||||
|
if (BuildOk(app, "second pass")) {
|
||||||
|
// All three sat on the old layout before the fix, and all three
|
||||||
|
// were left alone while base rebuilt and both binaries relinked.
|
||||||
|
Check(fs::last_write_time(widgetInterfaceObject) > interfaceBefore, "the importing interface unit is recompiled");
|
||||||
|
Check(fs::last_write_time(widgetImplObject) > implBefore, "that module's implementation unit is recompiled");
|
||||||
|
Check(fs::last_write_time(consumerObject) > consumerBefore, "the consumer's translation unit is recompiled");
|
||||||
|
|
||||||
|
CheckAgrees(binary, "second pass");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (Failures > 0) {
|
||||||
|
std::println(std::cerr, "{} assertions failed", Failures);
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue