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".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131084/new/

https://reviews.llvm.org/D131084

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to