[PATCH] First revision of the dynamic ASTMatcher library

[Samuel Benzaquen]

  Submitted the last diff with arc (the first one in this revision). I am not sure if it did what I was expecting...


================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:54
@@ +53,3 @@
+
+  static VariantValue parseMatcher(llvm::StringRef MatcherCode,
+                                   TokenProcessor *Processor);
----------------
Manuel Klimek wrote:
> Samuel Benzaquen wrote:
> > Manuel Klimek wrote:
> > > I'm wondering whether we want to call the return value differently to make sure it's usually a matcher? (I assume it's called value because it can also be just a literal value type).
> > The result could also be an error. Having the error in the VariantValue makes many interfaces simpler.
> > In particular, it allows this method and all the factory methods to return errors.
> > 
> > Also, some of the test code actually returns strings here (the processor does it) for simplicity.
> Yea, I think "the result can also be an error" is what I'm unhappy about, at least with the current naming.
> If VariantValue was simpler to understand I'd probably be fine with it, but it seems a tad to clever for my taste. As a user of this interface, what I really want is a matcher, and I'd like to hide the fact that somewhere in the implementation VariantValue is needed, because it can currently be either a matcher or a literal value.
Removed the error from VariantValue and changed all interfaces to return a bool/pointer.
Added a Diagnostics class to gather the errors.

================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:41
@@ +40,3 @@
+
+  class TokenProcessor {
+  public:
----------------
Manuel Klimek wrote:
> Samuel Benzaquen wrote:
> > Samuel Benzaquen wrote:
> > > Dmitri Gribenko wrote:
> > > > Manuel Klimek wrote:
> > > > > Since this is an interface for the Parser, I think this requires a lot more documentation to explain
> > > > > a) why it is an interface (instead of just built-in)
> > > > > b) why processValueToken has a default implementation
> > > > > 
> > > > Documentation comment?
> > > Done.
> > Already added some comments. Let me know if you want something extra.
> Cool, thanks, that helps a lot... Why not have a default implementation that does what RealProcessor does, though? Otherwise, I think the interface here is asymmetric - it should either be fully abstract, or have decent implementations for both.
I didn't want the registry to depend on the parser or the parser to depend on the registry, but gave up on that...
The registry now has a method to return an implementation of the processor.

================
Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:31-32
@@ +30,4 @@
+  // matcher by name.
+  // It will return VariantError if the matcher is not found, or if the
+  // number of arguments or argument types do not match the signature.
+  static VariantValue constructMatcher(const std::string &MatcherName,
----------------
Manuel Klimek wrote:
> Samuel Benzaquen wrote:
> > Manuel Klimek wrote:
> > > Can we instead use Diagnostics for error reporting?
> > I don't know. It looks like Diagnostics works with SourceManager/SourceLocation.
> > I could make the error type have more diagnostic info, like line/col numbers.
> Generally, in clang, when we parse stuff using the SourceManager/SourceLocation/Diagnostics is a good idea :) Every time I thought it was too much effort, in the end I regretted it...
I am not opposed to the idea. It is just that I don't think I can use some of it without using all of it, and I don't think I can use all of it cleanly.
I made a Diagnostics class for the errors. Let me know if that is ok.

================
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:119
@@ +118,3 @@
+
+static const RegistryMaps *const RegistryData = registerMatchers();
+
----------------
Doug Gregor wrote:
> This is a ton of global initialization. Is this really the best way?
The initialization is now done lazily.


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

BRANCH
  svn

ARCANIST PROJECT
  clang