================ Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:245 @@ -244,1 +244,3 @@ + MatcherCtor Ctor = + S->lookupMatcherCtor(NameToken.Text, NameToken.Range, Error); ---------------- Samuel Benzaquen wrote: > Move this call closer to the first use of Ctor. When the parser is taught to do completions, this will need to be here so that we can push it onto the context stack.
================ Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:40 @@ -33,3 +39,3 @@ /// - /// Consult the registry of known matchers and construct the appropriate - /// matcher by name. + /// \return An opaque pointer which may be used to refer to the matcher + /// constructor, or null if not found. In that case \c Error will contain the ---------------- Samuel Benzaquen wrote: > If you describe it as an "opaque pointer", I think you should return > MatcherCtor* (with the appropriate change in the typedef above). Clients don't normally care what this is unless they are trying to check for errors from this function. I guess we can use llvm::Optional for this return value if we don't want to mention the fact that this is a pointer, although I don't think it matters much. ================ Comment at: unittests/ASTMatchers/Dynamic/ParserTest.cpp:73 @@ +72,3 @@ + typedef std::map<std::string, ast_matchers::internal::Matcher<Stmt> > + ExpectedMatchersTy; + ExpectedMatchersTy ExpectedMatchers; ---------------- Samuel Benzaquen wrote: > Is Ty short for Type? Yeah. It's a pretty common abbreviation in LLVM land. ================ Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:49 @@ -36,3 +48,3 @@ /// /// \param MatcherName The name of the matcher to instantiate. /// ---------------- Samuel Benzaquen wrote: > s/MatcherName/Ctor/ Done. ================ Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:34 @@ +33,3 @@ + +typedef const internal::MatcherCreateCallback *MatcherCtor; + ---------------- Manuel Klimek wrote: > Again, saving 7 characters. Sigh :) Oh well, I guess Ctor is kind of a domain > name... Everyone who works on C++ compilers should know what a Ctor is... http://llvm-reviews.chandlerc.com/D2207 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
