[PATCH] D131084: Add support for specifying the severity of a SARIF Result.
Denis Nikitin via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 3 12:22:08 PDT 2022
denik added a comment.
In D131084#3697256 <https://reviews.llvm.org/D131084#3697256>, @cjdb wrote:
> In D131084#3697211 <https://reviews.llvm.org/D131084#3697211>, @vaibhav.y wrote:
>> Submitting for review:
>> Some notes:
>> There are a couple of ways I think we can acheive this, per the spec:
>> 1. The reportingDescriptor (rule) objects can be given a default configuration property <https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317850>, which can set the default warning level and other data such as rule parameters etc.
>> 2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.
>> The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?
>> There is also the question about the "kind" of results in clang. From my reading of the spec <https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317647>, it seems that "fail" is the only case that applies to us because:
>> - "pass": Implies no issue was found.
>> - "open": This value is used by proof-based tools. It could also mean additional assertions required
>> - "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
>> - "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.
>> Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's `bugprone-*` suite?
>> Let me know what you think is a good way to approach this wrt clang's diagnostics system.
> Hmm, we can probably use "informational" for notes, warnings, and remarks, but I'm kinda partial to proposing the latter two upstream.
I think we can skip `kind` in the clang diagnostic. I found this:
> If kind is absent, it SHALL default to "fail".
> If level has any value other than "none" and kind is present, then kind SHALL have the value "fail".
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits