Remove some template instantiations to reduce the number of symbols.

[sbenza]

Summary

Remove some template instantiations to reduce the number of symbols.
Registry.cpp instantiates all the matchers and it fails to build under MSVC in debug mode.

[klimek]

/me wonders whether that's really worth it...

[sbenza]

This is not the first time we see this.
Maybe someone can decide that /bigobj is required for MSVC in debug mode and we don't have to do this anymore.

[rnk]

MinGW doesn't support anything like /bigobj, so I think we need this or something like it.
http://stackoverflow.com/questions/16596876/object-file-has-too-many-sections

I wonder if putting LLVM_ATTRIBUTE_ALWAYS_INLINE on all IntrusiveRefCntPtr methods would also help reduce the section count.

[sbenza]

In D5072#7, @rnk wrote:

MinGW doesn't support anything like /bigobj, so I think we need this or something like it.
http://stackoverflow.com/questions/16596876/object-file-has-too-many-sections

I wonder if putting LLVM_ATTRIBUTE_ALWAYS_INLINE on all IntrusiveRefCntPtr methods would also help reduce the section count.

That would only work if the macro was defined for and used by MinGW.

[rnk]

The always inline macro should be defined pretty on MinGW and pretty much everywhere. It was introduced in gcc 4.0 and we already require gcc 4.7 at a minimum.

BTW, I think the ASTMatchersInternal.h change is a solid compile time win at very low cost. Using always_inline there would still cause us to perform instantiation, so I like this change better.

The unique_ptr change is more questionable, because now the ownership is unmanaged and you would have to implement reset manually in the future. Also consider that the definition of unique_ptr::reset is something like:

void reset(T *New) {
  T *Tmp = Out;
  Out = New;
  delete Tmp;
}

[sbenza]

Please take a look at D5124, which makes this one more or less obsolete.

[sbenza]

This was resolved with a different CL.

include/clang/ASTMatchers/ASTMatchersInternal.h

 /// with respect to T. The relationship is built via a type conversion
 /// operator rather than a type hierarchy to be able to templatize the
 /// type hierarchy instead of spelling it out.
 template <typename T>
 class Matcher {
 public:
   /// \brief Takes ownership of the provided implementation pointer.
   explicit Matcher(MatcherInterface<T> *Implementation)
-      : Implementation(Implementation) {}
+      : Implementation(Implementation), ImplRefCount(Implementation) {}
 
   /// \brief Implicitly converts \c Other to a Matcher<T>.
   ///
   /// Requires \c T to be derived from \c From.
   template <typename From>
   Matcher(const Matcher<From> &Other,
           typename std::enable_if<std::is_base_of<From, T>::value &&
                                   !std::is_same<From, T>::value>::type * = 0)
-      : Implementation(new ImplicitCastMatcher<From>(Other)) {}
+      : Implementation(new ImplicitCastMatcher<From>(Other)),
+        ImplRefCount(Implementation) {}
 
   /// \brief Implicitly converts \c Matcher<Type> to \c Matcher<QualType>.
   ///
   /// The resulting matcher is not strict, i.e. ignores qualifiers.
   template <typename TypeT>
   Matcher(const Matcher<TypeT> &Other,
-          typename std::enable_if<
-            std::is_same<T, QualType>::value &&
-            std::is_same<TypeT, Type>::value>::type* = 0)
-      : Implementation(new TypeToQualType<TypeT>(Other)) {}
+          typename std::enable_if<std::is_same<T, QualType>::value &&
+                                  std::is_same<TypeT, Type>::value>::type * = 0)
+      : Implementation(new TypeToQualType<TypeT>(Other)),
+        ImplRefCount(Implementation) {}
 
   /// \brief Forwards the call to the underlying MatcherInterface<T> pointer.
   bool matches(const T &Node,
                ASTMatchFinder *Finder,
                BoundNodesTreeBuilder *Builder) const {
     if (Implementation->matches(Node, Finder, Builder))
       return true;
     // Delete all bindings when a matcher does not match.
     // This prevents unexpected exposure of bound nodes in unmatches
     // branches of the match tree.
     *Builder = BoundNodesTreeBuilder();
     return false;
   }
 
   /// \brief Returns an ID that uniquely identifies the matcher.
   uint64_t getID() const {
     /// FIXME: Document the requirements this imposes on matcher
     /// implementations (no new() implementation_ during a Matches()).
-    return reinterpret_cast<uint64_t>(Implementation.get());
+    return reinterpret_cast<uint64_t>(Implementation);
   }
 
   /// \brief Allows the conversion of a \c Matcher<Type> to a \c
   /// Matcher<QualType>.
   ///
   /// Depending on the constructor argument, the matcher is either strict, i.e.
   /// does only matches in the absence of qualifiers, or not, i.e. simply
   /// ignores any qualifiers.
