[PATCH] D18275: [ASTMatchers] Add own version of VariadicFunction.

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:80
@@ +79,3 @@
+  ResultT operator()(ArrayRef<ArgT> Args) const {
+    std::vector<const ArgT *> InnerArgs;
+    for (const ArgT &Arg : Args)
----------------
alexfh wrote:
> It's unfortunate that we need to create an array of the argument pointers here, but it seems there's no larger common denominator of the two ways this functions can be called.
> 
> One nit though: SmallVector will be a better container here.
> It's unfortunate that we need to create an array of the argument pointers here, but it seems there's no larger  common denominator of the two ways this functions can be called.

Now that we control it, we can change it to be something different.
For example, instead of passing a function pointer as template parameter we could pass a class that contains overloaded operator() for both ArrayRef<T> and ArrayRef<const T*>.

On the other hand, constructing the matchers has never been the performance bottleneck of the framework.
They are usually created once and used thousands/millions of times.
Might not be worth it to optimize this too much.

> One nit though: SmallVector will be a better container here.

Done.


http://reviews.llvm.org/D18275