diff --git a/src/apps/fastcache-cc/DirectManifest.hpp b/src/apps/fastcache-cc/DirectManifest.hpp index 3cd192d8..7b183cef 100644 --- a/src/apps/fastcache-cc/DirectManifest.hpp +++ b/src/apps/fastcache-cc/DirectManifest.hpp @@ -136,11 +136,22 @@ enum class DirectError : std::uint8_t /// Toolchain headers are dropped (the stamp covers them); project headers are /// canonicalized and hashed. Entries are deduplicated and sorted by canonical /// path, so the same build state yields byte-identical manifests on any machine. -/// @param includePaths Absolute include paths, as emitted by the compiler. +/// +/// `includePaths` must also contain the translation unit's own source path, not +/// only the files it `#include`s: neither `/showIncludes` nor a GNU depfile ever +/// names the primary TU (a depfile rule's target is explicitly excluded from its +/// own dependency list), so a manifest built from headers alone has no entry +/// that changes when the TU's own body is edited, and ValidateManifest would +/// keep validating a stale object forever (issue #49 / issue #51). The caller +/// (RecordManifest) adds it before calling this function; there is nothing +/// source-specific about the handling here — it is canonicalized, hashed, and +/// deduplicated exactly like any header. +/// @param includePaths Absolute paths the compile depends on: every `#include`d +/// header plus the translation unit's own source path. /// @param layout This build's roots. /// @param toolchainStamp Identity standing in for the toolchain headers. /// @param objectKey Key the compiled object is already stored under. -/// @return The manifest, or DirectError when a project header cannot be canonicalized. +/// @return The manifest, or DirectError when an entry cannot be canonicalized. [[nodiscard]] std::expected BuildManifest(std::vector const& includePaths, PathCanon::Layout const& layout, std::string_view toolchainStamp, diff --git a/src/apps/fastcache-cc/DirectManifest_test.cpp b/src/apps/fastcache-cc/DirectManifest_test.cpp index 664fc30b..be42f913 100644 --- a/src/apps/fastcache-cc/DirectManifest_test.cpp +++ b/src/apps/fastcache-cc/DirectManifest_test.cpp @@ -319,6 +319,54 @@ TEST_CASE("Build then validate a manifest against real files on disk") std::filesystem::remove_all(root); } +TEST_CASE("ValidateManifest catches an edit to the translation unit itself, MSVC-style") +{ + // /showIncludes (and therefore ParseIncludePaths) never names the primary + // translation unit -- only the headers it pulls in. RecordManifest (main.cpp) + // compensates by adding the TU's own source path to the list it hands to + // BuildManifest, alongside the headers /showIncludes reported. Without that, + // editing a .cpp's own body while leaving every header untouched is invisible + // to ValidateManifest, and a direct-mode hit replays a stale object forever + // (see issue #49 / issue #51). + auto const root = std::filesystem::temp_directory_path() / "fc-direct-source-edit"; + std::filesystem::remove_all(root); + std::filesystem::create_directories(root / "src"); + + auto const headerPath = root / "src" / "header.hpp"; + { + std::ofstream out { headerPath, std::ios::binary }; + out << "#pragma once\nint helper();\n"; + } + + auto const sourcePath = root / "src" / "a.cpp"; + { + std::ofstream out { sourcePath, std::ios::binary }; + out << "#include \"header.hpp\"\nint main() { return 0; }\n"; + } + + FastCache::PathCanon::Layout const layout { .sourceRoot = root.string(), .buildTree = (root / "out").string() }; + constexpr std::string_view stamp = "cl-test-1"; + + // MSVC's /showIncludes lists only the header; RecordManifest appends the + // source path itself before calling BuildManifest, which this mirrors. + auto built = BuildManifest({ headerPath.string(), sourcePath.string() }, layout, stamp, "objkey-1"); + REQUIRE(built.has_value()); + REQUIRE(built->entries.size() == 2); + CHECK(ValidateManifest(*built, layout, stamp)); + + // Edit the .cpp body itself -- no header touched. + { + std::ofstream out { sourcePath, std::ios::binary }; + out << "#include \"header.hpp\"\nint main() { return 1; }\n"; + } + + // The manifest must no longer validate: the TU itself is part of what a hit + // reproduces, so its own content has to be covered too. + CHECK_FALSE(ValidateManifest(*built, layout, stamp)); + + std::filesystem::remove_all(root); +} + TEST_CASE("BuildManifest normalizes '..' segments and mixed separators to one token") { // Real /showIncludes output echoes the resolved-but-unnormalized path, e.g. diff --git a/src/apps/fastcache-cc/main.cpp b/src/apps/fastcache-cc/main.cpp index ddd97fbe..22a08979 100644 --- a/src/apps/fastcache-cc/main.cpp +++ b/src/apps/fastcache-cc/main.cpp @@ -667,8 +667,16 @@ void RecordManifest(Config const& cfg, if (auto const depText = ReadDepFile(cmd)) includes = Cc::ParseDepFilePaths(*depText); - if (includes.empty()) - return; + // The TU itself must be part of what a direct hit revalidates, not only the + // headers it includes. /showIncludes never names the primary source (it only + // emits "Note: including file:" for #include targets), and a GNU depfile's + // rule deliberately excludes its own target from its dependency list — so + // without this, editing a .cpp's own body while leaving every header + // untouched is invisible to ValidateManifest and a stale object is replayed + // forever (issue #49 / issue #51). BuildManifest already canonicalizes, + // hashes, and dedupes every path handed to it, so adding the source here is + // enough; no format change is needed. + includes.push_back(cmd.source); // The manifest points at the object's ordinary key rather than causing a second // copy to be stored: L1 keeps values uncompressed, so duplicating objects would