cjdb added a comment. It seems I got confused and conflated the two this morning.
In D131084#3697525 <https://reviews.llvm.org/D131084#3697525>, @vaibhav.y wrote: >> Hmm, we can probably use "informational" for notes, warnings, and remarks, >> but I'm kinda partial to proposing the latter two upstream. > > Interesting, I agree that remarks could fit under "informational". > > As for notes: As I understand it they are always child diagnostics of > warnings/errors, so seems it would be okay to associate these with the "fail" > kind. I don't think classifying warnings as "fail" is right unless `-Werror` is being used. For notes, I think there may be two options, if we don't want to use informational: - inherit from the parent diagnostic - use notApplicable, since they don't have any influence on their own In D131084#3697308 <https://reviews.llvm.org/D131084#3697308>, @denik wrote: > 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". If skipping the kind defaults to fail, then we shouldn't skip it, because getting a warning or remark doesn't necessarily mean that the build has failed. 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
