[clang] [Modules] No transitive source location change (PR #86912)

[Chuanqi Xu via cfe-commits]

================
@@ -2220,33 +2221,40 @@ class ASTReader
     return Sema::AlignPackInfo::getFromRawEncoding(Raw);
   }
 
+  using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
   /// Read a source location from raw form and return it in its
   /// originating module file's source location space.
-  SourceLocation ReadUntranslatedSourceLocation(SourceLocation::UIntTy Raw,
-                                                LocSeq *Seq = nullptr) const {
+  std::pair<SourceLocation, unsigned>
+  ReadUntranslatedSourceLocation(RawLocEncoding Raw,
+                                 LocSeq *Seq = nullptr) const {
     return SourceLocationEncoding::decode(Raw, Seq);
   }
 
   /// Read a source location from raw form.
-  SourceLocation ReadSourceLocation(ModuleFile &ModuleFile,
-                                    SourceLocation::UIntTy Raw,
-                                    LocSeq *Seq = nullptr) const {
-    SourceLocation Loc = ReadUntranslatedSourceLocation(Raw, Seq);
-    return TranslateSourceLocation(ModuleFile, Loc);
+  SourceLocation ReadSourceLocation(ModuleFile &MF, RawLocEncoding Raw,
+                                       LocSeq *Seq = nullptr) const {
+    if (!MF.ModuleOffsetMap.empty())
+      ReadModuleOffsetMap(MF);
+
+    auto [Loc, ModuleFileIndex] = ReadUntranslatedSourceLocation(Raw, Seq);
+    ModuleFile *OwningModuleFile =
+        ModuleFileIndex ? MF.DependentModules[ModuleFileIndex - 1] : &MF;
+    return TranslateSourceLocation(*OwningModuleFile, Loc);
   }
 
   /// Translate a source location from another module file's source
   /// location space into ours.
   SourceLocation TranslateSourceLocation(ModuleFile &ModuleFile,
                                          SourceLocation Loc) const {
-    if (!ModuleFile.ModuleOffsetMap.empty())
-      ReadModuleOffsetMap(ModuleFile);
-    assert(ModuleFile.SLocRemap.find(Loc.getOffset()) !=
-               ModuleFile.SLocRemap.end() &&
-           "Cannot find offset to remap.");
-    SourceLocation::IntTy Remap =
-        ModuleFile.SLocRemap.find(Loc.getOffset())->second;
-    return Loc.getLocWithOffset(Remap);
+    if (Loc.isInvalid())
+      return Loc;
+
+    // It implies that the Loc is already translated.
+    if (SourceMgr.isLoadedSourceLocation(Loc))
+      return Loc;
----------------
ChuanqiXu9 wrote:

> Now that TranslateSourceLocation() is only called from ReadSourceLocation()

Sadly, this is not true. `TranslateSourceLocation()` may be called in `ASTReader::ReadAST()`:

https://github.com/llvm/llvm-project/blob/aac4d03423dd6b7bdef0f2eb03c570f3e2ca6630/clang/lib/Serialization/ASTReader.cpp#L4588-L4591

The input value of  `TranslateSourceLocation()` there may come from a reading of untranslated source location in `ASTReader::ReadControlBlock` when reading imported modules. Or the input value may come from the argument of  `ASTReader::ReadAST()`, where must be a translated source location. 

Then it looks really dangerous to me. So I add the FIXME. We may not be able to change the signature of the argument of `TranslateSourceLocation()` to `UntranslatedSourceLocation` since that will require us to change the signature of `ASTReader::ReadAST()`.

The reason why actually it works, is that, in the case the input value comes from a translated source location (passed directly in `ASTReader::ReadAST()`), the value of `M.ImportedBy` may always be null **now**.  But I feel it is dangerous if someone changes it suddenly.

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