[PATCH] D13523: ASTMatchers: Keep AllCallbacks in a set instead of a vector

[Daniel Jasper via cfe-commits]

djasper created this revision.
djasper added a reviewer: klimek.
djasper added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

AllCallbacks is currently only used to call onStartOfTranslationUnit and onEndOfTranslationUnit on them. In this (and any other scenario I can come up with), it is important (or at least better) not to have duplicates in this container. E.g. currently onEndOfTranslationUnit is called repeatedly on the same callback for every matcher that is registered with it.

http://reviews.llvm.org/D13523

Files:
  include/clang/ASTMatchers/ASTMatchFinder.h
  lib/ASTMatchers/ASTMatchFinder.cpp

Index: lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -913,37 +913,37 @@
 void MatchFinder::addMatcher(const DeclarationMatcher &NodeMatch,
                              MatchCallback *Action) {
   Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeMatcher &NodeMatch,
                              MatchCallback *Action) {
   Matchers.Type.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const StatementMatcher &NodeMatch,
                              MatchCallback *Action) {
   Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierMatcher &NodeMatch,
                              MatchCallback *Action) {
   Matchers.NestedNameSpecifier.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierLocMatcher &NodeMatch,
                              MatchCallback *Action) {
   Matchers.NestedNameSpecifierLoc.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeLocMatcher &NodeMatch,
                              MatchCallback *Action) {
   Matchers.TypeLoc.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 bool MatchFinder::addDynamicMatcher(const internal::DynTypedMatcher &NodeMatch,
Index: include/clang/ASTMatchers/ASTMatchFinder.h
===================================================================
--- include/clang/ASTMatchers/ASTMatchFinder.h
+++ include/clang/ASTMatchers/ASTMatchFinder.h
@@ -42,6 +42,7 @@
 #define LLVM_CLANG_ASTMATCHERS_ASTMATCHFINDER_H
 
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Timer.h"
 
@@ -208,7 +209,7 @@
         NestedNameSpecifierLoc;
     std::vector<std::pair<TypeLocMatcher, MatchCallback *>> TypeLoc;
     /// \brief All the callbacks in one container to simplify iteration.
-    std::vector<MatchCallback *> AllCallbacks;
+    llvm::SmallPtrSet<MatchCallback *, 16> AllCallbacks;
   };
 
 private:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D13523.36770.patch
Type: text/x-patch
Size: 2666 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151007/5cca7b42/attachment.bin>