[PATCH] D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation

[Evgeny Eltsin via Phabricator via cfe-commits]

eaeltsin added a comment.

Looks like our tests fail because ReadFileID doesn't translate file ID as ReadSourceLocation/TranslateSourceLocation do. Please see the prototype fix inline.



================
Comment at: clang/lib/Serialization/ASTReader.cpp:6343
              "Invalid data, missing pragma diagnostic states");
-      SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]);
-      auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc);
-      assert(IDAndOffset.first.isValid() && "invalid FileID for transition");
-      assert(IDAndOffset.second == 0 && "not a start location for a FileID");
+      FileID FID = ReadFileID(F, Record, Idx);
+      assert(FID.isValid() && "invalid FileID for transition");
----------------
This doesn't work as before, likely because ReadFileID doesn't do TranslateSourceLocation.

Our tests fail.

I tried calling TranslateSourceLocation here and the tests passed:
```
      SourceLocation Loc = Diag.SourceMgr->getComposedLoc(FID, 0);
      SourceLocation Loc2 = TranslateSourceLocation(F, Loc);
      auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc2);

      // Note that we don't need to set up Parent/ParentOffset here, because
      // we won't be changing the diagnostic state within imported FileIDs
      // (other than perhaps appending to the main source file, which has no
      // parent).
      auto &F = Diag.DiagStatesByLoc.Files[IDAndOffset.first];
```

Sorry I don't know the codebase, so this fix is definitely ugly :) But it shows the problem.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137213/new/

https://reviews.llvm.org/D137213