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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D137213: [... Jan Svoboda via Phabricator via cfe-commits
    • [PATCH] D1372... Duncan P. N. Exon Smith via Phabricator via cfe-commits

Reply via email to