Allow newlines in AST Matchers in clang-query files

[aaron.ballman]

clang-tools-extra/clang-query/Query.cpp

103–104

Oh, I see the existing text is already inconsistent with naming conventions. You can ignore the renaming comments if you want -- we can always fix up the names post-commit -- or you can fix up this name (which is the only oddball in the file).

106

firstLine should be FirstLine and I have a slight preference for StringRef instead of auto (but don't insist because Lines is so nearby and clear).

111

MaxLength and perhaps this type should be unsigned or size_t rather than int to avoid the unnecessary type conversion?

clang-tools-extra/clang-query/QueryParser.cpp

30

Comment missing a trailing full stop.

132

Definitely prefer using StringRef here -- I had no idea drop_while() returned this.

140

auto -> StringRef

243–246

Would it make more sense to have Query with another constructor to hold the remaining content, and then thread that through LetQuery, MatchQuery, etc? Whether there is remaining content or not seems like a pretty important property of the query, so it seems reasonable to let people construct with that information rather than set it after the fact.

If not, these should be auto * rather than deducing the pointer.

clang/lib/ASTMatchers/Dynamic/Parser.cpp

301

Add a full stop to the comment.

361

Spurious semi-colon?

[steveire]

Update

[steveire]

clang-tools-extra/clang-query/Query.cpp

111

This is where auto makes most sense, but I made it unsigned instead.

[steveire]

clang-tools-extra/clang-query/QueryParser.cpp

243–246

I think it is set after the fact because it is set in endQuery().

[aaron.ballman]

LGTM aside from some auto -> auto * nits.

clang-tools-extra/clang-query/QueryParser.cpp

243–246

Okay, that's reasonable. Thanks! Can you switch to auto * for these cases?

[gribozavr2]

clang/include/clang/ASTMatchers/Dynamic/Parser.h

167

Sorry for the late review comment, but I'd really appreciate if you could update the doc comments to explain why MatcherCode is taken by reference, and how it is mutated.

[ymandel]

clang/include/clang/ASTMatchers/Dynamic/Parser.h

167

+1 This broke our build because were passing a temporary (foo.trim()). An explanation at least would be appreciated.

clang-tools-extra/clang-query/Query.h

struct Query : llvm::RefCountedBase<Query> {
   Query(QueryKind Kind) : Kind(Kind) {}
   virtual ~Query();
 
   /// Perform the query on \p QS and print output to \p OS.
   ///
   /// \return false if an error occurs, otherwise return true.
   virtual bool run(llvm::raw_ostream &OS, QuerySession &QS) const = 0;
 
+  StringRef RemainingContent;
   const QueryKind Kind;
 };
 
 typedef llvm::IntrusiveRefCntPtr<Query> QueryRef;
 
 /// Any query which resulted in a parse error.  The error message is in ErrStr.
 struct InvalidQuery : Query {
   InvalidQuery(const Twine &ErrStr) : Query(QK_Invalid), ErrStr(ErrStr.str()) {}

clang-tools-extra/clang-query/Query.cpp

for (auto &AST : QS.ASTs) {
     }
     CollectBoundNodes Collect(Matches);
     if (!Finder.addDynamicMatcher(MaybeBoundMatcher, &Collect)) {
       OS << "Not a valid top-level matcher.\n";
       return false;
     }
     Finder.matchAST(AST->getASTContext());
 
     if (QS.PrintMatcher) {
-      std::string prefixText = "Matcher: ";
-      OS << "\n  " << prefixText << Source << "\n";
-      OS << "  " << std::string(prefixText.size() + Source.size(), '=') << '\n';
+      SmallVector<StringRef, 4> Lines;
+      Source.split(Lines, "\n");
+      auto FirstLine = Lines[0];
+      Lines.erase(Lines.begin(), Lines.begin() + 1);
+      while (!Lines.empty() && Lines.back().empty()) {
+        Lines.resize(Lines.size() - 1);
+      }
+      unsigned MaxLength = FirstLine.size();
+      std::string PrefixText = "Matcher: ";
+      OS << "\n  " << PrefixText << FirstLine;
+
+      for (auto Line : Lines) {
+        OS << "\n" << std::string(PrefixText.size() + 2, ' ') << Line;
+        MaxLength = std::max<int>(MaxLength, Line.rtrim().size());
+      }
+
+      OS << "\n"
+         << "  " << std::string(PrefixText.size() + MaxLength, '=') << "\n\n";
     }
 
     for (auto MI = Matches.begin(), ME = Matches.end(); MI != ME; ++MI) {
       OS << "\nMatch #" << ++MatchCount << ":\n\n";
 
       for (auto BI = MI->getMap().begin(), BE = MI->getMap().end(); BI != BE;
            ++BI) {
         if (QS.DiagOutput) {

clang-tools-extra/clang-query/QueryParser.cpp

 namespace clang {
 namespace query {
 
 // Lex any amount of whitespace followed by a "word" (any sequence of
 // non-whitespace characters) from the start of region [Begin,End).  If no word
 // is found before End, return StringRef().  Begin is adjusted to exclude the
 // lexed region.
 StringRef QueryParser::lexWord() {
-  Line = Line.ltrim();
+  Line = Line.drop_while([this](char c) {
+    // Don't trim newlines.
+    return StringRef(" \t\v\f\r").contains(c);
+  });
 
   if (Line.empty())
     // Even though the Line is empty, it contains a pointer and
     // a (zero) length. The pointer is used in the LexOrCompleteWord
     // code completion.
     return Line;
 
-  if (Line.front() == '#') {
-    Line = {};
-    return StringRef();
-  }
+  StringRef Word;
+  if (Line.front() == '#')
+    Word = Line.substr(0, 1);
+  else
+    Word = Line.take_until(isWhitespace);
 
-  StringRef Word = Line.take_until(isWhitespace);
   Line = Line.drop_front(Word.size());
   return Word;
 }
 
 // This is the StringSwitch-alike used by lexOrCompleteWord below. See that
 // function for details.
 template <typename T> struct QueryParser::LexOrCompleteWord {
   StringRef Word;
template <typename QueryType> QueryRef QueryParser::parseSetOutputKind() {
   case OK_Print:
     return new QueryType(&QuerySession::PrintOutput);
   }
 
   llvm_unreachable("Invalid output kind");
 }
 
 QueryRef QueryParser::endQuery(QueryRef Q) {
-  const StringRef Extra = Line;
-  if (!lexWord().empty())
+  StringRef Extra = Line;
+  StringRef ExtraTrimmed = Extra.drop_while(
+      [](char c) { return StringRef(" \t\v\f\r").contains(c); });
+
+  if ((!ExtraTrimmed.empty() && ExtraTrimmed[0] == '\n') ||
+      (ExtraTrimmed.size() >= 2 && ExtraTrimmed[0] == '\r' &&
+       ExtraTrimmed[1] == '\n'))
+    Q->RemainingContent = Extra;
+  else {
+    StringRef TrailingWord = lexWord();
+    if (TrailingWord.front() == '#') {
+      Line = Line.drop_until([](char c) { return c == '\n'; });
+      Line = Line.drop_front();
+      return endQuery(Q);
+    }
+    if (!TrailingWord.empty()) {
       return new InvalidQuery("unexpected extra input: '" + Extra + "'");
+    }
+  }
   return Q;
 }
 
 namespace {
 
 enum ParsedQueryKind {
   PQK_Invalid,
   PQK_Comment,
ParsedQueryKind QKind = LexOrCompleteWord<ParsedQueryKind>(this, CommandStr)
                               .Case("enable", PQK_Enable)
                               .Case("disable", PQK_Disable)
                               .Case("unlet", PQK_Unlet)
                               .Default(PQK_Invalid);
 
   switch (QKind) {
   case PQK_Comment:
   case PQK_NoOp:
+    Line = Line.drop_until([](char c) { return c == '\n'; });
+    Line = Line.drop_front();
+    if (Line.empty())
       return new NoOpQuery;
+    return doParse();
 
   case PQK_Help:
     return endQuery(new HelpQuery);
 
   case PQK_Quit:
     return endQuery(new QuitQuery);
 
   case PQK_Let: {
     StringRef Name = lexWord();
 
     if (Name.empty())
       return new InvalidQuery("expected variable name");
 
     if (CompletionPos)
       return completeMatcherExpression();
 
     Diagnostics Diag;
     ast_matchers::dynamic::VariantValue Value;
     if (!Parser::parseExpression(Line, nullptr, &QS.NamedValues, &Value,
                                  &Diag)) {
       return makeInvalidQueryFromDiagnostics(Diag);
     }
 
-    return new LetQuery(Name, Value);
+    auto *Q = new LetQuery(Name, Value);
+    Q->RemainingContent = Line;
+    return Q;
   }
 
   case PQK_Match: {
     if (CompletionPos)
       return completeMatcherExpression();
 
     Diagnostics Diag;
     auto MatcherSource = Line.trim();
+    auto OrigMatcherSource = MatcherSource;
     Optional<DynTypedMatcher> Matcher = Parser::parseMatcherExpression(
         MatcherSource, nullptr, &QS.NamedValues, &Diag);
     if (!Matcher) {
       return makeInvalidQueryFromDiagnostics(Diag);
     }
-    return new MatchQuery(MatcherSource, *Matcher);
+    auto ActualSource = OrigMatcherSource.slice(0, OrigMatcherSource.size() -
+                                                       MatcherSource.size());
+    auto *Q = new MatchQuery(ActualSource, *Matcher);
+    Q->RemainingContent = MatcherSource;
+    return Q;
   }
 
   case PQK_Set: {
     StringRef VarStr;
     ParsedQueryVariable Var =
         LexOrCompleteWord<ParsedQueryVariable>(this, VarStr)
             .Case("output", PQV_Output)
             .Case("bind-root", PQV_BindRoot)

clang-tools-extra/clang-query/tool/ClangQuery.cpp

 
 bool runCommandsInFile(const char *ExeName, std::string const &FileName,
                        QuerySession &QS) {
   std::ifstream Input(FileName.c_str());
   if (!Input.is_open()) {
     llvm::errs() << ExeName << ": cannot open " << FileName << "\n";
     return 1;
   }
-  while (Input.good()) {
-    std::string Line;
-    std::getline(Input, Line);
 
-    QueryRef Q = QueryParser::parse(Line, QS);
+  std::string FileContent((std::istreambuf_iterator<char>(Input)),
+                          std::istreambuf_iterator<char>());
+
+  StringRef FileContentRef(FileContent);
+  while (!FileContentRef.empty()) {
+    QueryRef Q = QueryParser::parse(FileContentRef, QS);
     if (!Q->run(llvm::outs(), QS))
       return true;
+    FileContentRef = Q->RemainingContent;
   }
   return false;
 }
 
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
   CommonOptionsParser OptionsParser(argc, argv, ClangQueryCategory);

clang-tools-extra/unittests/clang-query/QueryParserTest.cpp

TEST_F(QueryParserTest, Complete) {
   EXPECT_EQ("atch ", Comps[0].TypedText);
   EXPECT_EQ("match", Comps[0].DisplayText);
 
   Comps = QueryParser::complete("l", 1, QS);
   ASSERT_EQ(1u, Comps.size());
   EXPECT_EQ("et ", Comps[0].TypedText);
   EXPECT_EQ("let", Comps[0].DisplayText);
 }
+
+TEST_F(QueryParserTest, Multiline) {
+
+  // Single string with multiple commands
+  QueryRef Q = parse(R"matcher(
+set bind-root false
+set output dump
+    )matcher");
+
+  ASSERT_TRUE(isa<SetQuery<bool>>(Q));
+
+  Q = parse(Q->RemainingContent);
+  ASSERT_TRUE(isa<SetExclusiveOutputQuery>(Q));
+
+  // Missing newline
+  Q = parse(R"matcher(
+set bind-root false set output dump
+    )matcher");
+
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("unexpected extra input: ' set output dump\n    '",
+            cast<InvalidQuery>(Q)->ErrStr);
+
+  // Commands which do their own parsing
+  Q = parse(R"matcher(
+let fn functionDecl(hasName("foo"))
+match callExpr(callee(functionDecl()))
+    )matcher");
+
+  ASSERT_TRUE(isa<LetQuery>(Q));
+
+  Q = parse(Q->RemainingContent);
+  ASSERT_TRUE(isa<MatchQuery>(Q));
+
+  // Multi-line matcher
+  Q = parse(R"matcher(
+match callExpr(callee(
+    functionDecl().bind("fn")
+    ))
+
+    )matcher");
+
+  ASSERT_TRUE(isa<MatchQuery>(Q));
+
+  // Comment locations
+  Q = parse(R"matcher(
+#nospacecomment
+# Leading comment
+match callExpr ( # Trailing comment
+            # Comment alone on line
+
+            callee(
+            functionDecl(
+            ).bind(
+            "fn"
+            )
+            )) # Comment trailing close
+# Comment after match
+    )matcher");
+
+  ASSERT_TRUE(isa<MatchQuery>(Q));
+
+  // \r\n
+  Q = parse("set bind-root false\r\nset output dump");
+
+  ASSERT_TRUE(isa<SetQuery<bool>>(Q));
+
+  Q = parse(Q->RemainingContent);
+  ASSERT_TRUE(isa<SetExclusiveOutputQuery>(Q));
+
+  // Leading and trailing space in lines
+  Q = parse("  set bind-root false  \r\n  set output dump  ");
+
+  ASSERT_TRUE(isa<SetQuery<bool>>(Q));
+
+  Q = parse(Q->RemainingContent);
+  ASSERT_TRUE(isa<SetExclusiveOutputQuery>(Q));
+
+  // Incomplete commands
+  Q = parse("set\nbind-root false");
+
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("expected variable name", cast<InvalidQuery>(Q)->ErrStr);
+
+  Q = parse("set bind-root\nfalse");
+
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("expected 'true' or 'false', got ''",
+            cast<InvalidQuery>(Q)->ErrStr);
+
+  Q = parse(R"matcher(
+match callExpr
+(
+)
+    )matcher");
+
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("1:9: Error parsing matcher. Found token <NewLine> "
+            "while looking for '('.",
+            cast<InvalidQuery>(Q)->ErrStr);
+}

clang/include/clang/ASTMatchers/Dynamic/Parser.h

public:
   ///   the dictionary for the <NamedValue> rule of the grammar.
   ///   If null, it is ignored.
   ///
   /// \return The matcher object constructed by the processor, or an empty
   ///   Optional if an error occurred. In that case, \c Error will contain a
   ///   description of the error.
   ///   The caller takes ownership of the DynTypedMatcher object returned.
   static llvm::Optional<DynTypedMatcher>
-  parseMatcherExpression(StringRef MatcherCode, Sema *S,
-                         const NamedValueMap *NamedValues,
-                         Diagnostics *Error);
+  parseMatcherExpression(StringRef &MatcherCode, Sema *S,
+                         const NamedValueMap *NamedValues, Diagnostics *Error);
   static llvm::Optional<DynTypedMatcher>
-  parseMatcherExpression(StringRef MatcherCode, Sema *S,
-                         Diagnostics *Error) {
+  parseMatcherExpression(StringRef &MatcherCode, Sema *S, Diagnostics *Error) {
     return parseMatcherExpression(MatcherCode, S, nullptr, Error);
   }
   static llvm::Optional<DynTypedMatcher>
-  parseMatcherExpression(StringRef MatcherCode, Diagnostics *Error) {
+  parseMatcherExpression(StringRef &MatcherCode, Diagnostics *Error) {
     return parseMatcherExpression(MatcherCode, nullptr, Error);
   }
 
   /// Parse an expression.
   ///
   /// Parses any expression supported by this parser. In general, the
   /// \c parseMatcherExpression function is a better approach to get a matcher
   /// object.
   ///
   /// \param S The Sema instance that will help the parser
   ///   construct the matchers. If null, it uses the default registry.
   ///
   /// \param NamedValues A map of precomputed named values.  This provides
   ///   the dictionary for the <NamedValue> rule of the grammar.
   ///   If null, it is ignored.
-  static bool parseExpression(StringRef Code, Sema *S,
+  static bool parseExpression(StringRef &Code, Sema *S,
                               const NamedValueMap *NamedValues,
                               VariantValue *Value, Diagnostics *Error);
-  static bool parseExpression(StringRef Code, Sema *S,
-                              VariantValue *Value, Diagnostics *Error) {
+  static bool parseExpression(StringRef &Code, Sema *S, VariantValue *Value,
+                              Diagnostics *Error) {
     return parseExpression(Code, S, nullptr, Value, Error);
   }
-  static bool parseExpression(StringRef Code, VariantValue *Value,
+  static bool parseExpression(StringRef &Code, VariantValue *Value,
                               Diagnostics *Error) {
     return parseExpression(Code, nullptr, Value, Error);
   }
 
   /// Complete an expression at the given offset.
   ///
   /// \param S The Sema instance that will help the parser
   ///   construct the matchers. If null, it uses the default registry.
   ///
   /// \param NamedValues A map of precomputed named values.  This provides
   ///   the dictionary for the <NamedValue> rule of the grammar.
   ///   If null, it is ignored.
   ///
   /// \return The list of completions, which may be empty if there are no
   /// available completions or if an error occurred.
   static std::vector<MatcherCompletion>
-  completeExpression(StringRef Code, unsigned CompletionOffset, Sema *S,
+  completeExpression(StringRef &Code, unsigned CompletionOffset, Sema *S,
                      const NamedValueMap *NamedValues);
   static std::vector<MatcherCompletion>
-  completeExpression(StringRef Code, unsigned CompletionOffset, Sema *S) {
+  completeExpression(StringRef &Code, unsigned CompletionOffset, Sema *S) {
     return completeExpression(Code, CompletionOffset, S, nullptr);
   }
   static std::vector<MatcherCompletion>
-  completeExpression(StringRef Code, unsigned CompletionOffset) {
+  completeExpression(StringRef &Code, unsigned CompletionOffset) {
     return completeExpression(Code, CompletionOffset, nullptr);
   }
 
 private:
   class CodeTokenizer;
   struct ScopedContextEntry;
   struct TokenInfo;
 

clang/lib/ASTMatchers/Dynamic/Parser.cpp

 namespace ast_matchers {
 namespace dynamic {
 
 /// Simple structure to hold information for one token from the parser.
 struct Parser::TokenInfo {
   /// Different possible tokens.
   enum TokenKind {
     TK_Eof,
+    TK_NewLine,
     TK_OpenParen,
     TK_CloseParen,
     TK_Comma,
     TK_Period,
     TK_Literal,
     TK_Ident,
     TK_InvalidChar,
     TK_Error,
struct Parser::TokenInfo {
   VariantValue Value;
 };
 
 const char* const Parser::TokenInfo::ID_Bind = "bind";
 
 /// Simple tokenizer for the parser.
 class Parser::CodeTokenizer {
 public:
-  explicit CodeTokenizer(StringRef MatcherCode, Diagnostics *Error)
+  explicit CodeTokenizer(StringRef &MatcherCode, Diagnostics *Error)
       : Code(MatcherCode), StartOfLine(MatcherCode), Error(Error) {
     NextToken = getNextToken();
   }
 
-  CodeTokenizer(StringRef MatcherCode, Diagnostics *Error,
+  CodeTokenizer(StringRef &MatcherCode, Diagnostics *Error,
                 unsigned CodeCompletionOffset)
       : Code(MatcherCode), StartOfLine(MatcherCode), Error(Error),
         CodeCompletionLocation(MatcherCode.data() + CodeCompletionOffset) {
     NextToken = getNextToken();
   }
 
   /// Returns but doesn't consume the next token.
   const TokenInfo &peekNextToken() const { return NextToken; }
 
   /// Consumes and returns the next token.
   TokenInfo consumeNextToken() {
     TokenInfo ThisToken = NextToken;
     NextToken = getNextToken();
     return ThisToken;
   }
 
+  TokenInfo SkipNewlines() {
+    while (NextToken.Kind == TokenInfo::TK_NewLine)
+      NextToken = getNextToken();
+    return NextToken;
+  }
+
+  TokenInfo consumeNextTokenIgnoreNewlines() {
+    SkipNewlines();
+    if (NextToken.Kind == TokenInfo::TK_Eof)
+      return NextToken;
+    return consumeNextToken();
+  }
+
   TokenInfo::TokenKind nextTokenKind() const { return NextToken.Kind; }
 
 private:
   TokenInfo getNextToken() {
     consumeWhitespace();
     TokenInfo Result;
     Result.Range.Start = currentLocation();
 
     if (CodeCompletionLocation && CodeCompletionLocation <= Code.data()) {
       Result.Kind = TokenInfo::TK_CodeCompletion;
       Result.Text = StringRef(CodeCompletionLocation, 0);
       CodeCompletionLocation = nullptr;
       return Result;
     }
 
     if (Code.empty()) {
       Result.Kind = TokenInfo::TK_Eof;
       Result.Text = "";
       return Result;
     }
 
     switch (Code[0]) {
     case '#':
-      Result.Kind = TokenInfo::TK_Eof;
-      Result.Text = "";
-      return Result;
+      Code = Code.drop_until([](char c) { return c == '\n'; });
+      return getNextToken();
     case ',':
       Result.Kind = TokenInfo::TK_Comma;
       Result.Text = Code.substr(0, 1);
       Code = Code.drop_front();
       break;
     case '.':
       Result.Kind = TokenInfo::TK_Period;
       Result.Text = Code.substr(0, 1);
       Code = Code.drop_front();
       break;
+    case '\n':
+      ++Line;
+      StartOfLine = Code.drop_front();
+      Result.Kind = TokenInfo::TK_NewLine;
+      Result.Text = Code.substr(0, 1);
+      Code = Code.drop_front();
+      break;
     case '(':
       Result.Kind = TokenInfo::TK_OpenParen;
       Result.Text = Code.substr(0, 1);
       Code = Code.drop_front();
       break;
     case ')':
       Result.Kind = TokenInfo::TK_CloseParen;
       Result.Text = Code.substr(0, 1);
void consumeStringLiteral(TokenInfo *Result) {
     Range.Start = Result->Range.Start;
     Range.End = currentLocation();
     Error->addError(Range, Error->ET_ParserStringError) << ErrorText;
     Result->Kind = TokenInfo::TK_Error;
   }
 
   /// Consume all leading whitespace from \c Code.
   void consumeWhitespace() {
-    while (!Code.empty() && isWhitespace(Code[0])) {
-      if (Code[0] == '\n') {
-        ++Line;
-        StartOfLine = Code.drop_front();
-      }
-      Code = Code.drop_front();
-    }
+    Code = Code.drop_while([this](char c) {
+      // Don't trim newlines.
+      return StringRef(" \t\v\f\r").contains(c);
+    });
   }
 
   SourceLocation currentLocation() {
     SourceLocation Location;
     Location.Line = Line;
     Location.Column = Code.data() - StartOfLine.data() + 1;
     return Location;
   }
 
-  StringRef Code;
+  StringRef &Code;
   StringRef StartOfLine;
   unsigned Line = 1;
   Diagnostics *Error;
   TokenInfo NextToken;
   const char *CodeCompletionLocation = nullptr;
 };
 
 Parser::Sema::~Sema() = default;

 /// 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_NewLine) {
+    Error->addError(Tokenizer->peekNextToken().Range,
+                    Error->ET_ParserNoOpenParen)
+        << "NewLine";
+    return false;
+  }
+
   if (Tokenizer->nextTokenKind() != TokenInfo::TK_OpenParen) {
     // Parse as a named value.
     if (const VariantValue NamedValue =
             NamedValues ? NamedValues->lookup(NameToken.Text)
                         : VariantValue()) {
 
       if (Tokenizer->nextTokenKind() != TokenInfo::TK_Period) {
         *Value = NamedValue;
if (const VariantValue NamedValue =
         }
       }
       return false;
     }
     // 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_NewLine ||
          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.
   }
 
+  Tokenizer->SkipNewlines();
+
   // Parse as a matcher expression.
   return parseMatcherExpressionImpl(NameToken, Value);
 }
 
 bool Parser::parseBindID(std::string &BindID) {
   // Parse .bind("foo")
   assert(Tokenizer->peekNextToken().Kind == TokenInfo::TK_Period);
   Tokenizer->consumeNextToken(); // consume the period.
   const TokenInfo BindToken = Tokenizer->consumeNextToken();
   if (BindToken.Kind == TokenInfo::TK_CodeCompletion) {
     addCompletion(BindToken, MatcherCompletion("bind(\"", "bind", 1));
     return false;
   }
 
   const TokenInfo OpenToken = Tokenizer->consumeNextToken();
-  const TokenInfo IDToken = Tokenizer->consumeNextToken();
-  const TokenInfo CloseToken = Tokenizer->consumeNextToken();
+  const TokenInfo IDToken = Tokenizer->consumeNextTokenIgnoreNewlines();
+  const TokenInfo CloseToken = Tokenizer->consumeNextTokenIgnoreNewlines();
 
   // TODO: We could use different error codes for each/some to be more
   //       explicit about the syntax error.
   if (BindToken.Kind != TokenInfo::TK_Ident ||
       BindToken.Text != TokenInfo::ID_Bind) {
     Error->addError(BindToken.Range, Error->ET_ParserMalformedBindExpr);
     return false;
   }
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;
 
+  Tokenizer->SkipNewlines();
+
   {
     ScopedContextEntry SCE(this, Ctor ? *Ctor : nullptr);
 
     while (Tokenizer->nextTokenKind() != TokenInfo::TK_Eof) {
       if (Tokenizer->nextTokenKind() == TokenInfo::TK_CloseParen) {
         // End of args.
         EndToken = Tokenizer->consumeNextToken();
         break;
       }
       if (!Args.empty()) {
         // We must find a , token to continue.
         const TokenInfo CommaToken = Tokenizer->consumeNextToken();
         if (CommaToken.Kind != TokenInfo::TK_Comma) {
           Error->addError(CommaToken.Range, Error->ET_ParserNoComma)
               << CommaToken.Text;
           return false;
         }
       }
 
       Diagnostics::Context Ctx(Diagnostics::Context::MatcherArg, Error,
                                NameToken.Text, NameToken.Range,
                                Args.size() + 1);
       ParserValue ArgValue;
+      Tokenizer->SkipNewlines();
       ArgValue.Text = Tokenizer->peekNextToken().Text;
       ArgValue.Range = Tokenizer->peekNextToken().Range;
       if (!parseExpressionImpl(&ArgValue.Value)) {
         return false;
       }
 
+      Tokenizer->SkipNewlines();
       Args.push_back(ArgValue);
       SCE.nextArg();
     }
   }
 
   if (EndToken.Kind == TokenInfo::TK_Eof) {
     Error->addError(OpenToken.Range, Error->ET_ParserNoCloseParen);
     return false;
if (Entry.getValue().isConvertibleTo(AcceptedTypes, &Specificity)) {
           (Entry.getValue().getTypeAsString() + " " + Entry.getKey()).str();
       Result.emplace_back(Entry.getKey(), Decl, Specificity);
     }
   }
   return Result;
 }
 
 void Parser::addExpressionCompletions() {
-  const TokenInfo CompToken = Tokenizer->consumeNextToken();
+  const TokenInfo CompToken = Tokenizer->consumeNextTokenIgnoreNewlines();
   assert(CompToken.Kind == TokenInfo::TK_CodeCompletion);
 
   // We cannot complete code if there is an invalid element on the context
   // stack.
   for (ContextStackTy::iterator I = ContextStack.begin(),
                                 E = ContextStack.end();
        I != E; ++I) {
     if (!I->first)
bool Parser::parseExpressionImpl(VariantValue *Value) {
   case TokenInfo::TK_Eof:
     Error->addError(Tokenizer->consumeNextToken().Range,
                     Error->ET_ParserNoCode);
     return false;
 
   case TokenInfo::TK_Error:
     // This error was already reported by the tokenizer.
     return false;
-
+  case TokenInfo::TK_NewLine:
+    assert(!"Newline should never be found here");
+    return false;
   case TokenInfo::TK_OpenParen:
   case TokenInfo::TK_CloseParen:
   case TokenInfo::TK_Comma:
   case TokenInfo::TK_Period:
   case TokenInfo::TK_InvalidChar:
     const TokenInfo Token = Tokenizer->consumeNextToken();
     Error->addError(Token.Range, Error->ET_ParserInvalidToken) << Token.Text;
     return false;
std::vector<ArgKind> Parser::RegistrySema::getAcceptedCompletionTypes(
   return Registry::getAcceptedCompletionTypes(Context);
 }
 
 std::vector<MatcherCompletion> Parser::RegistrySema::getMatcherCompletions(
     ArrayRef<ArgKind> AcceptedTypes) {
   return Registry::getMatcherCompletions(AcceptedTypes);
 }
 
-bool Parser::parseExpression(StringRef Code, Sema *S,
+bool Parser::parseExpression(StringRef &Code, Sema *S,
                              const NamedValueMap *NamedValues,
                              VariantValue *Value, Diagnostics *Error) {
   CodeTokenizer Tokenizer(Code, Error);
   if (!Parser(&Tokenizer, S, NamedValues, Error).parseExpressionImpl(Value))
     return false;
-  if (Tokenizer.peekNextToken().Kind != TokenInfo::TK_Eof) {
+  auto NT = Tokenizer.peekNextToken();
+  if (NT.Kind != TokenInfo::TK_Eof && NT.Kind != TokenInfo::TK_NewLine) {
     Error->addError(Tokenizer.peekNextToken().Range,
                     Error->ET_ParserTrailingCode);
     return false;
   }
   return true;
 }
 
 std::vector<MatcherCompletion>
-Parser::completeExpression(StringRef Code, unsigned CompletionOffset, Sema *S,
+Parser::completeExpression(StringRef &Code, unsigned CompletionOffset, Sema *S,
                            const NamedValueMap *NamedValues) {
   Diagnostics Error;
   CodeTokenizer Tokenizer(Code, &Error, CompletionOffset);
   Parser P(&Tokenizer, S, NamedValues, &Error);
   VariantValue Dummy;
   P.parseExpressionImpl(&Dummy);
 
   // Sort by specificity, then by name.
   llvm::sort(P.Completions,
              [](const MatcherCompletion &A, const MatcherCompletion &B) {
                if (A.Specificity != B.Specificity)
                  return A.Specificity > B.Specificity;
                return A.TypedText < B.TypedText;
              });
 
   return P.Completions;
 }
 
 llvm::Optional<DynTypedMatcher>
-Parser::parseMatcherExpression(StringRef Code, Sema *S,
+Parser::parseMatcherExpression(StringRef &Code, Sema *S,
                                const NamedValueMap *NamedValues,
                                Diagnostics *Error) {
   VariantValue Value;
   if (!parseExpression(Code, S, NamedValues, &Value, Error))
     return llvm::Optional<DynTypedMatcher>();
   if (!Value.isMatcher()) {
     Error->addError(SourceRange(), Error->ET_ParserNotAMatcher);
     return llvm::Optional<DynTypedMatcher>();

clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp

Parser::NamedValueMap getTestNamedValues() {
   Values["nameX"] = llvm::StringRef("x");
   Values["hasParamA"] = VariantMatcher::SingleMatcher(
       functionDecl(hasParameter(0, hasName("a"))));
   return Values;
 }
 
 TEST(ParserTest, FullParserTest) {
   Diagnostics Error;
-  llvm::Optional<DynTypedMatcher> VarDecl(Parser::parseMatcherExpression(
+
+  StringRef Code =
       "varDecl(hasInitializer(binaryOperator(hasLHS(integerLiteral()),"
-      "                                      hasOperatorName(\"+\"))))",
-      &Error));
+      "                                      hasOperatorName(\"+\"))))";
+  llvm::Optional<DynTypedMatcher> VarDecl(
+      Parser::parseMatcherExpression(Code, &Error));
   EXPECT_EQ("", Error.toStringFull());
   Matcher<Decl> M = VarDecl->unconditionalConvertTo<Decl>();
   EXPECT_TRUE(matches("int x = 1 + false;", M));
   EXPECT_FALSE(matches("int x = true + 1;", M));
   EXPECT_FALSE(matches("int x = 1 - false;", M));
   EXPECT_FALSE(matches("int x = true - 1;", M));
 
-  llvm::Optional<DynTypedMatcher> HasParameter(Parser::parseMatcherExpression(
-      "functionDecl(hasParameter(1, hasName(\"x\")))", &Error));
+  Code = "functionDecl(hasParameter(1, hasName(\"x\")))";
+  llvm::Optional<DynTypedMatcher> HasParameter(
+      Parser::parseMatcherExpression(Code, &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.
   auto NamedValues = getTestNamedValues();
+
+  Code = "functionDecl(hasParamA, hasParameter(1, hasName(nameX)))";
   llvm::Optional<DynTypedMatcher> HasParameterWithNamedValues(
-      Parser::parseMatcherExpression(
-          "functionDecl(hasParamA, hasParameter(1, hasName(nameX)))",
-          nullptr, &NamedValues, &Error));
+      Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &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());
+  Code = "hasInitializer(\n    binaryOperator(hasLHS(\"A\")))";
+  EXPECT_TRUE(!Parser::parseMatcherExpression(Code, &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)",
             Error.toStringFull());
 }
 
 TEST(ParserTest, VariadicMatchTest) {
   Diagnostics Error;
-  llvm::Optional<DynTypedMatcher> OM(Parser::parseMatcherExpression(
-      "stmt(objcMessageExpr(hasAnySelector(\"methodA\", \"methodB:\")))",
-      &Error));
+
+  StringRef Code =
+      "stmt(objcMessageExpr(hasAnySelector(\"methodA\", \"methodB:\")))";
+  llvm::Optional<DynTypedMatcher> OM(
+      Parser::parseMatcherExpression(Code, &Error));
   EXPECT_EQ("", Error.toStringFull());
   auto M = OM->unconditionalConvertTo<Stmt>();
   EXPECT_TRUE(matchesObjC("@interface I @end "
                           "void foo(I* i) { [i methodA]; }", M));
 }
 
 std::string ParseWithError(StringRef Code) {
   Diagnostics Error;
TEST(ParserTest, OverloadErrors) {
   EXPECT_EQ("1:1: Error building matcher callee.\n"
             "1:8: Candidate 1: Incorrect type for arg 1. "
             "(Expected = Matcher<Stmt>) != (Actual = String)\n"
             "1:8: Candidate 2: Incorrect type for arg 1. "
             "(Expected = Matcher<Decl>) != (Actual = String)",
             ParseWithError("callee(\"A\")"));
 }
 
+TEST(ParserTest, ParseMultiline) {
+  StringRef Code;
+
+  llvm::Optional<DynTypedMatcher> M;
+  {
+    Code = R"matcher(varDecl(
+  hasName("foo")
+  )
+)matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(varDecl(
+  # Internal comment
+  hasName("foo") # Internal comment
+# Internal comment
+  )
+)matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(decl().bind(
+  "paramName")
+)matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(decl().bind(
+  "paramName"
+  )
+)matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(decl(decl()
+, decl()))matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(decl(decl(),
+decl()))matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = "namedDecl(hasName(\"n\"\n))";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Diagnostics Error;
+
+    auto NamedValues = getTestNamedValues();
+
+    Code = R"matcher(hasParamA.bind
+  ("paramName")
+)matcher";
+    M = Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error);
+    EXPECT_FALSE(M.hasValue());
+    EXPECT_EQ("1:15: Malformed bind() expression.", Error.toStringFull());
+  }
+
+  {
+    Diagnostics Error;
+
+    auto NamedValues = getTestNamedValues();
+
+    Code = R"matcher(hasParamA.
+  bind("paramName")
+)matcher";
+    M = Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error);
+    EXPECT_FALSE(M.hasValue());
+    EXPECT_EQ("1:11: Malformed bind() expression.", Error.toStringFull());
+  }
+
+  {
+    Diagnostics Error;
+
+    Code = R"matcher(varDecl
+()
+)matcher";
+    M = Parser::parseMatcherExpression(Code, nullptr, nullptr, &Error);
+    EXPECT_FALSE(M.hasValue());
+    EXPECT_EQ("1:8: Error parsing matcher. Found token "
+              "<NewLine> while looking for '('.",
+              Error.toStringFull());
+  }
+
+  // Correct line/column numbers
+  {
+    Diagnostics Error;
+
+    Code = R"matcher(varDecl(
+  doesNotExist()
+  )
+)matcher";
+    M = Parser::parseMatcherExpression(Code, nullptr, nullptr, &Error);
+    EXPECT_FALSE(M.hasValue());
+    EXPECT_EQ(R"error(1:1: Error parsing argument 1 for matcher varDecl.
+2:3: Matcher not found: doesNotExist)error",
+              Error.toStringFull());
+  }
+}
+
 TEST(ParserTest, CompletionRegistry) {
-  std::vector<MatcherCompletion> Comps =
-      Parser::completeExpression("while", 5);
+  StringRef Code = "while";
+  std::vector<MatcherCompletion> Comps = Parser::completeExpression(Code, 5);
   ASSERT_EQ(1u, Comps.size());
   EXPECT_EQ("Stmt(", Comps[0].TypedText);
   EXPECT_EQ("Matcher<Stmt> whileStmt(Matcher<WhileStmt>...)",
             Comps[0].MatcherDecl);
 
-  Comps = Parser::completeExpression("whileStmt().", 12);
+  Code = "whileStmt().";
+  Comps = Parser::completeExpression(Code, 12);
   ASSERT_EQ(1u, Comps.size());
   EXPECT_EQ("bind(\"", Comps[0].TypedText);
   EXPECT_EQ("bind", Comps[0].MatcherDecl);
 }
 
 TEST(ParserTest, CompletionNamedValues) {
   // Can complete non-matcher types.
   auto NamedValues = getTestNamedValues();

 
 TEST(ParserTest, ParseBindOnLet) {
 
   auto NamedValues = getTestNamedValues();
 
   Diagnostics Error;
 
   {
+    StringRef Code = "hasParamA.bind(\"parmABinding\")";
     llvm::Optional<DynTypedMatcher> TopLevelLetBinding(
-        Parser::parseMatcherExpression("hasParamA.bind(\"parmABinding\")",
-                                       nullptr, &NamedValues, &Error));
+        Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error));
     EXPECT_EQ("", Error.toStringFull());
     auto M = TopLevelLetBinding->unconditionalConvertTo<Decl>();
 
     EXPECT_TRUE(matchAndVerifyResultTrue(
         "void foo(int a);", M,
         std::make_unique<VerifyIdIsBoundTo<FunctionDecl>>("parmABinding")));
     EXPECT_TRUE(matchAndVerifyResultFalse(
         "void foo(int b);", M,
         std::make_unique<VerifyIdIsBoundTo<FunctionDecl>>("parmABinding")));
   }
 
   {
+    StringRef Code = "functionDecl(hasParamA.bind(\"parmABinding\"))";
     llvm::Optional<DynTypedMatcher> NestedLetBinding(
-        Parser::parseMatcherExpression(
-            "functionDecl(hasParamA.bind(\"parmABinding\"))", nullptr,
-            &NamedValues, &Error));
+        Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error));
     EXPECT_EQ("", Error.toStringFull());
     auto M = NestedLetBinding->unconditionalConvertTo<Decl>();
 
     EXPECT_TRUE(matchAndVerifyResultTrue(
         "void foo(int a);", M,
         std::make_unique<VerifyIdIsBoundTo<FunctionDecl>>("parmABinding")));
     EXPECT_TRUE(matchAndVerifyResultFalse(
         "void foo(int b);", M,
         std::make_unique<VerifyIdIsBoundTo<FunctionDecl>>("parmABinding")));
   }
 }
 
 }  // end anonymous namespace
 }  // end namespace dynamic
 }  // end namespace ast_matchers
 }  // end namespace clang