[PATCH] D131632: [clang] Enable output of SARIF diagnostics
Abraham Corea Diaz via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 23 13:04:55 PDT 2022
abrahamcd added inline comments.
================
Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:116
+
+ Locations.push_back(
+ CharSourceRange{SourceRange{BeginLoc, EndLoc}, /* ITR = */ false});
----------------
I noticed that when processing additional source ranges, regular source locations are used instead of the presumed locations. Is this an intentional difference from single location diagnostics? (This issue is present in the current TextDiagnostics as well.)
================
Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:129-132
+ // FIXME: Enable the proper representation of PresumedLocs modified by a
+ // #line directive after relevant change is made in SarifDocumentWriter.
+ Locations.push_back(
+ CharSourceRange{SourceRange{DiagLoc, DiagLoc}, /* ITR = */ false});
----------------
There's an issue I encountered with adding presumed locations that have been modified by a #line directive to SARIF. To add a location, the Writer requires a CharSourceRange, which uses regular SourceLocations. If the presumed location has a modified line number, the source manager will still interpret it as a regular line number in the source file and can cause the wrong column number to be added to the SARIF object.
@vaibhav.y Do you have an idea of how involved it would be to add an option to the document writer that adds locations without first creating a CharSourceRange?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131632/new/
https://reviews.llvm.org/D131632
More information about the cfe-commits
mailing list