[PATCH] D22513: [clang-tidy] add check cppcoreguidelines-special-member-functions

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.

================
Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:62
@@ +61,3 @@
+std::string
+SpecialMemberFunctionsCheck::join(llvm::ArrayRef<SpecialMemberFunctionKind> SMFS,
+                             llvm::StringRef AndOr) {
----------------
aaron.ballman wrote:
> I think this join function can maybe be replaced by a call to llvm::join() from StringExtras.h?
I want to join the last member function with "and" or "for", `llvm::join` won't let me do that.
Thanks for the suggestion though. I need to explore more of llvm/ADT.

================
Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h:42-44
@@ +41,5 @@
+
+  using ClassDefId = std::pair<SourceLocation, std::string>;
+
+  using ClassDefiningSpecialMembersMap = llvm::DenseMap<ClassDefId, llvm::SmallVector<SpecialMemberFunctionKind, 5>>;
+
----------------
aaron.ballman wrote:
> Do these need to be public?
I think so, I can't get the specialisation of DenseMapInfo to work otherwise.

================
Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h:62-63
@@ +61,4 @@
+/// Specialisation of DenseMapInfo to allow ClassDefId objects in DenseMaps
+/// FIXME: Move this to the corresponding cpp file as is done for
+/// clang-tidy/readability/IdentifierNamingCheck.cpp. 
+template <>
----------------
aaron.ballman wrote:
> Any reason why not to do this as part of this patch?
I've spent days trying. I can't get code to compile if it's moved and can't work out why. As the FIXME says, it works in IdentifierNaming.


https://reviews.llvm.org/D22513