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

Tamás Zolnai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 12 11:32:51 PDT 2019


ztamas marked 13 inline comments as done.
ztamas added inline comments.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10
+
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP54-CPP. Gracefully handle self-copy assignment
----------------
aaron.ballman wrote:
> Eugene.Zelenko wrote:
> > alexfh wrote:
> > > JonasToth wrote:
> > > > It is worth an alias into the cert module. Please add this with this revision already.
> > > Please add a corresponding alias into the cert module.
> > See also other aliased checks for documentation examples.
> I kind of wonder whether we want it as an alias in the CERT module or just move it into the CERT module directly (and maybe provide an alias to bugprone, if we think the fp rate is reasonable enough).
Now, I just added a cert alias. I can move this check to cert module entirely if needed. In general, I prefer a meaningful name over a cert number, that's why it might be more useful to have also a bugprone prefixed check for this. And it seems to me cert checks used to be the aliases.


================
Comment at: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:351
+  int *p;
+};
----------------
JonasToth wrote:
> Please add tests with templated classes and self-assignment.
I tested with template classes, but TemplateCopyAndSwap test case, for example, was wrongly caught by the check. So I just changed the code to ignore template classes. I did not find any template class related catch either in LibreOffice or LLVM when I run the first version of this check, so we don't lose too much with ignoring template classes, I think.


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

https://reviews.llvm.org/D60507





More information about the cfe-commits mailing list