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 ↗

(On Diff #167846)

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 ↗

(On Diff #167846)

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 ↗

(On Diff #167987)

const auto &E please.

188 ↗

(On Diff #167987)

const auto &E please.

cfe/trunk/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);

cfe/trunk/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 (const 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 (const 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;