carlosgalvezp added a comment.

Using `hasSimpleCopyConstructor` and so on greatly simplifies the logic, great! 
Let me know if you are happy with it or I should go ahead and merge.



================
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:
> carlosgalvezp wrote:
> > 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?
> No, bind at the end need to be removed (I forgot about that).
Somehow I didn't manage to get it to work with your proposed change and 
removing `bind`, I feel I'm walking on thin ice here :) 


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