[AST] Convert Marshallers to use unique_ptr.

[timshen]

clang/lib/ASTMatchers/Dynamic/Marshallers.h

514

Notice that this is 2x slower, because move on unique_ptr resets the move-from side. But it is less likely to have an issue.

clang/lib/ASTMatchers/Dynamic/Registry.cpp

501

const MatcherDescriptor &Matcher = *M.getValue()?

[jlebar]

clang/lib/ASTMatchers/Dynamic/Marshallers.h

514

It may be even worse than that because we used to be able to do memcpy, but now, unless llvm is incredibly smart, we're forced to copy one element at a time.

I think these lists are at most two elements, though.

clang/lib/ASTMatchers/Dynamic/Marshallers.h

 #undef CHECK_ARG_TYPE
 
 /// \brief Helper class used to collect all the possible overloads of an
 ///   argument adaptative matcher function.
 template <template <typename ToArg, typename FromArg> class ArgumentAdapterT,
           typename FromTypes, typename ToTypes>
 class AdaptativeOverloadCollector {
 public:
-  AdaptativeOverloadCollector(StringRef Name,
-                              std::vector<MatcherDescriptor *> &Out)
+  AdaptativeOverloadCollector(
+      StringRef Name, std::vector<std::unique_ptr<MatcherDescriptor>> &Out)
       : Name(Name), Out(Out) {
     collect(FromTypes());
   }
 
 private:
   typedef ast_matchers::internal::ArgumentAdaptingMatcherFunc<
       ArgumentAdapterT, FromTypes, ToTypes> AdaptativeFunc;
 
   /// \brief End case for the recursion
   static void collect(ast_matchers::internal::EmptyTypeList) {}
 
   /// \brief Recursive case. Get the overload for the head of the list, and
   ///   recurse to the tail.
   template <typename FromTypeList>
   inline void collect(FromTypeList);
 
   StringRef Name;
-  std::vector<MatcherDescriptor *> &Out;
+  std::vector<std::unique_ptr<MatcherDescriptor>> &Out;
 };
 
 /// \brief MatcherDescriptor that wraps multiple "overloads" of the same
 ///   matcher.
 ///
 /// It will try every overload and generate appropriate errors for when none or
 /// more than one overloads match the arguments.
 class OverloadedMatcherDescriptor : public MatcherDescriptor {
 public:
-  OverloadedMatcherDescriptor(ArrayRef<MatcherDescriptor *> Callbacks)
-      : Overloads(Callbacks.begin(), Callbacks.end()) {}
+  OverloadedMatcherDescriptor(
+      MutableArrayRef<std::unique_ptr<MatcherDescriptor>> Callbacks)
+      : Overloads(std::make_move_iterator(Callbacks.begin()),
+                  std::make_move_iterator(Callbacks.end())) {}
 
   ~OverloadedMatcherDescriptor() override {}
 
   VariantMatcher create(SourceRange NameRange,
                         ArrayRef<ParserValue> Args,
                         Diagnostics *Error) const override {
     std::vector<VariantMatcher> Constructed;
     Diagnostics::OverloadContext Ctx(Error);
private:
   const StringRef MatcherName;
 };
 
 /// Helper functions to select the appropriate marshaller functions.
 /// They detect the number of arguments, arguments types and return type.
 
 /// \brief 0-arg overload
 template <typename ReturnType>
-MatcherDescriptor *makeMatcherAutoMarshall(ReturnType (*Func)(),
-                                     StringRef MatcherName) {
+std::unique_ptr<MatcherDescriptor>
+makeMatcherAutoMarshall(ReturnType (*Func)(), StringRef MatcherName) {
   std::vector<ast_type_traits::ASTNodeKind> RetTypes;
   BuildReturnTypeVector<ReturnType>::build(RetTypes);
-  return new FixedArgCountMatcherDescriptor(
+  return llvm::make_unique<FixedArgCountMatcherDescriptor>(
       matcherMarshall0<ReturnType>, reinterpret_cast<void (*)()>(Func),
       MatcherName, RetTypes, None);
 }
 
 /// \brief 1-arg overload
 template <typename ReturnType, typename ArgType1>
-MatcherDescriptor *makeMatcherAutoMarshall(ReturnType (*Func)(ArgType1),
-                                     StringRef MatcherName) {
+std::unique_ptr<MatcherDescriptor>
+makeMatcherAutoMarshall(ReturnType (*Func)(ArgType1), StringRef MatcherName) {
   std::vector<ast_type_traits::ASTNodeKind> RetTypes;
   BuildReturnTypeVector<ReturnType>::build(RetTypes);
   ArgKind AK = ArgTypeTraits<ArgType1>::getKind();
-  return new FixedArgCountMatcherDescriptor(
+  return llvm::make_unique<FixedArgCountMatcherDescriptor>(
       matcherMarshall1<ReturnType, ArgType1>,
       reinterpret_cast<void (*)()>(Func), MatcherName, RetTypes, AK);
 }
 
 /// \brief 2-arg overload
 template <typename ReturnType, typename ArgType1, typename ArgType2>
-MatcherDescriptor *makeMatcherAutoMarshall(ReturnType (*Func)(ArgType1, ArgType2),
-                                     StringRef MatcherName) {
+std::unique_ptr<MatcherDescriptor>
+makeMatcherAutoMarshall(ReturnType (*Func)(ArgType1, ArgType2),
   std::vector<ast_type_traits::ASTNodeKind> RetTypes;
   BuildReturnTypeVector<ReturnType>::build(RetTypes);
   ArgKind AKs[] = { ArgTypeTraits<ArgType1>::getKind(),
                     ArgTypeTraits<ArgType2>::getKind() };
-  return new FixedArgCountMatcherDescriptor(
+  return llvm::make_unique<FixedArgCountMatcherDescriptor>(
       matcherMarshall2<ReturnType, ArgType1, ArgType2>,
       reinterpret_cast<void (*)()>(Func), MatcherName, RetTypes, AKs);
 }
 
 /// \brief Variadic overload.
 template <typename ResultT, typename ArgT,
           ResultT (*Func)(ArrayRef<const ArgT *>)>
-MatcherDescriptor *makeMatcherAutoMarshall(
+std::unique_ptr<MatcherDescriptor> makeMatcherAutoMarshall(
     ast_matchers::internal::VariadicFunction<ResultT, ArgT, Func> VarFunc,
     StringRef MatcherName) {
-  return new VariadicFuncMatcherDescriptor(VarFunc, MatcherName);
+  return llvm::make_unique<VariadicFuncMatcherDescriptor>(VarFunc, MatcherName);
 }
 
 /// \brief Overload for VariadicDynCastAllOfMatchers.
 ///
 /// Not strictly necessary, but DynCastAllOfMatcherDescriptor gives us better
 /// completion results for that type of matcher.
 template <typename BaseT, typename DerivedT>
-MatcherDescriptor *
-makeMatcherAutoMarshall(ast_matchers::internal::VariadicDynCastAllOfMatcher<
-                            BaseT, DerivedT> VarFunc,
-                        StringRef MatcherName) {
-  return new DynCastAllOfMatcherDescriptor(VarFunc, MatcherName);
+std::unique_ptr<MatcherDescriptor> makeMatcherAutoMarshall(
+    ast_matchers::internal::VariadicDynCastAllOfMatcher<BaseT, DerivedT>
+        VarFunc,
+  return llvm::make_unique<DynCastAllOfMatcherDescriptor>(VarFunc, MatcherName);
 }
 
 /// \brief Argument adaptative overload.
 template <template <typename ToArg, typename FromArg> class ArgumentAdapterT,
           typename FromTypes, typename ToTypes>
-MatcherDescriptor *
-makeMatcherAutoMarshall(ast_matchers::internal::ArgumentAdaptingMatcherFunc<
-                            ArgumentAdapterT, FromTypes, ToTypes>,
-                        StringRef MatcherName) {
-  std::vector<MatcherDescriptor *> Overloads;
+std::unique_ptr<MatcherDescriptor> makeMatcherAutoMarshall(
+    ast_matchers::internal::ArgumentAdaptingMatcherFunc<ArgumentAdapterT,
+                                                        FromTypes, ToTypes>,
+  std::vector<std::unique_ptr<MatcherDescriptor>> Overloads;
   AdaptativeOverloadCollector<ArgumentAdapterT, FromTypes, ToTypes>(MatcherName,
                                                                     Overloads);
-  return new OverloadedMatcherDescriptor(Overloads);
+  return llvm::make_unique<OverloadedMatcherDescriptor>(Overloads);
 }
 
 template <template <typename ToArg, typename FromArg> class ArgumentAdapterT,
           typename FromTypes, typename ToTypes>
 template <typename FromTypeList>
 inline void AdaptativeOverloadCollector<ArgumentAdapterT, FromTypes,
                                         ToTypes>::collect(FromTypeList) {
   Out.push_back(makeMatcherAutoMarshall(
       &AdaptativeFunc::template create<typename FromTypeList::head>, Name));
   collect(typename FromTypeList::tail());
 }
 
 /// \brief Variadic operator overload.
 template <unsigned MinCount, unsigned MaxCount>
-MatcherDescriptor *
-makeMatcherAutoMarshall(ast_matchers::internal::VariadicOperatorMatcherFunc<
-                            MinCount, MaxCount> Func,
-                        StringRef MatcherName) {
-  return new VariadicOperatorMatcherDescriptor(MinCount, MaxCount, Func.Op,
-                                               MatcherName);
+std::unique_ptr<MatcherDescriptor> makeMatcherAutoMarshall(
+    ast_matchers::internal::VariadicOperatorMatcherFunc<MinCount, MaxCount>
+        Func,
+  return llvm::make_unique<VariadicOperatorMatcherDescriptor>(
+      MinCount, MaxCount, Func.Op, MatcherName);
 }
 
 } // namespace internal
 } // namespace dynamic
 } // namespace ast_matchers
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_MATCHERS_DYNAMIC_MARSHALLERS_H

clang/lib/ASTMatchers/Dynamic/Registry.cpp

 
 namespace clang {
 namespace ast_matchers {
 namespace dynamic {
 namespace {
 
 using internal::MatcherDescriptor;
 
-typedef llvm::StringMap<const MatcherDescriptor *> ConstructorMap;
+typedef llvm::StringMap<std::unique_ptr<const MatcherDescriptor>> ConstructorMap;
 class RegistryMaps {
 public:
   RegistryMaps();
   ~RegistryMaps();
 
   const ConstructorMap &constructors() const { return Constructors; }
 
 private:
-  void registerMatcher(StringRef MatcherName, MatcherDescriptor *Callback);
+  void registerMatcher(StringRef MatcherName,
+                       std::unique_ptr<MatcherDescriptor> Callback);
+
   ConstructorMap Constructors;
 };
 
-void RegistryMaps::registerMatcher(StringRef MatcherName,
-                                   MatcherDescriptor *Callback) {
+void RegistryMaps::registerMatcher(
+    StringRef MatcherName, std::unique_ptr<MatcherDescriptor> Callback) {
   assert(Constructors.find(MatcherName) == Constructors.end());
-  Constructors[MatcherName] = Callback;
+  Constructors[MatcherName] = std::move(Callback);
 }
 
 #define REGISTER_MATCHER(name)                                                 \
   registerMatcher(#name, internal::makeMatcherAutoMarshall(                    \
                              ::clang::ast_matchers::name, #name));
 
 #define SPECIFIC_MATCHER_OVERLOAD(name, Id)                                    \
-  static_cast< ::clang::ast_matchers::name##_Type##Id>(                        \
+  static_cast<::clang::ast_matchers::name##_Type##Id>(                         \
       ::clang::ast_matchers::name)
 
 #define REGISTER_OVERLOADED_2(name)                                            \
   do {                                                                         \
-    MatcherDescriptor *Callbacks[] = {                                         \
-      internal::makeMatcherAutoMarshall(SPECIFIC_MATCHER_OVERLOAD(name, 0),    \
-                                        #name),                                \
-      internal::makeMatcherAutoMarshall(SPECIFIC_MATCHER_OVERLOAD(name, 1),    \
-                                        #name)                                 \
-    };                                                                         \
-    registerMatcher(#name,                                                     \
-                    new internal::OverloadedMatcherDescriptor(Callbacks));     \
+    std::unique_ptr<MatcherDescriptor> Callbacks[] = {                         \
+        internal::makeMatcherAutoMarshall(SPECIFIC_MATCHER_OVERLOAD(name, 0),  \
+                                          #name),                              \
+        internal::makeMatcherAutoMarshall(SPECIFIC_MATCHER_OVERLOAD(name, 1),  \
+                                          #name)};                             \
+    registerMatcher(                                                           \
+        #name,                                                                 \
+        llvm::make_unique<internal::OverloadedMatcherDescriptor>(Callbacks));  \
   } while (0)
 
 /// \brief Generate a registry map with all the known matchers.
 RegistryMaps::RegistryMaps() {
   // TODO: Here is the list of the missing matchers, grouped by reason.
   //
   // Need Variant/Parser fixes:
   // ofKind
RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(valueDecl);
   REGISTER_MATCHER(varDecl);
   REGISTER_MATCHER(variableArrayType);
   REGISTER_MATCHER(voidType);
   REGISTER_MATCHER(whileStmt);
   REGISTER_MATCHER(withInitializer);
 }
 
-RegistryMaps::~RegistryMaps() {
-  llvm::DeleteContainerSeconds(Constructors);
-}
+RegistryMaps::~RegistryMaps() {}
 
 static llvm::ManagedStatic<RegistryMaps> RegistryData;
 
 } // anonymous namespace
 
 // static
 llvm::Optional<MatcherCtor> Registry::lookupMatcherCtor(StringRef MatcherName) {
-  ConstructorMap::const_iterator it =
-      RegistryData->constructors().find(MatcherName);
+  auto it = RegistryData->constructors().find(MatcherName);
   return it == RegistryData->constructors().end()
              ? llvm::Optional<MatcherCtor>()
-             : it->second;
+             : it->second.get();
 }
 
 namespace {
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
                               const std::set<ASTNodeKind> &KS) {
   unsigned Count = 0;
   for (std::set<ASTNodeKind>::const_iterator I = KS.begin(), E = KS.end();

 }
 
 std::vector<MatcherCompletion>
 Registry::getMatcherCompletions(ArrayRef<ArgKind> AcceptedTypes) {
   std::vector<MatcherCompletion> Completions;
 
   // Search the registry for acceptable matchers.
   for (const auto &M : RegistryData->constructors()) {
-    const auto *Matcher = M.getValue();
+    const std::unique_ptr<const MatcherDescriptor>& Matcher = M.getValue();
     StringRef Name = M.getKey();
 
     std::set<ASTNodeKind> RetKinds;
     unsigned NumArgs = Matcher->isVariadic() ? 1 : Matcher->getNumArgs();
     bool IsPolymorphic = Matcher->isPolymorphic();
     std::vector<std::vector<ArgKind>> ArgsKinds(NumArgs);
     unsigned MaxSpecificity = 0;
     for (const ArgKind& Kind : AcceptedTypes) {