aaron.ballman added reviewers: dblaikie, echristo, xbolva00.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
Mostly LG aside from a question about an assertion. Adding other reviewers
outside of GrammaTech in case there are other concerns we've missed.
================
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) &&
----------------
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?
================
Comment at: clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c:35-36
+{
+ int løçål = 0;
+ /* ☃ */ return 1 / løçål; // expected-warning {{Division by zero}}
+}
----------------
jranieri-grammatech wrote:
> aaron.ballman wrote:
> > What file encoding is used by this file (and is there a BOM)? Have you
> > tried this test on multiple platforms by any chance?
> The file is encoded in UTF-8 without a BOM. I have not tried the test on
> other platforms.
Okay, I think that's reasonable, thanks!
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
