carlosgalvezp added a comment.

In D155625#4512123 <https://reviews.llvm.org/D155625#4512123>, @PiotrZSL wrote:

> 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.

Yeah I spent a lot of time trying to figure this out, somehow I expected this 
logic to already exist somewhere in Sema! Thanks for the pointers, I will 
experiment with those functions as well.



================
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(
----------------
PiotrZSL wrote:
> Check first type, should be cheaper and consider mering those two. 
Thanks for the tip! I'm not familiar with having multiple binds in the same 
`addMatcher` call. Do I still need to keep the `bind("ref")` at the end?


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

Reply via email to