[clang-tidy] Don't use a SmallSetVector of an enum.

[jlebar]

Summary

This doesn't work after converting SmallSetVector to use DenseSet.

Instead we can just use a SmallVector.

[jlebar]

Hi, friendly ping? This trivial patch is the only blocker remaining before I can land D25648, which is the first part of my Grand Set Refactoring (see mail to llvm-dev about a week ago).

[aaron.ballman]

LGTM with two minor nits.

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

102 ↗

(On Diff #74780)

Please don't use auto since the type is not spelled out in the initializer.

103 ↗

(On Diff #74780)

Please drop the push_back() onto its own line.

[jlebar]

Thank you very much for the review!

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

103 ↗

(On Diff #74780)

This was actually how clang-tidy formatted the line. My understanding is that we go with that in llvm projects? Unless you're saying I have an outdated clang-tidy, in which case I will investigate that.

[bkramer]

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

103 ↗

(On Diff #74780)

It's what happens when your clang-format is using Google style. Some versions of clang-format ship with weird defaults, so you have to add -style=LLVM manually.

[jlebar]

This comment has been deleted.

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

103 ↗

(On Diff #74780)

Er, s/clang-tidy/clang-format/

[jlebar]

Adjust formatting, write out type of 'auto'.

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

103 ↗

(On Diff #74780)

Oh, this is happening because clang-tools-extra does not have a .clang-format file in tree. It's relying on us picking up clang's .clang-format file, but if you use the monorepo [0] (or if you have a tool that's smart and stops at git repository boundaries) you won't find it.

Mystery solved. I can check those files in if you want, or not if you want. :)

[0] https://github.com/llvm-project/llvm-project

[jlebar]

Thank you for the reviews, everyone!

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h

 class SpecialMemberFunctionsCheck : public ClangTidyCheck {
 public:
   SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void onEndOfTranslationUnit() override;
 
-  enum class SpecialMemberFunctionKind {
+  enum class SpecialMemberFunctionKind : uint8_t {
     Destructor,
     CopyConstructor,
     CopyAssignment,
     MoveConstructor,
     MoveAssignment
   };
 
   using ClassDefId = std::pair<SourceLocation, std::string>;
 
   using ClassDefiningSpecialMembersMap =
       llvm::DenseMap<ClassDefId,
-                     llvm::SmallSetVector<SpecialMemberFunctionKind, 5>>;
+                     llvm::SmallVector<SpecialMemberFunctionKind, 5>>;
 
 private:
   ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
 };
 
 } // namespace cppcoreguidelines
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

void SpecialMemberFunctionsCheck::check(
   std::initializer_list<std::pair<std::string, SpecialMemberFunctionKind>>
       Matchers = {{"dtor", SpecialMemberFunctionKind::Destructor},
                   {"copy-ctor", SpecialMemberFunctionKind::CopyConstructor},
                   {"copy-assign", SpecialMemberFunctionKind::CopyAssignment},
                   {"move-ctor", SpecialMemberFunctionKind::MoveConstructor},
                   {"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
 
   for (const auto &KV : Matchers)
-    if (Result.Nodes.getNodeAs<CXXMethodDecl>(KV.first))
-      ClassWithSpecialMembers[ID].insert(KV.second);
+    if (Result.Nodes.getNodeAs<CXXMethodDecl>(KV.first)) {
+      SpecialMemberFunctionKind Kind = KV.second;
+      llvm::SmallVectorImpl<SpecialMemberFunctionKind> &Members =
+          ClassWithSpecialMembers[ID];
+      if (find(Members, Kind) == Members.end())
+        Members.push_back(Kind);
+    }
 }
 
 void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
   llvm::SmallVector<SpecialMemberFunctionKind, 5> AllSpecialMembers = {
       SpecialMemberFunctionKind::Destructor,
       SpecialMemberFunctionKind::CopyConstructor,
       SpecialMemberFunctionKind::CopyAssignment};
 
for (const auto &C : ClassWithSpecialMembers) {
 
     llvm::SmallVector<SpecialMemberFunctionKind, 5> UndefinedSpecialMembers;
     std::set_difference(AllSpecialMembers.begin(), AllSpecialMembers.end(),
                         DefinedSpecialMembers.begin(),
                         DefinedSpecialMembers.end(),
                         std::back_inserter(UndefinedSpecialMembers));
 
     diag(C.first.first, "class '%0' defines %1 but does not define %2")
-        << C.first.second << join(DefinedSpecialMembers.getArrayRef(), " and ")
+        << C.first.second << join(DefinedSpecialMembers, " and ")
         << join(UndefinedSpecialMembers, " or ");
   }
 }
 } // namespace cppcoreguidelines
 } // namespace tidy
 } // namespace clang