[AST] Convert Marshallers to use unique_ptr.

[timshen]

clang/lib/ASTMatchers/Dynamic/Marshallers.h

514 ↗

(On Diff #74103)

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 ↗

(On Diff #74103)

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

[jlebar]

clang/lib/ASTMatchers/Dynamic/Marshallers.h

514 ↗

(On Diff #74103)

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.

cfe/trunk/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

cfe/trunk/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 MatcherDescriptor& Matcher = *M.getValue();
     StringRef Name = M.getKey();
 
     std::set<ASTNodeKind> RetKinds;
-    unsigned NumArgs = Matcher->isVariadic() ? 1 : Matcher->getNumArgs();
-    bool IsPolymorphic = Matcher->isPolymorphic();
+    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) {
       if (Kind.getArgKind() != Kind.AK_Matcher)
         continue;
       unsigned Specificity;
       ASTNodeKind LeastDerivedKind;
-      if (Matcher->isConvertibleTo(Kind.getMatcherKind(), &Specificity,
-                                   &LeastDerivedKind)) {
+      if (Matcher.isConvertibleTo(Kind.getMatcherKind(), &Specificity,
         if (MaxSpecificity < Specificity)
           MaxSpecificity = Specificity;
         RetKinds.insert(LeastDerivedKind);
         for (unsigned Arg = 0; Arg != NumArgs; ++Arg)
-          Matcher->getArgKinds(Kind.getMatcherKind(), Arg, ArgsKinds[Arg]);
+          Matcher.getArgKinds(Kind.getMatcherKind(), Arg, ArgsKinds[Arg]);
         if (IsPolymorphic)
           break;
       }
     }
 
     if (!RetKinds.empty() && MaxSpecificity > 0) {
       std::string Decl;
       llvm::raw_string_ostream OS(Decl);
if (!RetKinds.empty() && MaxSpecificity > 0) {
             }
           }
           if (!MatcherKinds.empty()) {
             if (!FirstArgKind) OS << "|";
             OS << "Matcher<" << MatcherKinds << ">";
           }
         }
       }
-      if (Matcher->isVariadic())
+      if (Matcher.isVariadic())
         OS << "...";
       OS << ")";
 
       std::string TypedText = Name;
       TypedText += "(";
       if (ArgsKinds.empty())
         TypedText += ")";
       else if (ArgsKinds[0][0].getArgKind() == ArgKind::AK_String)