ztamas added a comment. > I definitely want to see the diagnostic text changed away from "might be" -- > that's too noncommittal for my tastes. I'd also like to see the additional > test case (I suspect it will just work out of the box, but if it doesn't, it > would be good to know about it). The CERT alias is a bit more of a grey area > -- I'd like to insist on it being added because it's trivial, it improves the > behavior of this check by introducing an option some users will want, and > checks conforming to coding standards are always more compelling than one-off > checks. However, you seem very resistant to that and I don't want to add to > your work load if you are opposed to doing it, so I don't insist on the > change.
During LibreOffice development, I'm socialized to keep a patch as small as possible. Versioning history can be more "clean" with smaller patches. For example, it's easier to find and fix regressions with bisecting. Also smaller patches make the review process faster, etc. That's why I'm trying to separate what is absolutely necessary to include and what can be cut off from this patch. I changed the warning message and I added the requested test case. This test case is ignored by the check as expected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
