[SyntaxTree] Fix crash on pointer to member function

[eduucaldas]

Additionally deleting TraverseParenTypeLoc changes nothing in the tests. I can also investigate that, I can try to come up with a test that makes this Traverse override relevant and if I'm not able, I would remove this override.

clang/unittests/Tooling/Syntax/TreeTest.cpp

4087–4088

I spent 15 minutes investigating that, if you think I should go further I will.

[gribozavr2]

clang/unittests/Tooling/Syntax/TreeTest.cpp

4083–4084

Please add spaces before {.

4089

Could you add separate tests for X::Y:: and for **?

[eduucaldas]

answer comments

clang/lib/Tooling/Syntax/BuildTree.cpp

if (Tokens.front().kind() == tok::coloncolon) {
       // namespace a^::b {}
       // FIXME: build corresponding nodes for the name of this namespace.
       return true;
     }
     Builder.foldNode(Tokens, new (allocator()) syntax::NamespaceDefinition, S);
     return true;
   }
 
+  // FIXME: Deleting the `TraverseParenTypeLoc` override doesn't change test
+  // results. Find test coverage or remove it.
   bool TraverseParenTypeLoc(ParenTypeLoc L) {
     // We reverse order of traversal to get the proper syntax structure.
     if (!WalkUpFromParenTypeLoc(L))
       return false;
     return TraverseTypeLoc(L.getInnerLoc());
   }
 
   bool WalkUpFromParenTypeLoc(ParenTypeLoc L) {
bool WalkUpFromFunctionProtoTypeLoc(FunctionProtoTypeLoc L) {
 
     auto *TrailingReturnTokens = BuildTrailingReturn(L);
     // Finish building the node for parameters.
     Builder.markChild(TrailingReturnTokens,
                       syntax::NodeRole::ParametersAndQualifiers_trailingReturn);
     return WalkUpFromFunctionTypeLoc(L);
   }
 
+  bool TraverseMemberPointerTypeLoc(MemberPointerTypeLoc L) {
+    // In the source code "void (Y::*mp)()" `MemberPointerTypeLoc` corresponds
+    // to "Y::*" but it points to a `ParenTypeLoc` that corresponds to
+    // "(Y::*mp)" We thus reverse the order of traversal to get the proper
+    // syntax structure.
+    if (!WalkUpFromMemberPointerTypeLoc(L))
+      return false;
+    return TraverseTypeLoc(L.getPointeeLoc());
+  }
+
   bool WalkUpFromMemberPointerTypeLoc(MemberPointerTypeLoc L) {
     auto SR = L.getLocalSourceRange();
     Builder.foldNode(Builder.getRange(SR),
                      new (allocator()) syntax::MemberPointer, L);
     return true;
   }
 
   // The code below is very regular, it could even be generated with some

clang/unittests/Tooling/Syntax/TreeTest.cpp

`-SimpleDeclaration
   | | |-X
   | | |-::
   | | `-*
   | `-b
   `-;
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, MemberFunctionPointer) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+      R"cpp(
+struct X {
+  struct Y {};
+};
+void (X::*xp)();
+void (X::**xpp)(const int*);
+// FIXME: Generate the right syntax tree for this type,
+// i.e. create a syntax node for the outer member pointer
+void (X::Y::*xyp)(const int*, char);
+)cpp",
+      R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-struct
+| |-X
+| |-{
+| |-SimpleDeclaration
+| | |-struct
+| | |-Y
+| | |-{
+| | |-}
+| | `-;
+| |-}
+| `-;
+|-SimpleDeclaration
+| |-void
+| |-SimpleDeclarator
+| | |-ParenDeclarator
+| | | |-(
+| | | |-MemberPointer
+| | | | |-X
+| | | | |-::
+| | | | `-*
+| | | |-xp
+| | | `-)
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
+| `-;
+|-SimpleDeclaration
+| |-void
+| |-SimpleDeclarator
+| | |-ParenDeclarator
+| | | |-(
+| | | |-MemberPointer
+| | | | |-X
+| | | | |-::
+| | | | `-*
+| | | |-*
+| | | |-xpp
+| | | `-)
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | |-const
+| |   | |-int
+| |   | `-SimpleDeclarator
+| |   |   `-*
+| |   `-)
+| `-;
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-ParenDeclarator
+  | | |-(
+  | | |-X
+  | | |-::
+  | | |-MemberPointer
+  | | | |-Y
+  | | | |-::
+  | | | `-*
+  | | |-xyp
+  | | `-)
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-const
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   `-*
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | `-char
+  |   `-)
+  `-;
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, ComplexDeclarator) {
   EXPECT_TRUE(treeDumpEqual(
       R"cpp(
 void x(char a, short (*b)(int));
 )cpp",
       R"txt(
 *: TranslationUnit
 `-SimpleDeclaration