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

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 28 18:33:41 PDT 2023


dexonsmith added a comment.

Now that the behaviour change is understood, maybe it'd be useful to split the patch in two:

- First, this patch, plus a call to `ASTReader::getInputFile()` only for its side effects, to make this patch actually NFC.
- Second, committed a few days later, a patch that removes the call and adds a test confirming `-I` is no longer implied by `-fembed-all-input-files`.

That way, the future temporary reverts won't churn the file format. For example, downstreams that hit problems and need to temporarily revert can just revert the second patch.

(... assuming that @rsmith/others confirm that implying `-I` is undesirable... or, if it's desired, surely there's a more explicit way to implement it.)



================
Comment at: clang/lib/Serialization/ASTWriter.cpp:3013
 
-    SourceLocation Loc = Diag.SourceMgr->getComposedLoc(FileIDAndFile.first, 0);
-    assert(!Loc.isInvalid() && "start loc for valid FileID is invalid");
-    AddSourceLocation(Loc, Record);
+    AddFileID(FileIDAndFile.first, Record);
 
----------------
This is where I'm suggesting a side-effects-only call to `ASTReader::getInputFile()` could be added. (Or to something that transitively will call it.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137213



More information about the cfe-commits mailing list