[ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

[ymandel]

Summary

Add tests for matchers on, onImplicitObjectArgument and hasObjectExpression.

[lebedev.ri]

Please always subscribe lists.
The reviews that happen without lists are null and void.

Is this testing pre-existing behavior, and thus NFC?

[ymandel]

In D56850#1361436, @lebedev.ri wrote:

Please always subscribe lists.
The reviews that happen without lists are null and void.

Thanks for updating my diffs with the relevant list. Noted for next time.

Is this testing pre-existing behavior, and thus NFC?

Yes.

[steveire]

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

475 ↗

(On Diff #182262)

Could you add a test which uses the same c++ snippet as this, but with a matcher using hasName("Y")?

[ymandel]

Extended test for matcher on.

[ymandel]

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

475 ↗

(On Diff #182262)

Good suggestion -- this caught a mistake in the comments. I'll update the other diff to reflect.

[steveire]

Thanks, do you need someone to commit this for you?

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

492 ↗

(On Diff #182295)

In the OnImplicitObjectArgument test, you don't check snippet 1 because it has no X. This line is probably not needed.

558 ↗

(On Diff #182295)

Are we missing a matcher that would reach the type of X in this case? hasImplicitObjectExpression, or something equivalent?

[ymandel]

Remove unnecessary expectation.

[ymandel]

Extended test of hasObjectExpression.

[ymandel]

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

558 ↗

(On Diff #182295)

Good question. The reason we're not reaching X in this case is that we're distinguishing between T and T* (like thisPointerType does). That's separate from the difference between on and onImplicitObjectArgument. So, I think we'll at least want a matcher that elides this difference, e.g.
hasObjectType.

I'm less inclined to add matchers that distinguish the written from the coerced member expression (one variant each for the expression and type matchers) given that I think this issue comes up far less for member expressions that don't involve calls. I'm just afraid that the proliferation of matchers might confuse beginners.

Note that I extended the test to include the hasPointerType() case (especially since it's now mentioned in the comments).

[ymandel]

Aaron, Alex, any additional comments on this change?

[aaron.ballman]

LGTM!

[steveire]

Sorry, this fell off my radar :). LGTM too.

[ymandel]

Sync to head.

[mstorsjo]

This broke compilation with GCC 5.4 on Ubuntu 16.04:

../tools/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:474:243: warning: missing terminating " character
../tools/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:474:3: error: missing terminating " character
   EXPECT_TRUE(matches(
   ^
../tools/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:480:7: error: stray ‘\’ in program
         void z(X x) { x.m(); }
       ^

cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

TEST(Matcher, isInstanceMessage) {
 
   EXPECT_FALSE(matchesObjC(
       "@interface NSString +(NSString *) stringWithFormat; @end "
       "void f() { [NSString stringWithFormat]; }",
       objcMessageExpr(isInstanceMessage())));
 
 }
 
+TEST(MatcherCXXMemberCallExpr, On) {
+  auto MatchesType = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X")))));
+  EXPECT_TRUE(matches(
+      R"cc(
+        struct Y {
+          void m();
+        };
+        struct X : public Y {};
+        void z(X x) { x.m(); }
+      )cc",
+      MatchesType));
+  EXPECT_TRUE(notMatches(
+      R"cc(
+        struct Y {
+          void m();
+        };
+        void z(Y y) { y.m(); }
+      )cc",
+      MatchesType));
+
+  // Parens are ignored.
+  auto MatchesCall = cxxMemberCallExpr(on(callExpr()));
+  EXPECT_TRUE(matches(
+      R"cc(
+        struct Y {
+          void m();
+        };
+        Y g();
+        void z(Y y) { (g()).m(); }
+      )cc",
+      MatchesCall));
+}
+
+TEST(MatcherCXXMemberCallExpr, OnImplicitObjectArgument) {
+  auto Snippet1 = R"cc(
+    struct Y {
+      void m();
+    };
+    void z(Y y) { y.m(); }
+  )cc";
+  auto Snippet2 = R"cc(
+    struct Y {
+      void m();
+    };
+    struct X : public Y {};
+    void z(X x) { x.m(); }
+  )cc";
+  auto MatchesY = cxxMemberCallExpr(
+      onImplicitObjectArgument(hasType(cxxRecordDecl(hasName("Y")))));
+  EXPECT_TRUE(matches(Snippet1, MatchesY));
+  EXPECT_TRUE(matches(Snippet2, MatchesY));
+
+  auto MatchesX = cxxMemberCallExpr(
+      onImplicitObjectArgument(hasType(cxxRecordDecl(hasName("X")))));
+  EXPECT_TRUE(notMatches(Snippet2, MatchesX));
+
+  // Parens are not ignored.
+  auto MatchesCall = cxxMemberCallExpr(onImplicitObjectArgument(callExpr()));
+  EXPECT_TRUE(notMatches(
+      R"cc(
+        struct Y {
+          void m();
+        };
+        Y g();
+        void z(Y y) { (g()).m(); }
+      )cc",
+      MatchesCall));
+}
+
+TEST(Matcher, HasObjectExpr) {
+  auto M = memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))));
+  EXPECT_TRUE(matches(
+      R"cc(
+        struct X {
+          int m;
+          int f(X x) { return x.m; }
+        };
+      )cc",
+      M));
+  EXPECT_TRUE(notMatches(
+      R"cc(
+        struct X {
+          int m;
+          int f(X x) { return m; }
+        };
+      )cc",
+      M));
+}
+
 TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {
   StatementMatcher ArgumentY =
     declRefExpr(to(varDecl(hasName("y")))).bind("arg");
   DeclarationMatcher IntParam = parmVarDecl(hasType(isInteger())).bind("param");
   StatementMatcher CallExpr =
     callExpr(forEachArgumentWithParam(ArgumentY, IntParam));
 
   // IntParam does not match.