Add support for named values in the parser.

[sbenza]

Summary

Add support for named values in the parser.

[sbenza]

I rolled back the premature cl and sent it again.
Please review on this thread instead.

[sbenza]

Added "Value not found" error.
Added explicit conversion to bool on VariantValue. Replaced isNothing() with hasValue().

[sbenza]

Added the check you asked for. It will say "Value not found" only when it think you really meant a to use a value.

[pcc]

lib/ASTMatchers/Dynamic/Parser.cpp

308

Typo: "expression".

lib/ASTMatchers/Dynamic/Registry.cpp

330–331

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.

[sbenza]

Move error handling into the parser.

[sbenza]

lib/ASTMatchers/Dynamic/Parser.cpp

308

Fixed.

lib/ASTMatchers/Dynamic/Registry.cpp

330–331

Yes. It made sense when it was inside constructMatcher, but now it makes more sense outside.

[pcc]

LGTM.

[pcc]

Was committed in r206176.

include/clang/ASTMatchers/Dynamic/Diagnostics.h

enum ContextType {
     CT_MatcherArg = 0,
     CT_MatcherConstruct = 1
   };
 
   /// \brief All errors from the system.
   enum ErrorType {
     ET_None = 0,
 
-    ET_RegistryNotFound = 1,
+    ET_RegistryMatcherNotFound = 1,
     ET_RegistryWrongArgCount = 2,
     ET_RegistryWrongArgType = 3,
     ET_RegistryNotBindable = 4,
     ET_RegistryAmbiguousOverload = 5,
+    ET_RegistryValueNotFound = 6,
 
     ET_ParserStringError = 100,
     ET_ParserNoOpenParen = 101,
     ET_ParserNoCloseParen = 102,
     ET_ParserNoComma = 103,
     ET_ParserNoCode = 104,
     ET_ParserNotAMatcher = 105,
     ET_ParserInvalidToken = 106,

include/clang/ASTMatchers/Dynamic/Parser.h

 /// The parser understands matcher expressions of the form:
 ///   MatcherName(Arg0, Arg1, ..., ArgN)
 /// as well as simple types like strings.
 /// The parser does not know how to process the matchers. It delegates this task
 /// to a Sema object received as an argument.
 ///
 /// \code
 /// Grammar for the expressions supported:
-/// <Expression>        := <Literal> | <MatcherExpression>
+/// <Expression>        := <Literal> | <NamedValue> | <MatcherExpression>
 /// <Literal>           := <StringLiteral> | <Unsigned>
 /// <StringLiteral>     := "quoted string"
 /// <Unsigned>          := [0-9]+
-/// <MatcherExpression> := <MatcherName>(<ArgumentList>) |
-///                        <MatcherName>(<ArgumentList>).bind(<StringLiteral>)
-/// <MatcherName>       := [a-zA-Z]+
+/// <NamedValue>        := <Identifier>
+/// <MatcherExpression> := <Identifier>(<ArgumentList>) |
+///                        <Identifier>(<ArgumentList>).bind(<StringLiteral>)
+/// <Identifier>        := [a-zA-Z]+
 /// <ArgumentList>      := <Expression> | <Expression>,<ArgumentList>
 /// \endcode
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_AST_MATCHERS_DYNAMIC_PARSER_H
 #define LLVM_CLANG_AST_MATCHERS_DYNAMIC_PARSER_H
 
public:
   /// However, a more complex processor might decide to intercept the matcher
   /// creation and do some extra work. For example, it could apply some
   /// transformation to the matcher by adding some id() nodes, or could detect
   /// specific matcher nodes for more efficient lookup.
   class Sema {
   public:
     virtual ~Sema();
 
+    /// \brief Lookup a value by name.
+    ///
+    /// This can be used in the Sema layer to declare known constants or to
+    /// allow to split an expression in pieces.
+    ///
+    /// \param Name The name of the value to lookup.
+    ///
+    /// \return The named value. It could be any type that VariantValue
+    ///   supports. An empty value means that the name is not recognized.
+    virtual VariantValue getNamedValue(StringRef Name);
+
     /// \brief Process a matcher expression.
     ///
     /// All the arguments passed here have already been processed.
     ///
     /// \param Ctor A matcher constructor looked up by lookupMatcherCtor.
     ///
     /// \param NameRange The location of the name in the matcher source.
     ///   Useful for error reporting.
virtual VariantMatcher actOnMatcherExpression(MatcherCtor Ctor,
                                                   StringRef BindID,
                                                   ArrayRef<ParserValue> Args,
                                                   Diagnostics *Error) = 0;
 
     /// \brief Look up a matcher by name.
     ///
     /// \param MatcherName The matcher name found by the parser.
     ///
-    /// \param NameRange The location of the name in the matcher source.
-    ///   Useful for error reporting.
-    ///
-    /// \return The matcher constructor, or Optional<MatcherCtor>() if an error
-    ///   occurred. In that case, \c Error will contain a description of the
-    ///   error.
+    /// \return The matcher constructor, or Optional<MatcherCtor>() if not
+    /// found.
     virtual llvm::Optional<MatcherCtor>
-    lookupMatcherCtor(StringRef MatcherName, const SourceRange &NameRange,
-                      Diagnostics *Error) = 0;
+    lookupMatcherCtor(StringRef MatcherName) = 0;
+  };
+
+  /// \brief Sema implementation that uses the matcher registry to process the
+  ///   tokens.
+  class RegistrySema : public Parser::Sema {
+   public:
+    virtual ~RegistrySema();
+
+    llvm::Optional<MatcherCtor>
+    lookupMatcherCtor(StringRef MatcherName) override;
+
+    VariantMatcher actOnMatcherExpression(MatcherCtor Ctor,
+                                          const SourceRange &NameRange,
+                                          StringRef BindID,
+                                          ArrayRef<ParserValue> Args,
+                                          Diagnostics *Error) override;
   };
 
   /// \brief Parse a matcher expression, creating matchers from the registry.
   ///
   /// This overload creates matchers calling directly into the registry. If the
   /// caller needs more control over how the matchers are created, then it can
   /// use the overload below that takes a Sema.
   ///
private:
   class CodeTokenizer;
   struct ScopedContextEntry;
   struct TokenInfo;
 
   Parser(CodeTokenizer *Tokenizer, Sema *S,
          Diagnostics *Error);
 
   bool parseExpressionImpl(VariantValue *Value);
-  bool parseMatcherExpressionImpl(VariantValue *Value);
+  bool parseMatcherExpressionImpl(const TokenInfo &NameToken,
+                                  VariantValue *Value);
+  bool parseIdentifierPrefixImpl(VariantValue *Value);
 
   void addCompletion(const TokenInfo &CompToken, StringRef TypedText,
                      StringRef Decl);
   void addExpressionCompletions();
 
   CodeTokenizer *const Tokenizer;
   Sema *const S;
   Diagnostics *const Error;

include/clang/ASTMatchers/Dynamic/Registry.h

struct MatcherCompletion {
   }
 };
 
 class Registry {
 public:
   /// \brief Look up a matcher in the registry by name,
   ///
   /// \return An opaque value which may be used to refer to the matcher
-  /// constructor, or Optional<MatcherCtor>() if not found.  In that case
-  /// \c Error will contain the description of the error.
-  static llvm::Optional<MatcherCtor>
-  lookupMatcherCtor(StringRef MatcherName, const SourceRange &NameRange,
-                    Diagnostics *Error);
+  /// constructor, or Optional<MatcherCtor>() if not found.
+  static llvm::Optional<MatcherCtor> lookupMatcherCtor(StringRef MatcherName);
 
   /// \brief Compute the list of completions for \p Context.
   ///
   /// Each element of \p Context represents a matcher invocation, going from
   /// outermost to innermost. Elements are pairs consisting of a reference to the
   /// matcher constructor and the index of the next element in the argument list
   /// of that matcher (or for the last element, the index of the completion
   /// point in the argument list). An empty list requests completion for the

include/clang/ASTMatchers/Dynamic/VariantValue.h

public:
   VariantMatcher();
 
   /// \brief Clones the provided matcher.
   static VariantMatcher SingleMatcher(const DynTypedMatcher &Matcher);
 
   /// \brief Clones the provided matchers.
   ///
   /// They should be the result of a polymorphic matcher.
-  static VariantMatcher PolymorphicMatcher(std::vector<DynTypedMatcher> Matchers);
+  static VariantMatcher
+  PolymorphicMatcher(std::vector<DynTypedMatcher> Matchers);
 
   /// \brief Creates a 'variadic' operator matcher.
   ///
   /// It will bind to the appropriate type on getTypedMatcher<T>().
   static VariantMatcher VariadicOperatorMatcher(
       ast_matchers::internal::VariadicOperatorFunction Func,
       std::vector<VariantMatcher> Args);
 
public:
   ~VariantValue();
   VariantValue &operator=(const VariantValue &Other);
 
   /// \brief Specific constructors for each supported type.
   VariantValue(unsigned Unsigned);
   VariantValue(const std::string &String);
   VariantValue(const VariantMatcher &Matchers);
 
+  /// \brief Returns true iff this is not an empty value.
+  LLVM_EXPLICIT operator bool() const { return hasValue(); }
+  bool hasValue() const { return Type != VT_Nothing; }
+
   /// \brief Unsigned value functions.
   bool isUnsigned() const;
   unsigned getUnsigned() const;
   void setUnsigned(unsigned Unsigned);
 
   /// \brief String value functions.
   bool isString() const;
   const std::string &getString() const;

lib/ASTMatchers/Dynamic/Diagnostics.cpp

switch (Type) {
     case Diagnostics::CT_MatcherArg:
       return "Error parsing argument $0 for matcher $1.";
   }
   llvm_unreachable("Unknown ContextType value.");
 }
 
 StringRef errorTypeToFormatString(Diagnostics::ErrorType Type) {
   switch (Type) {
-  case Diagnostics::ET_RegistryNotFound:
+  case Diagnostics::ET_RegistryMatcherNotFound:
     return "Matcher not found: $0";
   case Diagnostics::ET_RegistryWrongArgCount:
     return "Incorrect argument count. (Expected = $0) != (Actual = $1)";
   case Diagnostics::ET_RegistryWrongArgType:
     return "Incorrect type for arg $0. (Expected = $1) != (Actual = $2)";
   case Diagnostics::ET_RegistryNotBindable:
     return "Matcher does not support binding.";
   case Diagnostics::ET_RegistryAmbiguousOverload:
     // TODO: Add type info about the overload error.
     return "Ambiguous matcher overload.";
+  case Diagnostics::ET_RegistryValueNotFound:
+    return "Value not found: $0";
 
   case Diagnostics::ET_ParserStringError:
     return "Error parsing string token: <$0>";
   case Diagnostics::ET_ParserNoOpenParen:
     return "Error parsing matcher. Found token <$0> while looking for '('.";
   case Diagnostics::ET_ParserNoCloseParen:
     return "Error parsing matcher. Found end-of-code while looking for ')'.";
   case Diagnostics::ET_ParserNoComma:

lib/ASTMatchers/Dynamic/Parser.cpp

private:
   unsigned Line;
   Diagnostics *Error;
   TokenInfo NextToken;
   const char *CodeCompletionLocation;
 };
 
 Parser::Sema::~Sema() {}
 
+VariantValue Parser::Sema::getNamedValue(StringRef Name) {
+  return VariantValue();
+}
+
 struct Parser::ScopedContextEntry {
   Parser *P;
 
   ScopedContextEntry(Parser *P, MatcherCtor C) : P(P) {
     P->ContextStack.push_back(std::make_pair(C, 0u));
   }
 
   ~ScopedContextEntry() {
     P->ContextStack.pop_back();
   }
 
   void nextArg() {
     ++P->ContextStack.back().second;
   }
 };
 
+/// \brief Parse expressions that start with an identifier.
+///
+/// This function can parse named values and matchers.
+/// In case of failure it will try to determine the user's intent to give
+/// an appropriate error message.
+bool Parser::parseIdentifierPrefixImpl(VariantValue *Value) {
+  const TokenInfo NameToken = Tokenizer->consumeNextToken();
+
+  if (Tokenizer->nextTokenKind() != TokenInfo::TK_OpenParen) {
+    // Parse as a named value.
+    if (const VariantValue NamedValue = S->getNamedValue(NameToken.Text)) {
+      *Value = NamedValue;
+      return true;
+    }
+    // If the syntax is correct and the name is not a matcher either, report
+    // unknown named value.
+    if ((Tokenizer->nextTokenKind() == TokenInfo::TK_Comma ||
+         Tokenizer->nextTokenKind() == TokenInfo::TK_CloseParen ||
+         Tokenizer->nextTokenKind() == TokenInfo::TK_Eof) &&
+        !S->lookupMatcherCtor(NameToken.Text)) {
+      Error->addError(NameToken.Range, Error->ET_RegistryValueNotFound)
+          << NameToken.Text;
+      return false;
+    }
+    // Otherwise, fallback to the matcher parser.
+  }
+
+  // Parse as a matcher expression.
+  return parseMatcherExpressionImpl(NameToken, Value);
+}
+
 /// \brief Parse and validate a matcher expression.
 /// \return \c true on success, in which case \c Value has the matcher parsed.
 ///   If the input is malformed, or some argument has an error, it
 ///   returns \c false.
-bool Parser::parseMatcherExpressionImpl(VariantValue *Value) {
-  const TokenInfo NameToken = Tokenizer->consumeNextToken();
+bool Parser::parseMatcherExpressionImpl(const TokenInfo &NameToken,
+                                        VariantValue *Value) {
   assert(NameToken.Kind == TokenInfo::TK_Ident);
   const TokenInfo OpenToken = Tokenizer->consumeNextToken();
   if (OpenToken.Kind != TokenInfo::TK_OpenParen) {
     Error->addError(OpenToken.Range, Error->ET_ParserNoOpenParen)
         << OpenToken.Text;
     return false;
   }
 
-  llvm::Optional<MatcherCtor> Ctor =
-      S->lookupMatcherCtor(NameToken.Text, NameToken.Range, Error);
+  llvm::Optional<MatcherCtor> Ctor = S->lookupMatcherCtor(NameToken.Text);
+
+  if (!Ctor) {
+    Error->addError(NameToken.Range, Error->ET_RegistryMatcherNotFound)
+        << NameToken.Text;
+    // Do not return here. We need to continue to give completion suggestions.
+  }
+
   std::vector<ParserValue> Args;
   TokenInfo EndToken;
 
   {
     ScopedContextEntry SCE(this, Ctor ? *Ctor : 0);
 
     while (Tokenizer->nextTokenKind() != TokenInfo::TK_Eof) {
       if (Tokenizer->nextTokenKind() == TokenInfo::TK_CloseParen) {

 /// \brief Parse an <Expresssion>
 bool Parser::parseExpressionImpl(VariantValue *Value) {
   switch (Tokenizer->nextTokenKind()) {
   case TokenInfo::TK_Literal:
     *Value = Tokenizer->consumeNextToken().Value;
     return true;
 
   case TokenInfo::TK_Ident:
-    return parseMatcherExpressionImpl(Value);
+    return parseIdentifierPrefixImpl(Value);
 
   case TokenInfo::TK_CodeCompletion:
     addExpressionCompletions();
     return false;
 
   case TokenInfo::TK_Eof:
     Error->addError(Tokenizer->consumeNextToken().Range,
                     Error->ET_ParserNoCode);
bool Parser::parseExpressionImpl(VariantValue *Value) {
 
   llvm_unreachable("Unknown token kind.");
 }
 
 Parser::Parser(CodeTokenizer *Tokenizer, Sema *S,
                Diagnostics *Error)
     : Tokenizer(Tokenizer), S(S), Error(Error) {}
 
-class RegistrySema : public Parser::Sema {
-public:
-  virtual ~RegistrySema() {}
-  llvm::Optional<MatcherCtor> lookupMatcherCtor(StringRef MatcherName,
-                                                const SourceRange &NameRange,
-                                                Diagnostics *Error) {
-    return Registry::lookupMatcherCtor(MatcherName, NameRange, Error);
-  }
-  VariantMatcher actOnMatcherExpression(MatcherCtor Ctor,
-                                        const SourceRange &NameRange,
-                                        StringRef BindID,
-                                        ArrayRef<ParserValue> Args,
-                                        Diagnostics *Error) {
-    if (BindID.empty()) {
-      return Registry::constructMatcher(Ctor, NameRange, Args, Error);
-    } else {
-      return Registry::constructBoundMatcher(Ctor, NameRange, BindID, Args,
-                                             Error);
-    }
-  }
-};
+Parser::RegistrySema::~RegistrySema() {}
+
+llvm::Optional<MatcherCtor>
+Parser::RegistrySema::lookupMatcherCtor(StringRef MatcherName) {
+  return Registry::lookupMatcherCtor(MatcherName);
+
+VariantMatcher Parser::RegistrySema::actOnMatcherExpression(
+    MatcherCtor Ctor, const SourceRange &NameRange, StringRef BindID,
+    ArrayRef<ParserValue> Args, Diagnostics *Error) {
 
 bool Parser::parseExpression(StringRef Code, VariantValue *Value,
                              Diagnostics *Error) {
   RegistrySema S;
   return parseExpression(Code, &S, Value, Error);
 }
 
 bool Parser::parseExpression(StringRef Code, Sema *S,

lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::~RegistryMaps() {
   }
 }
 
 static llvm::ManagedStatic<RegistryMaps> RegistryData;
 
 } // anonymous namespace
 
 // static
-llvm::Optional<MatcherCtor>
-Registry::lookupMatcherCtor(StringRef MatcherName, const SourceRange &NameRange,
-                            Diagnostics *Error) {
+llvm::Optional<MatcherCtor> Registry::lookupMatcherCtor(StringRef MatcherName) {
   ConstructorMap::const_iterator it =
       RegistryData->constructors().find(MatcherName);
-  if (it == RegistryData->constructors().end()) {
-    Error->addError(NameRange, Error->ET_RegistryNotFound) << MatcherName;
-    return llvm::Optional<MatcherCtor>();
-  }
-
-  return it->second;
+  return it == RegistryData->constructors().end()
+             ? llvm::Optional<MatcherCtor>()
+             : it->second;
 }
 
 namespace {
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
                               const std::set<ASTNodeKind> &KS) {
   unsigned Count = 0;
   for (std::set<ASTNodeKind>::const_iterator I = KS.begin(), E = KS.end();

unittests/ASTMatchers/Dynamic/ParserTest.cpp

public:
   void parse(StringRef Code) {
     Diagnostics Error;
     VariantValue Value;
     Parser::parseExpression(Code, this, &Value, &Error);
     Values.push_back(Value);
     Errors.push_back(Error.toStringFull());
   }
 
-  llvm::Optional<MatcherCtor> lookupMatcherCtor(StringRef MatcherName,
-                                                const SourceRange &NameRange,
-                                                Diagnostics *Error) {
+  llvm::Optional<MatcherCtor> lookupMatcherCtor(StringRef MatcherName) {
     const ExpectedMatchersTy::value_type *Matcher =
         &*ExpectedMatchers.find(MatcherName);
     return reinterpret_cast<MatcherCtor>(Matcher);
   }
 
   VariantMatcher actOnMatcherExpression(MatcherCtor Ctor,
                                         const SourceRange &NameRange,
                                         StringRef BindID,
TEST(ParserTest, FullParserTest) {
   llvm::Optional<DynTypedMatcher> HasParameter(Parser::parseMatcherExpression(
       "functionDecl(hasParameter(1, hasName(\"x\")))", &Error));
   EXPECT_EQ("", Error.toStringFull());
   M = HasParameter->unconditionalConvertTo<Decl>();
 
   EXPECT_TRUE(matches("void f(int a, int x);", M));
   EXPECT_FALSE(matches("void f(int x, int a);", M));
 
+  // Test named values.
+  struct NamedSema : public Parser::RegistrySema {
+   public:
+    virtual VariantValue getNamedValue(StringRef Name) {
+      if (Name == "nameX")
+        return std::string("x");
+      if (Name == "param0")
+        return VariantMatcher::SingleMatcher(hasParameter(0, hasName("a")));
+      return VariantValue();
+    }
+  };
+  NamedSema Sema;
+  llvm::Optional<DynTypedMatcher> HasParameterWithNamedValues(
+      Parser::parseMatcherExpression(
+          "functionDecl(param0, hasParameter(1, hasName(nameX)))", &Sema,
+          &Error));
+  EXPECT_EQ("", Error.toStringFull());
+  M = HasParameterWithNamedValues->unconditionalConvertTo<Decl>();
+
+  EXPECT_TRUE(matches("void f(int a, int x);", M));
+  EXPECT_FALSE(matches("void f(int x, int a);", M));
+
+
   EXPECT_TRUE(!Parser::parseMatcherExpression(
                    "hasInitializer(\n    binaryOperator(hasLHS(\"A\")))",
                    &Error).hasValue());
   EXPECT_EQ("1:1: Error parsing argument 1 for matcher hasInitializer.\n"
             "2:5: Error parsing argument 1 for matcher binaryOperator.\n"
             "2:20: Error building matcher hasLHS.\n"
             "2:27: Incorrect type for arg 1. "
             "(Expected = Matcher<Expr>) != (Actual = String)",
TEST(ParserTest, Errors) {
   EXPECT_EQ(
       "1:5: Error parsing matcher. Found token <123> while looking for '('.",
       ParseWithError("Foo 123"));
   EXPECT_EQ(
       "1:1: Matcher not found: Foo\n"
       "1:9: Error parsing matcher. Found token <123> while looking for ','.",
       ParseWithError("Foo(\"A\" 123)"));
   EXPECT_EQ(
+      "1:1: Error parsing argument 1 for matcher stmt.\n"
+      "1:6: Value not found: someValue",
+      ParseWithError("stmt(someValue)"));
+  EXPECT_EQ(
       "1:1: Matcher not found: Foo\n"
       "1:4: Error parsing matcher. Found end-of-code while looking for ')'.",
       ParseWithError("Foo("));
   EXPECT_EQ("1:1: End of code found while looking for token.",
             ParseWithError(""));
   EXPECT_EQ("Input value is not a matcher expression.",
             ParseMatcherWithError("\"A\""));
   EXPECT_EQ("1:1: Matcher not found: Foo\n"

unittests/ASTMatchers/Dynamic/RegistryTest.cpp

public:
   std::vector<ParserValue> Args(const VariantValue &Arg1,
                                 const VariantValue &Arg2) {
     std::vector<ParserValue> Out(2);
     Out[0].Value = Arg1;
     Out[1].Value = Arg2;
     return Out;
   }
 
-  llvm::Optional<MatcherCtor> lookupMatcherCtor(StringRef MatcherName,
-                                                Diagnostics *Error = 0) {
-    Diagnostics DummyError;
-    if (!Error) Error = &DummyError;
-    llvm::Optional<MatcherCtor> Ctor =
-        Registry::lookupMatcherCtor(MatcherName, SourceRange(), Error);
-    EXPECT_EQ("", DummyError.toStringFull());
-    return Ctor;
+  llvm::Optional<MatcherCtor> lookupMatcherCtor(StringRef MatcherName) {
+    return Registry::lookupMatcherCtor(MatcherName);
   }
 
   VariantMatcher constructMatcher(StringRef MatcherName,
                                   Diagnostics *Error = NULL) {
     Diagnostics DummyError;
     if (!Error) Error = &DummyError;
-    llvm::Optional<MatcherCtor> Ctor = lookupMatcherCtor(MatcherName, Error);
+    llvm::Optional<MatcherCtor> Ctor = lookupMatcherCtor(MatcherName);
     VariantMatcher Out;
     if (Ctor)
       Out = Registry::constructMatcher(*Ctor, SourceRange(), Args(), Error);
     EXPECT_EQ("", DummyError.toStringFull());
     return Out;
   }
 
   VariantMatcher constructMatcher(StringRef MatcherName,
                                   const VariantValue &Arg1,
                                   Diagnostics *Error = NULL) {
     Diagnostics DummyError;
     if (!Error) Error = &DummyError;
-    llvm::Optional<MatcherCtor> Ctor = lookupMatcherCtor(MatcherName, Error);
+    llvm::Optional<MatcherCtor> Ctor = lookupMatcherCtor(MatcherName);
     VariantMatcher Out;
     if (Ctor)
       Out = Registry::constructMatcher(*Ctor, SourceRange(), Args(Arg1), Error);
     EXPECT_EQ("", DummyError.toStringFull()) << MatcherName;
     return Out;
   }
 
   VariantMatcher constructMatcher(StringRef MatcherName,
                                   const VariantValue &Arg1,
                                   const VariantValue &Arg2,
                                   Diagnostics *Error = NULL) {
     Diagnostics DummyError;
     if (!Error) Error = &DummyError;
-    llvm::Optional<MatcherCtor> Ctor = lookupMatcherCtor(MatcherName, Error);
+    llvm::Optional<MatcherCtor> Ctor = lookupMatcherCtor(MatcherName);
     VariantMatcher Out;
     if (Ctor)
       Out = Registry::constructMatcher(*Ctor, SourceRange(), Args(Arg1, Arg2),
                                        Error);
     EXPECT_EQ("", DummyError.toStringFull());
     return Out;
   }
 

unittests/ASTMatchers/Dynamic/VariantValueTest.cpp

 
 TEST(VariantValueTest, Unsigned) {
   const unsigned kUnsigned = 17;
   VariantValue Value = kUnsigned;
 
   EXPECT_TRUE(Value.isUnsigned());
   EXPECT_EQ(kUnsigned, Value.getUnsigned());
 
+  EXPECT_TRUE(Value.hasValue());
   EXPECT_FALSE(Value.isString());
   EXPECT_FALSE(Value.isMatcher());
 }
 
 TEST(VariantValueTest, String) {
   const ::std::string kString = "string";
   VariantValue Value = kString;
 
   EXPECT_TRUE(Value.isString());
   EXPECT_EQ(kString, Value.getString());
   EXPECT_EQ("String", Value.getTypeAsString());
 
+  EXPECT_TRUE(Value.hasValue());
   EXPECT_FALSE(Value.isUnsigned());
   EXPECT_FALSE(Value.isMatcher());
 }
 
 TEST(VariantValueTest, DynTypedMatcher) {
   VariantValue Value = VariantMatcher::SingleMatcher(stmt());
 
+  EXPECT_TRUE(Value.hasValue());
   EXPECT_FALSE(Value.isUnsigned());
   EXPECT_FALSE(Value.isString());
 
   EXPECT_TRUE(Value.isMatcher());
   EXPECT_FALSE(Value.getMatcher().hasTypedMatcher<Decl>());
   EXPECT_TRUE(Value.getMatcher().hasTypedMatcher<UnaryOperator>());
   EXPECT_EQ("Matcher<Stmt>", Value.getTypeAsString());
 
TEST(VariantValueTest, DynTypedMatcher) {
   EXPECT_FALSE(Value.getMatcher().hasTypedMatcher<GotoStmt>());
   EXPECT_EQ("Matcher<Expr>", Value.getTypeAsString());
 }
 
 TEST(VariantValueTest, Assignment) {
   VariantValue Value = std::string("A");
   EXPECT_TRUE(Value.isString());
   EXPECT_EQ("A", Value.getString());
+  EXPECT_TRUE(Value.hasValue());
   EXPECT_FALSE(Value.isUnsigned());
   EXPECT_FALSE(Value.isMatcher());
   EXPECT_EQ("String", Value.getTypeAsString());
 
   Value = VariantMatcher::SingleMatcher(recordDecl());
+  EXPECT_TRUE(Value.hasValue());
   EXPECT_FALSE(Value.isUnsigned());
   EXPECT_FALSE(Value.isString());
   EXPECT_TRUE(Value.isMatcher());
   EXPECT_TRUE(Value.getMatcher().hasTypedMatcher<Decl>());
   EXPECT_FALSE(Value.getMatcher().hasTypedMatcher<UnaryOperator>());
   EXPECT_EQ("Matcher<Decl>", Value.getTypeAsString());
 
   Value = 17;
   EXPECT_TRUE(Value.isUnsigned());
   EXPECT_EQ(17U, Value.getUnsigned());
+  EXPECT_TRUE(Value.hasValue());
   EXPECT_FALSE(Value.isMatcher());
   EXPECT_FALSE(Value.isString());
 
   Value = VariantValue();
+  EXPECT_FALSE(Value.hasValue());
   EXPECT_FALSE(Value.isUnsigned());
   EXPECT_FALSE(Value.isString());
   EXPECT_FALSE(Value.isMatcher());
   EXPECT_EQ("Nothing", Value.getTypeAsString());
 }
 
+TEST(VariantValueTest, ImplicitBool) {
+  VariantValue Value;
+  bool IfTrue = false;
+  if (Value) {
+    IfTrue = true;
+  }
+  EXPECT_FALSE(IfTrue);
+  EXPECT_TRUE(!Value);
+
+  Value = std::string();
+  IfTrue = false;
+  if (Value) {
+    IfTrue = true;
+  }
+  EXPECT_TRUE(IfTrue);
+  EXPECT_FALSE(!Value);
+}
+
 TEST(VariantValueTest, Matcher) {
   EXPECT_TRUE(matches("class X {};", VariantValue(VariantMatcher::SingleMatcher(
                                                       recordDecl(hasName("X"))))
                                          .getMatcher()
                                          .getTypedMatcher<Decl>()));
   EXPECT_TRUE(
       matches("int x;", VariantValue(VariantMatcher::SingleMatcher(varDecl()))
                             .getMatcher()