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