[PATCH] Add support for named values in the parser.

Peter Collingbourne peter at pcc.me.uk
Fri Apr 11 11:47:19 PDT 2014


LGTM.

On Mon, Apr 07, 2014 at 08:41:53AM -0700, Samuel Benzaquen wrote:
> 
> 
> ================
> Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:304
> @@ +303,3 @@
> +
> +  // Parse as a matcher expressoin.
> +  return parseMatcherExpressionImpl(NameToken, Value);
> ----------------
> Peter Collingbourne wrote:
> > Typo: "expression".
> Fixed.
> 
> ================
> Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:334
> @@ -333,2 +333,3 @@
>    if (it == RegistryData->constructors().end()) {
> -    Error->addError(NameRange, Error->ET_RegistryNotFound) << MatcherName;
> +    if (Error) {
> +      Error->addError(NameRange, Error->ET_RegistryMatcherNotFound)
> ----------------
> Peter Collingbourne wrote:
> > I wonder if it would be better to drop the Error parameter from this function (and Sema::lookupMatcherCtor) and move diagnostic emission to the parser. That would be more consistent with how Sema::getNamedValue works, and would also allow you to do both lookups once, in parseIdentifierPrefixImpl.
> Yes. It made sense when it was inside constructMatcher, but now it makes more sense outside.
> 
> 
> http://reviews.llvm.org/D3276
> 
> 
> 

-- 
Peter



More information about the cfe-commits mailing list