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