Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

Note that getTimestampFilename just constructs a file name, so it's
mostly a "pure" function except possible heap allocation.

Note that getTimestampFilename just constructs a file name, so it's
mostly a "pure" function except possible heap allocation.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels May 24, 2025
@llvmbot
Copy link
Member

llvmbot commented May 24, 2025

@llvm/pr-subscribers-clang

Author: Kazu Hirata (kazutakahirata)

Changes

Note that getTimestampFilename just constructs a file name, so it's
mostly a "pure" function except possible heap allocation.


Full diff: https://github.com/llvm/llvm-project/pull/141358.diff

1 Files Affected:

  • (modified) clang/lib/Serialization/ModuleCache.cpp (-2)
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index 4ae49c4ec9a05..f42bdc16d815d 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -34,8 +34,6 @@ class CrossProcessModuleCache : public ModuleCache {
   }
 
   std::time_t getModuleTimestamp(StringRef ModuleFilename) override {
-    std::string TimestampFilename =
-        serialization::ModuleFile::getTimestampFilename(ModuleFilename);
     llvm::sys::fs::file_status Status;
     if (llvm::sys::fs::status(ModuleFilename, Status) != std::error_code{})
       return 0;

@llvmbot
Copy link
Member

llvmbot commented May 24, 2025

@llvm/pr-subscribers-clang-modules

Author: Kazu Hirata (kazutakahirata)

Changes

Note that getTimestampFilename just constructs a file name, so it's
mostly a "pure" function except possible heap allocation.


Full diff: https://github.com/llvm/llvm-project/pull/141358.diff

1 Files Affected:

  • (modified) clang/lib/Serialization/ModuleCache.cpp (-2)
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index 4ae49c4ec9a05..f42bdc16d815d 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -34,8 +34,6 @@ class CrossProcessModuleCache : public ModuleCache {
   }
 
   std::time_t getModuleTimestamp(StringRef ModuleFilename) override {
-    std::string TimestampFilename =
-        serialization::ModuleFile::getTimestampFilename(ModuleFilename);
     llvm::sys::fs::file_status Status;
     if (llvm::sys::fs::status(ModuleFilename, Status) != std::error_code{})
       return 0;

@kazutakahirata kazutakahirata merged commit 3ac2b5c into llvm:main May 24, 2025
14 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250524_unused_local_clang_Serialization branch May 24, 2025 16:39
jansvoboda11 added a commit to jansvoboda11/llvm-project that referenced this pull request Oct 11, 2025
The llvm#137363 PR was supposed to be NFC, but accidentally passed the wrong path to `sys::fs::status`. Then, llvm#141358 removed the correct path that was flagged by the compiler as unused. None of our existing regression tests caught this. We only find out due to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, add new remark to Clang for PCM input file validation, and uses it to create more reliable tests of the `-fmodules-validate-once-per-build-session` flag.
jansvoboda11 added a commit that referenced this pull request Oct 13, 2025
…2965)

#137363 was supposed to be NFC for the `CrossProcessModuleCache` (a.k.a
normal implicit module builds), but accidentally passed the wrong path
to `sys::fs::status`. Then, #141358 removed the correct path that
should've been passed instead. (The variable was flagged as unused.)
None of our existing tests caught this regression, we only found out due
to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, adds new remark to Clang
for PCM input file validation, and uses it to create more reliable tests
of the `-fmodules-validate-once-per-build-session` flag.
jansvoboda11 added a commit to jansvoboda11/llvm-project that referenced this pull request Oct 13, 2025
…m#162965)

llvm#137363 was supposed to be NFC for the `CrossProcessModuleCache` (a.k.a
normal implicit module builds), but accidentally passed the wrong path
to `sys::fs::status`. Then, llvm#141358 removed the correct path that
should've been passed instead. (The variable was flagged as unused.)
None of our existing tests caught this regression, we only found out due
to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, adds new remark to Clang
for PCM input file validation, and uses it to create more reliable tests
of the `-fmodules-validate-once-per-build-session` flag.

(cherry picked from commit ce8abef)
akadutta pushed a commit to akadutta/llvm-project that referenced this pull request Oct 14, 2025
…m#162965)

llvm#137363 was supposed to be NFC for the `CrossProcessModuleCache` (a.k.a
normal implicit module builds), but accidentally passed the wrong path
to `sys::fs::status`. Then, llvm#141358 removed the correct path that
should've been passed instead. (The variable was flagged as unused.)
None of our existing tests caught this regression, we only found out due
to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, adds new remark to Clang
for PCM input file validation, and uses it to create more reliable tests
of the `-fmodules-validate-once-per-build-session` flag.
c-rhodes pushed a commit to jansvoboda11/llvm-project that referenced this pull request Oct 16, 2025
…m#162965)

llvm#137363 was supposed to be NFC for the `CrossProcessModuleCache` (a.k.a
normal implicit module builds), but accidentally passed the wrong path
to `sys::fs::status`. Then, llvm#141358 removed the correct path that
should've been passed instead. (The variable was flagged as unused.)
None of our existing tests caught this regression, we only found out due
to a SourceKit-LSP benchmark getting slower.

This PR re-implements the original behavior, adds new remark to Clang
for PCM input file validation, and uses it to create more reliable tests
of the `-fmodules-validate-once-per-build-session` flag.

(cherry picked from commit ce8abef)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants