[PATCH] D80490: [clang-tidy] Check for rule of five and zero.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

It would take more complex matching and diagnostics logic, but it would be so much nicer if the diagnostics were grouped. So instead of having a diagnostic about class `X` defining a copy construct and another about it defining a copy assignment operator, It could just say `X` defines a copy constructor and copy assignment operator without all 5 special members defined. The `optionally` matcher could be handy in implementing this.



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp:22
+    : ClangTidyCheck(Name, Context),
+      StrictCheck(Options.get("StrictCheck", false)) {}
+
----------------
You need to implement the `storeOptions` method to store this.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp:25-26
+void RuleOfFiveAndZeroCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
----------------
move this to the `isLanguageVersionSupported` virtual function


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp:81
+  if (StrictCheck) {
+    Finder->addMatcher(cxxRecordDecl(allOf(DefinesDestructor,
+                                           unless(DefinesAllSpecialMembers)))
----------------
You don't need to specify `allOf` here, top level matchers are implicitly `allOf` matchers. Same goes for all cases below.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero-strict.cpp:18-22
+  DeletesEverything(const DeletesEverything &);
+  DeletesEverything &operator=(const DeletesEverything &);
+  DeletesEverything(DeletesEverything &&);
+  DeletesEverything &operator=(DeletesEverything &&);
+  ~DeletesEverything();
----------------
Probably doesn't affect the tests, but shouldn't these all be ` = delete;`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80490