simon_tatham created this revision.
simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi.
Herald added subscribers: dexonsmith, martong.
simon_tatham requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is part of a patch series working towards the ability to make
SourceLocation into a 64-bit type to handle larger translation units.

In many places in the code, SourceLocation::getLocWithOffset is used
with an argument of type `unsigned`, holding a value that's
conceptually negative (to find a location shortly before the current
one). If SourceLocation becomes a 64-bit type, this won't work any
more, because the negation will happen before widening to 64 bits, so
that instead of an offset of (say) -3, you'll get 2^32-3.

One answer would be to add casts to SourceLocation::IntType at all the
call sites, but that's quite cumbersome. In this patch I've taken a
different approach: getLocWithOffset now takes an optional second
parameter, which is subtracted from the location in addition to adding
the first parameter. That happens after widening to SourceLocation's
native integer type, so it will avoid this problem.

So you can compute an offset of -N for known-positive N by saying
`Loc.getLocWithOffset(0, N)`. And if you know the file offsets of two
features A,B and want to translate a SourceLocation for A into one for
B, you can say `Loc.getLocWithOffset(B, A)` to add (B-A) to the
position, without having to worry about integer types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105495

Files:
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/AST/SelectorLocationsKind.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -1081,8 +1081,7 @@
   // Highlight the range.  Make the span tag the outermost tag for the
   // selected range.
 
-  SourceLocation E =
-    InstantiationEnd.getLocWithOffset(EndColNo - OldEndColNo);
+  SourceLocation E = InstantiationEnd.getLocWithOffset(EndColNo, OldEndColNo);
 
   html::HighlightRange(R, InstantiationStart, E, HighlightStart, HighlightEnd);
 }
Index: clang/lib/Parse/ParseStmtAsm.cpp
===================================================================
--- clang/lib/Parse/ParseStmtAsm.cpp
+++ clang/lib/Parse/ParseStmtAsm.cpp
@@ -185,7 +185,7 @@
   if (TokIndex < AsmToks.size()) {
     const Token &Tok = AsmToks[TokIndex];
     Loc = Tok.getLocation();
-    Loc = Loc.getLocWithOffset(Offset - TokOffset);
+    Loc = Loc.getLocWithOffset(Offset, TokOffset);
   }
   return Loc;
 }
Index: clang/lib/Lex/Lexer.cpp
===================================================================
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -527,7 +527,7 @@
     return Loc;
 
   // Create a lexer starting at the beginning of this token.
-  SourceLocation LexerStartLoc = Loc.getLocWithOffset(-LocInfo.second);
+  SourceLocation LexerStartLoc = Loc.getLocWithOffset(0, LocInfo.second);
   Lexer TheLexer(LexerStartLoc, LangOpts, Buffer.data(), LexStart,
                  Buffer.end());
   TheLexer.SetCommentRetentionState(true);
@@ -570,7 +570,7 @@
       SM.getDecomposedLoc(BeginFileLoc);
   assert(FileLocInfo.first == BeginFileLocInfo.first &&
          FileLocInfo.second >= BeginFileLocInfo.second);
-  return Loc.getLocWithOffset(BeginFileLocInfo.second - FileLocInfo.second);
+  return Loc.getLocWithOffset(BeginFileLocInfo.second, FileLocInfo.second);
 }
 
 namespace {
Index: clang/lib/Format/FormatTokenLexer.cpp
===================================================================
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -832,7 +832,7 @@
   FormatTok = new (Allocator.Allocate()) FormatToken;
   readRawToken(*FormatTok);
   SourceLocation WhitespaceStart =
-      FormatTok->Tok.getLocation().getLocWithOffset(-TrailingWhitespace);
+      FormatTok->Tok.getLocation().getLocWithOffset(0, TrailingWhitespace);
   FormatTok->IsFirst = IsFirstToken;
   IsFirstToken = false;
 
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -233,7 +233,7 @@
   /// Return the start location of an included file or expanded macro.
   SourceLocation getStartOfFileOrMacro(SourceLocation Loc) {
     if (Loc.isMacroID())
-      return Loc.getLocWithOffset(-SM.getFileOffset(Loc));
+      return Loc.getLocWithOffset(0, SM.getFileOffset(Loc));
     return SM.getLocForStartOfFile(SM.getFileID(Loc));
   }
 
Index: clang/lib/AST/SelectorLocationsKind.cpp
===================================================================
--- clang/lib/AST/SelectorLocationsKind.cpp
+++ clang/lib/AST/SelectorLocationsKind.cpp
@@ -28,7 +28,7 @@
       return SourceLocation();
     IdentifierInfo *II = Sel.getIdentifierInfoForSlot(0);
     unsigned Len = II ? II->getLength() : 0;
-    return EndLoc.getLocWithOffset(-Len);
+    return EndLoc.getLocWithOffset(0, Len);
   }
 
   assert(Index < NumSelArgs);
@@ -38,7 +38,7 @@
   unsigned Len = /* selector id */ (II ? II->getLength() : 0) + /* ':' */ 1;
   if (WithArgSpace)
     ++Len;
-  return ArgLoc.getLocWithOffset(-Len);
+  return ArgLoc.getLocWithOffset(0, Len);
 }
 
 namespace {
Index: clang/include/clang/Basic/SourceLocation.h
===================================================================
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -132,9 +132,18 @@
   }
 
 public:
-  /// Return a source location with the specified offset from this
+  /// Return a source location with a specified offset from this
   /// SourceLocation.
-  SourceLocation getLocWithOffset(IntType Offset) const {
+  ///
+  /// If the second parameter NegOffset is provided, then the combined offset
+  /// is computed as Offset - NegOffset. This is convenient if the desired
+  /// offset is held in a 32-bit integer type such as `unsigned`, but
+  /// SourceLocations are 64 bits: if you compute a negative offset in an
+  /// unsigned without widening it first, it becomes positive again. Allowing
+  /// the subtraction to be done inside this function avoids a cumbersome cast
+  /// at every call site.
+  SourceLocation getLocWithOffset(IntType Offset, IntType NegOffset = 0) const {
+    Offset -= NegOffset;
     assert(((getOffset()+Offset) & MacroIDBit) == 0 && "offset overflow");
     SourceLocation L;
     L.ID = ID+Offset;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to