bool matches(const T &Node, ASTMatchFinder *Finder,
                  BoundNodesTreeBuilder *Builder) const override {
       return From.matches(Node, Finder, Builder);
     }
 
   private:
     const Matcher<Base> From;
   };
 
-  IntrusiveRefCntPtr< MatcherInterface<T> > Implementation;
+  MatcherInterface<T> *Implementation;
+  // Do not instantiate IntrusiveRefCntPtr<> for each MatcherInterface<T>.
+  // This reduces the symbol bloat that breaks MSVC builds.
+  IntrusiveRefCntPtr<RefCountedBaseVPTR> ImplRefCount;
 };  // class Matcher
 
 /// \brief A convenient helper for creating a Matcher<T> without specifying
 /// the template type argument.
 template <typename T>
 inline Matcher<T> makeMatcher(MatcherInterface<T> *Implementation) {
   return Matcher<T>(Implementation);
 }

include/clang/ASTMatchers/Dynamic/VariantValue.h

private:
   class PolymorphicPayload;
   class VariadicOpPayload;
 
   template <typename T>
   class TypedMatcherOps : public MatcherOps {
   public:
     typedef ast_matchers::internal::Matcher<T> MatcherT;
 
+    TypedMatcherOps() : Out() {}
+    ~TypedMatcherOps() { delete Out; }
+
     virtual bool canConstructFrom(const DynTypedMatcher &Matcher,
                                   bool &IsExactMatch) const {
       IsExactMatch = Matcher.getSupportedKind().isSame(
           ast_type_traits::ASTNodeKind::getFromNodeKind<T>());
       return Matcher.canConvertTo<T>();
     }
 
     virtual void constructFrom(const DynTypedMatcher& Matcher) {
-      Out.reset(new MatcherT(Matcher.convertTo<T>()));
+      delete Out;
+      Out = new MatcherT(Matcher.convertTo<T>());
     }
 
     virtual void constructVariadicOperator(
         ast_matchers::internal::VariadicOperatorFunction Func,
         ArrayRef<VariantMatcher> InnerMatchers) {
       std::vector<DynTypedMatcher> DynMatchers;
       for (size_t i = 0, e = InnerMatchers.size(); i != e; ++i) {
         // Abort if any of the inner matchers can't be converted to
         // Matcher<T>.
         if (!InnerMatchers[i].hasTypedMatcher<T>()) {
           return;
         }
         DynMatchers.push_back(InnerMatchers[i].getTypedMatcher<T>());
       }
-      Out.reset(new MatcherT(
+      delete Out;
+      Out = new MatcherT(
           new ast_matchers::internal::VariadicOperatorMatcherInterface<T>(
-              Func, DynMatchers)));
+              Func, DynMatchers));
     }
 
-    bool hasMatcher() const { return Out.get() != nullptr; }
+    bool hasMatcher() const { return Out != nullptr; }
     const MatcherT &matcher() const { return *Out; }
 
   private:
-    std::unique_ptr<MatcherT> Out;
+    // Do not instantiate std::unique_ptr<> for each MatcherT.
+    // This reduces the symbol bloat that breaks MSVC builds.
+    MatcherT* Out;
   };
 
   IntrusiveRefCntPtr<const Payload> Value;
 };
 
 /// \brief Variant value class.
 ///
 /// Basically, a tagged union with value type semantics.