aaron.ballman added inline comments.
================
Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:62
@@ +61,3 @@
+std::string
+SpecialMemberFunctionsCheck::join(llvm::ArrayRef<SpecialMemberFunctionKind>
SMFS,
+ llvm::StringRef AndOr) {
----------------
I think this join function can maybe be replaced by a call to llvm::join() from
StringExtras.h?
================
Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:64
@@ +63,3 @@
+ llvm::StringRef AndOr) {
+ assert(!SMFS.empty());
+ std::string Buffer;
----------------
It's good to put an && along with some explanatory text for debugging if this
assertion ever triggers.
================
Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:73
@@ +72,3 @@
+ }
+ if (LastIndex != 0) {
+ Stream << AndOr << toString(SMFS[LastIndex]);
----------------
Elide braces per usual style conventions.
================
Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:94
@@ +93,3 @@
+
+ for (const auto& KV : Matchers)
+ if (Result.Nodes.getNodeAs<CXXMethodDecl>(KV.first))
----------------
The & should bind to KV rather than auto (clang-format should handle this for
you).
================
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>>;
+
----------------
Do these need to be public?
================
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 <>
----------------
Any reason why not to do this as part of this patch?
https://reviews.llvm.org/D22513
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
