aaron.ballman added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:31 std::string OutputFile; + const Preprocessor &PP; ---------------- You don't really need the whole preprocessor, just the language options, right? If so, why not thread the language options through rather than the PP? ================ Comment at: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:150 + unsigned int TokenLen = 0) { + assert(!Loc.isInvalid()); + ---------------- Can you add some text to the assertion to explain what's gone wrong if the assert triggers? e.g. `assert(!Loc.isInvalid() && "invalid location when adjusting column position");` or something (same for the others asserts you've added)? ================ Comment at: clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c:34 +int unicode() +{ + int løçål = 0; ---------------- Formatting of the curly brace looks wrong. ================ 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}} +} ---------------- What file encoding is used by this file (and is there a BOM)? Have you tried this test on multiple platforms by any chance? 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