aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM! I think we've had sufficient time for other reviewers to lodge concerns and we can deal with any other issues post-commit. Do you need me to commit on your behalf? ================ Comment at: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:157 + const MemoryBuffer *Buf = SM.getBuffer(LocInfo.first); + assert(Buf && "got a NULL buffer for the location's file"); + assert(Buf->getBufferSize() >= (LocInfo.second + TokenLen) && ---------------- jranieri-grammatech wrote: > aaron.ballman wrote: > > I don't think this API is able to return a null buffer, but it can return > > an invalid buffer under legitimate circumstances, I believe. For instance, > > if the source location is in the scratch space used for things like macros > > defined on the command line with `-D`. I think you should pass the optional > > `invalid` parameter to the call to `getBuffer()` and then do no adjustments > > if the buffer is invalid. WDYT? > Good catch that this can't return NULL. > > I tested out defining things on the command line, but this code deals with > expansion locations and wasn't affected by it. Unless you can think of a way > to reproduce it, I think I'd rather pass in the `invalid` argument and assert > that it isn't invalid. > I think I'd rather pass in the invalid argument and assert that it isn't > invalid. That's fine by me -- I can't think of another way to reproduce it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70689/new/ https://reviews.llvm.org/D70689 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
