tmatheson added a comment. I'm not a huge fan of this as an API; it is not obvious what the function and parameters do without reading the comment or implementation (e.g. whether `Offset` is ignored if `NegOffset` is given, and what it means to pass offsets to both). It moves the subtraction logic from the call site, where it is meaningful, to inside the function where it is not. I also think it's the caller's responsibility to make sure what they pass in is correctly signed. I've put suggestions for how each call site could be updated without casting while keeping `getLocWithOffset` unchanged.
================ Comment at: clang/lib/AST/SelectorLocationsKind.cpp:41 ++Len; - return ArgLoc.getLocWithOffset(-Len); } ---------------- These are probably fine as they were ================ 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)); ---------------- Seems like getFileOffset should now return ui64. ================ Comment at: clang/lib/Format/FormatTokenLexer.cpp:835 SourceLocation WhitespaceStart = - FormatTok->Tok.getLocation().getLocWithOffset(-TrailingWhitespace); + FormatTok->Tok.getLocation().getLocWithOffset(0, TrailingWhitespace); FormatTok->IsFirst = IsFirstToken; ---------------- This probably would need a cast; widening `TrailingWhitespace` seems overkill. ================ Comment at: clang/lib/Lex/Lexer.cpp:530 // 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, ---------------- ditto `getDecomposedLoc` ================ Comment at: clang/lib/Lex/Lexer.cpp:568-570 std::pair<FileID, unsigned> FileLocInfo = SM.getDecomposedLoc(FileLoc); std::pair<FileID, unsigned> BeginFileLocInfo = SM.getDecomposedLoc(BeginFileLoc); ---------------- Shouldn't the return type of `getDecomposedLoc` need updating at some stage? If so that would take care of this case ================ Comment at: clang/lib/Parse/ParseStmtAsm.cpp:174-179 unsigned Offset = SMLoc.getPointer() - LBuf->getBufferStart(); // Figure out which token that offset points into. const unsigned *TokOffsetPtr = llvm::lower_bound(AsmTokOffsets, Offset); unsigned TokIndex = TokOffsetPtr - AsmTokOffsets.begin(); unsigned TokOffset = *TokOffsetPtr; ---------------- ================ Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:1073-1074 // Compute the column number of the end. unsigned EndColNo = SM.getExpansionColumnNumber(InstantiationEnd); unsigned OldEndColNo = EndColNo; ---------------- Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105495/new/ https://reviews.llvm.org/D105495 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits