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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
