Allow comments with '#' in dynamic AST Matchers

[steveire]

Summary

This is necessary for clang-query to be able to handle comments.

[aaron.ballman]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

188–190

Why are empty errors created? I would have expected this to be EXPECT_EQ(0, Sema.Errors.size()).

(If this formulation is desired, it should use a range-based for loop.)

[steveire]

use cxx_range_for

[steveire]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

188–190

This is a copy-paste from elsewhere in the file.

My guess is the author intended the test to print any messages that get recorded while running the unit test (They will differ from the empty string).

[aaron.ballman]

Aside from some naming and style convention nits, LGTM.

unittests/ASTMatchers/Dynamic/ParserTest.cpp

151

const auto &E please.

188

const auto &E please.

lib/ASTMatchers/Dynamic/Parser.cpp

TokenInfo getNextToken() {
 
     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;
     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);

unittests/ASTMatchers/Dynamic/ParserTest.cpp

 }
 
 TEST(ParserTest, ParseMatcher) {
   MockSema Sema;
   const uint64_t ExpectedFoo = Sema.expectMatcher("Foo");
   const uint64_t ExpectedBar = Sema.expectMatcher("Bar");
   const uint64_t ExpectedBaz = Sema.expectMatcher("Baz");
   Sema.parse(" Foo ( Bar ( 17), Baz( \n \"B A,Z\") ) .bind( \"Yo!\") ");
-  for (size_t i = 0, e = Sema.Errors.size(); i != e; ++i) {
-    EXPECT_EQ("", Sema.Errors[i]);
+  for (auto e : Sema.Errors) {
+    EXPECT_EQ("", e);
   }
 
   EXPECT_NE(ExpectedFoo, ExpectedBar);
   EXPECT_NE(ExpectedFoo, ExpectedBaz);
   EXPECT_NE(ExpectedBar, ExpectedBaz);
 
   EXPECT_EQ(1ULL, Sema.Values.size());
   EXPECT_EQ(ExpectedFoo, getSingleMatcher(Sema.Values[0])->getID().second);
TEST(ParserTest, ParseMatcher) {
   EXPECT_EQ("Foo", Foo.MatcherName);
   EXPECT_TRUE(matchesRange(Foo.NameRange, 1, 2, 2, 12));
   EXPECT_EQ(2ULL, Foo.Args.size());
   EXPECT_EQ(ExpectedBar, getSingleMatcher(Foo.Args[0].Value)->getID().second);
   EXPECT_EQ(ExpectedBaz, getSingleMatcher(Foo.Args[1].Value)->getID().second);
   EXPECT_EQ("Yo!", Foo.BoundID);
 }
 
+TEST(ParserTest, ParseComment) {
+  MockSema Sema;
+  const uint64_t ExpectedFoo = Sema.expectMatcher("Foo");
+  Sema.parse(" Foo() # Bar() ");
+  for (auto e : Sema.Errors) {
+    EXPECT_EQ("", e);
+  }
+
+  EXPECT_EQ(1ULL, Sema.Matchers.size());
+
+  Sema.parse("Foo(#) ");
+
+  EXPECT_EQ("1:4: Error parsing matcher. Found end-of-code while looking for ')'.", Sema.Errors[1]);
+}
+
 using ast_matchers::internal::Matcher;
 
 Parser::NamedValueMap getTestNamedValues() {
   Parser::NamedValueMap Values;
   Values["nameX"] = llvm::StringRef("x");
   Values["hasParamA"] = VariantMatcher::SingleMatcher(
       functionDecl(hasParameter(0, hasName("a"))));
   return Values;