[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

Tamás Zolnai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 9 04:36:16 PDT 2019


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





More information about the cfe-commits mailing list