[PATCH] First revision of the dynamic ASTMatcher library

[Dmitri Gribenko]

  There are still a few llvm::ArrayRefs and llvm::StringRefs in the patch left.

  re VariantValue:
  Is it necessary to introduce the is()/get() magic and a union instead of using Casting.h-like casts?  Do you expect it to significantly help with readability?


================
Comment at: lib/ASTMatchers/Dynamic/VariantValue.cpp:60-61
@@ +59,4 @@
+void VariantValue::setImpl(const std::string &NewValue) {
+  Type = VT_String;
+  Value.String = new std::string(NewValue);
+}
----------------
Samuel Benzaquen wrote:
> Dmitri Gribenko wrote:
> > What if someone calls setImpl() repeatedly?  Do we just leak old values?
> > 
> I could call reset() from all the setImpl() overloads. Right now it is called once from the place where setImpl() is called.
No need to make the functionality more complex than it is now.  `assert(Type == VT_Nothing)` here and in the other setImpl() overload should catch the issue.


================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:42-43
@@ +41,4 @@
+
+/// \brief Matcher expression parser.
+class Parser {
+public:
----------------
Samuel Benzaquen wrote:
> Dmitri Gribenko wrote:
> > This class just holds a few static member functions.  Is the class really needed here?
> It is not really needed. I could take out the class / rename as namespace.
> The same applies to Registry, I guess.
Or you could take advantage of 'Parser' being a real class: if it is appropriate, you could move 'TokenProcessor' and 'Diagnostics' to be class members.


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

BRANCH
  svn

ARCANIST PROJECT
  clang