[PATCH] First revision of the dynamic ASTMatcher library

[Samuel Benzaquen]

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:308
@@ +307,3 @@
+  /// \brief Makes a copy of this matcher object.
+  virtual Matcher<T>* clone() const {
+    return new Matcher<T>(*this);
----------------
Dmitri Gribenko wrote:
> Star goes to the right.
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:35-38
@@ +34,6 @@
+    llvm::StringRef Token;
+    int StartLine;
+    int EndLine;
+    int StartColumn;
+    int EndColumn;
+  };
----------------
Dmitri Gribenko wrote:
> 'unsigned'?
Done

================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:43
@@ +42,3 @@
+  public:
+    virtual ~TokenProcessor() {}
+    virtual VariantValue processValueToken(const VariantValue &Value,
----------------
Dmitri Gribenko wrote:
> This should be defined in .cpp file, probably (or vtable will be emitted into every object file).
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:41
@@ +40,3 @@
+
+  class TokenProcessor {
+  public:
----------------
Dmitri Gribenko wrote:
> Documentation comment?
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:50
@@ +49,3 @@
+    processMatcherToken(llvm::StringRef MatcherName,
+                        const std::vector<VariantValue> &Args,
+                        const TokenInfo &Info) = 0;
----------------
Dmitri Gribenko wrote:
> Why not ArrayRef?
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:58
@@ +57,3 @@
+private:
+  Parser(); // Declared, but not defined.
+};
----------------
Dmitri Gribenko wrote:
> Use LLVM_DELETED_FUNCTION.
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:33-34
@@ +32,4 @@
+  // number of arguments or argument types do not match the signature.
+  static VariantValue constructMatcher(const std::string &MatcherName,
+                                       const std::vector<VariantValue> &Args);
+
----------------
Dmitri Gribenko wrote:
> Dmitri Gribenko wrote:
> > StringRef, ArrayRef?
> Three slashes.
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:37
@@ +36,3 @@
+private:
+  Registry(); // Declared, but not defined.
+};
----------------
Dmitri Gribenko wrote:
> Use LLVM_DELETED_FUNCTION.
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/VariantValue.h:40
@@ +39,3 @@
+struct VariantValueReturnType {
+  typedef const T& type;
+};
----------------
Dmitri Gribenko wrote:
> & goes to the right.
Done. Force of habit... clang-format helps a lot here!

================
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:105-106
@@ +104,4 @@
+  llvm::StringRef StartOfLine;
+  int Line;
+  int column() const { return Code.data() - StartOfLine.data() + 1; }
+};
----------------
Dmitri Gribenko wrote:
> unsigned?
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:33
@@ +32,3 @@
+  // number of arguments or argument types do not match the signature.
+  static VariantValue constructMatcher(const std::string &MatcherName,
+                                       const std::vector<VariantValue> &Args);
----------------
Samuel Benzaquen wrote:
> Dmitri Gribenko wrote:
> > Dmitri Gribenko wrote:
> > > StringRef, ArrayRef?
> > Three slashes.
> Done.
Done.

================
Comment at: include/clang/ASTMatchers/Dynamic/VariantValue.h:35-37
@@ +34,5 @@
+
+// Helper template metafunction to determine the return type of
+// VariantValue::get(). For Matcher<T> we return by value, since we do matcher
+// conversion on demand.
+template <class T>
----------------
Dmitri Gribenko wrote:
> Three slashes.
Done.


http://llvm-reviews.chandlerc.com/D714