simon_tatham updated this revision to Diff 361957.
simon_tatham edited the summary of this revision.
simon_tatham added a comment.
In D105495#2882628 <https://reviews.llvm.org/D105495#2882628>, @tmatheson wrote:
> I also think it's the caller's responsibility to make sure what they pass in
> is correctly signed.
I don't disagree with that in principle, but in a case like this, where the
error is extremely easy to make, not warned about at compile time (so you have
to exercise the failing code path to spot it), and won't even show up at
//runtime// in the default build configuration (even once 64-bit
SourceLocations are actually implemented, the current plan is for them to be
off by default), I think it's worth at least //trying// to think of ways to
help the caller get it right.
Perhaps an alternative API might be to replace
`SourceLocation::getLocWithOffset` with an `operator+`, and add an `operator-`
to go with it? Then you could write `NewLoc = OldLoc + ThisInteger -
ThatInteger` and each integer would be implicitly widened if it was the wrong
size.
But in this version of the patch, I haven't done that; I've just reverted the
introduction of dyadic `getLocWithOffset` and added cumbersome casts or
intermediate variables at the call sites where I'd previously used it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105495/new/
https://reviews.llvm.org/D105495
Files:
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,8 @@
// Highlight the range. Make the span tag the outermost tag for the
// selected range.
- SourceLocation E =
- InstantiationEnd.getLocWithOffset(EndColNo - OldEndColNo);
+ SourceLocation E = InstantiationEnd.getLocWithOffset(
+ static_cast<SourceLocation::UIntTy>(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,8 @@
if (TokIndex < AsmToks.size()) {
const Token &Tok = AsmToks[TokIndex];
Loc = Tok.getLocation();
- Loc = Loc.getLocWithOffset(Offset - TokOffset);
+ Loc = Loc.getLocWithOffset(static_cast<SourceLocation::UIntTy>(Offset) -
+ TokOffset);
}
return Loc;
}
Index: clang/lib/Lex/Lexer.cpp
===================================================================
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -510,24 +510,29 @@
const SourceManager &SM,
const LangOptions &LangOpts) {
assert(Loc.isFileID());
- std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
- if (LocInfo.first.isInvalid())
+ FileID LocFileID;
+ SourceLocation::UIntTy LocOffset;
+ std::tie(LocFileID, LocOffset) = SM.getDecomposedLoc(Loc);
+ if (LocFileID.isInvalid())
return Loc;
bool Invalid = false;
- StringRef Buffer = SM.getBufferData(LocInfo.first, &Invalid);
+ StringRef Buffer = SM.getBufferData(LocFileID, &Invalid);
if (Invalid)
return Loc;
+ if (LocOffset > Buffer.size())
+ return Loc;
+
// Back up from the current location until we hit the beginning of a line
// (or the buffer). We'll relex from that point.
- const char *StrData = Buffer.data() + LocInfo.second;
- const char *LexStart = findBeginningOfLine(Buffer, LocInfo.second);
+ const char *StrData = Buffer.data() + LocOffset;
+ const char *LexStart = findBeginningOfLine(Buffer, LocOffset);
if (!LexStart || LexStart == StrData)
return Loc;
// Create a lexer starting at the beginning of this token.
- SourceLocation LexerStartLoc = Loc.getLocWithOffset(-LocInfo.second);
+ SourceLocation LexerStartLoc = Loc.getLocWithOffset(-LocOffset);
Lexer TheLexer(LexerStartLoc, LangOpts, Buffer.data(), LexStart,
Buffer.end());
TheLexer.SetCommentRetentionState(true);
@@ -565,12 +570,12 @@
SourceLocation FileLoc = SM.getSpellingLoc(Loc);
SourceLocation BeginFileLoc = getBeginningOfFileToken(FileLoc, SM, LangOpts);
- std::pair<FileID, unsigned> FileLocInfo = SM.getDecomposedLoc(FileLoc);
- std::pair<FileID, unsigned> BeginFileLocInfo =
- SM.getDecomposedLoc(BeginFileLoc);
- assert(FileLocInfo.first == BeginFileLocInfo.first &&
- FileLocInfo.second >= BeginFileLocInfo.second);
- return Loc.getLocWithOffset(BeginFileLocInfo.second - FileLocInfo.second);
+ FileID FID, BeginFID;
+ SourceLocation::UIntTy FileOffset, BeginFileOffset;
+ std::tie(FID, FileOffset) = SM.getDecomposedLoc(FileLoc);
+ std::tie(BeginFID, BeginFileOffset) = SM.getDecomposedLoc(BeginFileLoc);
+ assert(FID == BeginFID && FileOffset >= BeginFileOffset);
+ return Loc.getLocWithOffset(BeginFileOffset - FileOffset);
}
namespace {
Index: clang/lib/Format/FormatTokenLexer.cpp
===================================================================
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -832,7 +832,8 @@
FormatTok = new (Allocator.Allocate()) FormatToken;
readRawToken(*FormatTok);
SourceLocation WhitespaceStart =
- FormatTok->Tok.getLocation().getLocWithOffset(-TrailingWhitespace);
+ FormatTok->Tok.getLocation().getLocWithOffset(
+ -static_cast<SourceLocation::UIntTy>(TrailingWhitespace));
FormatTok->IsFirst = IsFirstToken;
IsFirstToken = false;
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -232,8 +232,10 @@
/// Return the start location of an included file or expanded macro.
SourceLocation getStartOfFileOrMacro(SourceLocation Loc) {
- if (Loc.isMacroID())
- return Loc.getLocWithOffset(-SM.getFileOffset(Loc));
+ if (Loc.isMacroID()) {
+ SourceLocation::UIntTy FileOffset = SM.getFileOffset(Loc);
+ return Loc.getLocWithOffset(-FileOffset);
+ }
return SM.getLocForStartOfFile(SM.getFileID(Loc));
}
Index: clang/lib/AST/SelectorLocationsKind.cpp
===================================================================
--- clang/lib/AST/SelectorLocationsKind.cpp
+++ clang/lib/AST/SelectorLocationsKind.cpp
@@ -27,7 +27,7 @@
if (EndLoc.isInvalid())
return SourceLocation();
IdentifierInfo *II = Sel.getIdentifierInfoForSlot(0);
- unsigned Len = II ? II->getLength() : 0;
+ SourceLocation::UIntTy Len = II ? II->getLength() : 0;
return EndLoc.getLocWithOffset(-Len);
}
@@ -35,7 +35,8 @@
if (ArgLoc.isInvalid())
return SourceLocation();
IdentifierInfo *II = Sel.getIdentifierInfoForSlot(Index);
- unsigned Len = /* selector id */ (II ? II->getLength() : 0) + /* ':' */ 1;
+ SourceLocation::UIntTy Len =
+ /* selector id */ (II ? II->getLength() : 0) + /* ':' */ 1;
if (WithArgSpace)
++Len;
return ArgLoc.getLocWithOffset(-Len);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
