[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

LGTM, but I'm not sure if isCopyableOrMovable will always work correctly, for a user defined special members it looks ok, but for some implicit ones I worry it may not always work. Probably things like "(hasSimpleCopyAssigment()) || (hasUserDeclaredCopyAssigment() && check here if its not deleted)" would be needed.
Thing is that CXXRecordDecl got most info (in confused way), there are things like DefaultedMoveConstructorIsDeleted that could be used to verify somehow base class.



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp:91-102
+  Finder->addMatcher(
+      fieldDecl(unless(isMemberOfLambda()),
+                hasDeclContext(cxxRecordDecl(isCopyableOrMovable())),
+                hasType(hasCanonicalType(referenceType())))
+          .bind("ref"),
+      this);
+  Finder->addMatcher(
----------------
Check first type, should be cheaper and consider mering those two. 


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst:6
 
-This check warns when structs or classes have const-qualified or reference
-(lvalue or rvalue) data members. Having such members is rarely useful, and
-makes the class only copy-constructible but not copy-assignable.
+This check warns when structs or classes that are copyable or movable have
+const-qualified or reference (lvalue or rvalue) data members. Having such
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155625