[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:22
+
+bool CheckPotentiallyTriviallyDestructible(const CXXDestructorDecl *Dtor) {
+  if (Dtor->isFirstDecl() || !Dtor->isExplicitlyDefaulted())
----------------
AntonBikineev wrote:
> lebedev.ri wrote:
> > I believe static functions are preferred to anon namespaces;
> > most (if not all) of this checking should/could be done in the matcher itself,
> > i.e. the check() could be called to unconditionally issue diag.
> I couldn't find matchers to check triviallity of special functions or something like isFirstDecl(). Perhaps, would make sense to add some.
You can trivially add your own matchers, see e.g.
https://github.com/llvm/llvm-project/blob/058b628264d276515a0aab66285816fe6cde2aa2/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp#L15-L39


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp:1
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+
----------------
AntonBikineev wrote:
> lebedev.ri wrote:
> > Do you also want to check that fix-it applies and result passes sema?
> Aren't fix-its already checked by CHECK-FIXES?
> It would be great to check if the result is compilable. Should I run another clang_tidy instance to check that or is there another way to do so?
It `FileCheck`s the `// CHECK-FIXES` lines, but i don't think it actually applies them.
https://github.com/llvm/llvm-project/blob/885c559369fe3d6323898c17787bd0454065fc34/clang-tools-extra/test/clang-tidy/checkers/cert-uppercase-literal-suffix-integer.cpp#L2-L4


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

https://reviews.llvm.org/D69435