[clang] [llvm] [clang][Modules][Serialization] Avoid serializing duplicate source location entries for shared input files (PR #209795)

[Ayokunle Amodu via cfe-commits]

================
@@ -1858,6 +1851,70 @@ ASTReader::readSLocOffset(ModuleFile *F, unsigned Index) {
   }
 }
 
+void ASTReader::buildLoadedInputFiles() {
+  LoadedInputFiles.emplace();
+  // Modules are visited in index order, which is fixed before this runs, so the
+  // copy chosen for a file is the same on every write of this module.
+  for (ModuleFile &F : ModuleMgr) {
+    for (unsigned I = 0, N = F.InputFilesLoaded.size(); I != N; ++I) {
+      InputFileInfo FI = getInputFileInfo(F, I + 1);
+      if (FI.UnresolvedImportedFilename.empty())
+        continue;
+      // An overridden input holds a buffer rather than the file named by its
+      // path, so its path and size cannot identify matching contents.
+      if (FI.Overridden)
+        continue;
+      (*LoadedInputFiles)[FI.StoredSize].push_back({&F, I + 1});
----------------
ayokunle321 wrote:

Okay makes sense, I've followed the `EqualKey` method in HeaderFileInfo to account for mtime. I made it a secondary check within a size bucket. 

See [ceb0986](https://github.com/llvm/llvm-project/pull/209795/commits/ceb0986a8af45caa2448de863ba2a4af0fcdf8cc)

My problem with making a computed hash of size+mtime is that the HeaderFileInfo table is per-module so all files within that module have the same timestamp policy. 

Here, the map is across all loaded modules, so you could have a module that has timestamps off (stores 0 as mtime for all its files) and another one that has timestamps on. And so the same header in both modules could end up in different buckets because the hashes differ (but they are exactly the same). 

The way the commit above works, we only reject when both are stored and they differ. 

But let me know if you see need to be exactly consistent.





https://github.com/llvm/llvm-project/pull/209795