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())
----------------
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.
================
Comment at:
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:50
+void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(cxxDestructorDecl().bind("decl"), this);
+}
----------------
https://en.cppreference.com/w/cpp/language/destructor says defaulted destructor
is C++11 feature,
so not register if this isn't C++11?
================
Comment at:
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:77
+ << FirstDecl << FixItHint::CreateRemoval(FirstDeclRange)
+ << FixItHint::CreateRemoval(SecondDeclRange);
+ diag(MatchedDecl->getLocation(), "destructor definition is here",
----------------
This always drops out-of-line defaulting.
Is there/can there be any case where such defaultig should just be moved
in-line into the class?
================
Comment at:
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h:18-19
+
+/// A check that finds out-of-line declared defaulted destructors, which would
+/// otherwise be trivial:
+/// struct A: TrivialClass {
----------------
This doesn't read right, should be closer to this maybe
```
/// A check that finds classes that would be trivial if not for the defaulted
destructors declared out-of-line:
```
================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:136
+
+ Finds types that could be made trivially-destrictuble by removing out-of-line
+ defaulted destructor declarations.
----------------
s/destrictuble/destructible/?
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst:6
+
+Finds types that could be made trivially-destrictuble by removing out-of-line
+defaulted destructor declarations.
----------------
s/destrictuble/destructible/?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp:1
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+
----------------
Do you also want to check that fix-it applies and result passes sema?
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69435/new/
https://reviews.llvm.org/D69435
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
