[clang] [clang][modules] Remove preloaded SLocEntries from PCM files (PR #66962)

[Jan Svoboda via cfe-commits]

================
@@ -458,14 +458,12 @@ SourceManager::AllocateLoadedSLocEntries(unsigned NumSLocEntries,
       CurrentLoadedOffset - TotalSize < NextLocalOffset) {
     return std::make_pair(0, 0);
   }
-
-  unsigned NewTableSize = LoadedSLocEntryTable.size() + NumSLocEntries;
-  LoadedSLocEntryTableSegments.push_back(NewTableSize);
-  LoadedSLocEntryTable.resize(NewTableSize);
-  SLocEntryLoaded.resize(NewTableSize);
-
+  LoadedSLocEntryTable.resize(LoadedSLocEntryTable.size() + NumSLocEntries);
+  SLocEntryLoaded.resize(LoadedSLocEntryTable.size());
   CurrentLoadedOffset -= TotalSize;
-  return std::make_pair(-NewTableSize - 1, CurrentLoadedOffset);
+  int ID = LoadedSLocEntryTable.size();
+  LoadedSLocEntryAllocBegin.push_back(FileID::get(-ID - 2));
----------------
jansvoboda11 wrote:

*Sigh*, I don't think my previous comment is right.

The `- 1` here is just to make space for the sentinel `FileID(-1)`.

The `- 1` in `ASTReader::TranslateFileID()` is to account for the fact that the module had it's own invalid `FileID(0)` which isn't necessary to serialize/deserialize, since it won't show up in valid AST or PP state. However, we serialize `FileID` values without accounting for the fact that `FileID(0)` gets dropped. `ASTReader` doesn't allocate space in `SourceManager` for `FileID(0)` and so we need to decrease all deserialized `FileID` values by one to fit into the allocation.

The "we rely on `ASTWriter` serializing the wrong thing" I got confused with the index for `SLocEntry` that should be preloaded, which used to be `SLocEntryOffsets.size()` **after** pushing the entry offset into the vector, so the `ASTReader` always needed to do do `- 1` when computing the index to jump to: `int Index = int(F.PreloadSLocEntries[I] - 1) + F.SLocEntryBaseID;`. That's actually removed by this patch, though.

So in short, `AllocateLoadedSLocEntries()` returns the correct base ID and doing `FileID(ID - 2)` is wrong. Fixed in the latest commit.

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