[PATCH] D105495: [clang] Make negative getLocWithOffset widening-safe.

Simon Tatham via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 27 02:50:59 PDT 2021


simon_tatham added inline comments.


================
Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:236
     if (Loc.isMacroID())
-      return Loc.getLocWithOffset(-SM.getFileOffset(Loc));
+      return Loc.getLocWithOffset(0, SM.getFileOffset(Loc));
     return SM.getLocForStartOfFile(SM.getFileID(Loc));
----------------
tmatheson wrote:
> Seems like getFileOffset should now return ui64.
That seems sensible from first principles, but I tried tugging on that thread and it turned out to have a //lot// of extra work behind it, because file offsets are absolutely all over the place (perhaps even more so that SourceLocations proper), and in particular, they're often used as indexes into in-memory buffers, which gets awkward if you want 64-bit SourceLocations and 64-bit builds of clang to be independent choices.

Perhaps you're right that expanding file offsets to 64-bit would also be a useful piece of work. But doing it as part of //this// work would double the size of the job, so I think it would make more sense to keep it separate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105495



More information about the cfe-commits mailing list