[clang] [clang] SourceManager: Cache offsets for LastFileIDLookup to speed up getFileID (PR #146782)

[Haojian Wu via cfe-commits]

================
@@ -832,13 +835,11 @@ FileID SourceManager::getFileIDLocal(SourceLocation::UIntTy SLocOffset) const {
   unsigned LessIndex = 0;
   // upper bound of the search range.
   unsigned GreaterIndex = LocalSLocEntryTable.size();
-  if (LastFileIDLookup.ID >= 0) {
-    // Use the LastFileIDLookup to prune the search space.
-    if (LocalSLocEntryTable[LastFileIDLookup.ID].getOffset() < SLocOffset)
-      LessIndex = LastFileIDLookup.ID;
-    else
-      GreaterIndex = LastFileIDLookup.ID;
-  }
+  // Use the LastFileIDLookup to prune the search space.
+  if (LastLookupStartOffset < SLocOffset)
+    LessIndex = LastFileIDLookup.ID;
----------------
hokein wrote:

`ID` cannot be negative here — `LastFileIDLookup` is only a cache for the **local** SLoc entry table, not the `Loaded` one.

When there is no cached entry yet, the state is:

* Both `LastLookupStartOffset` and `LastLookupEndOffset` are `0`.
* `LastFileIDLookup` is initialized to an invalid value (i.e., `LastFileIDLookup.ID` is `0`).

The only possible bug here would be if `SLocOffset` were `0`. In that case, `GreaterIndex` would be set to `0`, and the subsequent `--GreaterIndex;` would cause an out-of-bounds access on `LocalSLocEntryTable`.

However, this can't happen in practice: `SLocOffset` is guaranteed to be non-zero in this function, as ensured by the guard here:
[https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/SourceManager.cpp#L789-L790](https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/SourceManager.cpp#L789-L790)

I’ve also added some assertions to clarify this in the code.


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