[clang] ba32915 - [SyntaxTree] Add support for `MemberExpression`

Eduardo Caldas via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 20 08:00:38 PDT 2020


Author: Eduardo Caldas
Date: 2020-08-20T14:57:35Z
New Revision: ba32915db2ce78256115a9db7b07bb3806e6364a

URL: https://github.com/llvm/llvm-project/commit/ba32915db2ce78256115a9db7b07bb3806e6364a
DIFF: https://github.com/llvm/llvm-project/commit/ba32915db2ce78256115a9db7b07bb3806e6364a.diff

LOG: [SyntaxTree] Add support for `MemberExpression`

Differential Revision: https://reviews.llvm.org/D86227

Added: 
    

Modified: 
    clang/include/clang/Tooling/Syntax/Nodes.h
    clang/lib/Tooling/Syntax/BuildTree.cpp
    clang/lib/Tooling/Syntax/Nodes.cpp
    clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/Syntax/Nodes.h b/clang/include/clang/Tooling/Syntax/Nodes.h
index 8e65fa1d8923..8b499bc5ceb9 100644
--- a/clang/include/clang/Tooling/Syntax/Nodes.h
+++ b/clang/include/clang/Tooling/Syntax/Nodes.h
@@ -55,6 +55,7 @@ enum class NodeKind : uint16_t {
   CharUserDefinedLiteralExpression,
   StringUserDefinedLiteralExpression,
   IdExpression,
+  MemberExpression,
 
   // Statements.
   UnknownStatement,
@@ -173,7 +174,10 @@ enum class NodeRole : uint8_t {
   ParametersAndQualifiers_trailingReturn,
   IdExpression_id,
   IdExpression_qualifier,
-  ParenExpression_subExpression
+  ParenExpression_subExpression,
+  MemberExpression_object,
+  MemberExpression_accessToken,
+  MemberExpression_member,
 };
 /// For debugging purposes.
 raw_ostream &operator<<(raw_ostream &OS, NodeRole R);
@@ -322,6 +326,26 @@ class ParenExpression final : public Expression {
   Leaf *closeParen();
 };
 
+/// Models a class member access. C++ [expr.ref]
+/// member-expression:
+///   expression -> template_opt id-expression
+///   expression .  template_opt id-expression
+/// e.g. `x.a`, `xp->a`
+///
+/// Note: An implicit member access inside a class, i.e. `a` instead of
+/// `this->a`, is an `id-expression`.
+class MemberExpression final : public Expression {
+public:
+  MemberExpression() : Expression(NodeKind::MemberExpression) {}
+  static bool classof(const Node *N) {
+    return N->kind() == NodeKind::MemberExpression;
+  }
+  Expression *object();
+  Leaf *accessToken();
+  Leaf *templateKeyword();
+  IdExpression *member();
+};
+
 /// Expression for literals. C++ [lex.literal]
 class LiteralExpression : public Expression {
 public:

diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 11d399730040..a77149d66207 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -881,6 +881,49 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
     return true;
   }
 
+  bool WalkUpFromMemberExpr(MemberExpr *S) {
+    if (auto QualifierLoc = S->getQualifierLoc())
+      Builder.markChild(QualifierLoc, syntax::NodeRole::IdExpression_qualifier);
+
+    auto TemplateKeywordLoc = S->getTemplateKeywordLoc();
+    if (TemplateKeywordLoc.isValid())
+      Builder.markChildToken(TemplateKeywordLoc,
+                             syntax::NodeRole::TemplateKeyword);
+
+    auto *TheUnqualifiedId = new (allocator()) syntax::UnqualifiedId;
+    Builder.foldNode(Builder.getRange(S->getMemberLoc(), S->getEndLoc()),
+                     TheUnqualifiedId, nullptr);
+
+    Builder.markChild(TheUnqualifiedId, syntax::NodeRole::IdExpression_id);
+
+    auto *TheIdExpression = new (allocator()) syntax::IdExpression;
+    auto MemberRange =
+        Builder.getRange(S->hasQualifier() ? S->getQualifierLoc().getBeginLoc()
+                                           : S->getMemberLoc(),
+                         S->getEndLoc());
+
+    // For `MemberExpr` with implicit `this->` we generate a simple
+    // `id-expression` syntax node, beacuse an implicit `member-expression` is
+    // syntactically undistinguishable from an `id-expression`
+    if (S->isImplicitAccess()) {
+      Builder.foldNode(MemberRange, TheIdExpression, S);
+      return true;
+    }
+    Builder.foldNode(MemberRange, TheIdExpression, nullptr);
+
+    Builder.markChild(TheIdExpression,
+                      syntax::NodeRole::MemberExpression_member);
+
+    Builder.markExprChild(S->getBase(),
+                          syntax::NodeRole::MemberExpression_object);
+    Builder.markChildToken(S->getOperatorLoc(),
+                           syntax::NodeRole::MemberExpression_accessToken);
+
+    Builder.foldNode(Builder.getExprRange(S),
+                     new (allocator()) syntax::MemberExpression, S);
+    return true;
+  }
+
   bool WalkUpFromDeclRefExpr(DeclRefExpr *S) {
     if (auto QualifierLoc = S->getQualifierLoc())
       Builder.markChild(QualifierLoc, syntax::NodeRole::IdExpression_qualifier);

diff  --git a/clang/lib/Tooling/Syntax/Nodes.cpp b/clang/lib/Tooling/Syntax/Nodes.cpp
index bf3c3108cb69..e09c8f20f210 100644
--- a/clang/lib/Tooling/Syntax/Nodes.cpp
+++ b/clang/lib/Tooling/Syntax/Nodes.cpp
@@ -126,6 +126,8 @@ raw_ostream &syntax::operator<<(raw_ostream &OS, NodeKind K) {
     return OS << "SimpleTemplateNameSpecifier";
   case NodeKind::NestedNameSpecifier:
     return OS << "NestedNameSpecifier";
+  case NodeKind::MemberExpression:
+    return OS << "MemberExpression";
   }
   llvm_unreachable("unknown node kind");
 }
@@ -202,6 +204,12 @@ raw_ostream &syntax::operator<<(raw_ostream &OS, NodeRole R) {
     return OS << "IdExpression_qualifier";
   case syntax::NodeRole::ParenExpression_subExpression:
     return OS << "ParenExpression_subExpression";
+  case syntax::NodeRole::MemberExpression_object:
+    return OS << "MemberExpression_object";
+  case syntax::NodeRole::MemberExpression_accessToken:
+    return OS << "MemberExpression_accessToken";
+  case syntax::NodeRole::MemberExpression_member:
+    return OS << "MemberExpression_member";
   }
   llvm_unreachable("invalid role");
 }
@@ -230,6 +238,26 @@ syntax::NestedNameSpecifier::specifiersAndDoubleColons() {
   return Children;
 }
 
+syntax::Expression *syntax::MemberExpression::object() {
+  return cast_or_null<syntax::Expression>(
+      findChild(syntax::NodeRole::MemberExpression_object));
+}
+
+syntax::Leaf *syntax::MemberExpression::templateKeyword() {
+  return llvm::cast_or_null<syntax::Leaf>(
+      findChild(syntax::NodeRole::TemplateKeyword));
+}
+
+syntax::Leaf *syntax::MemberExpression::accessToken() {
+  return llvm::cast_or_null<syntax::Leaf>(
+      findChild(syntax::NodeRole::MemberExpression_accessToken));
+}
+
+syntax::IdExpression *syntax::MemberExpression::member() {
+  return cast_or_null<syntax::IdExpression>(
+      findChild(syntax::NodeRole::MemberExpression_member));
+}
+
 syntax::NestedNameSpecifier *syntax::IdExpression::qualifier() {
   return cast_or_null<syntax::NestedNameSpecifier>(
       findChild(syntax::NodeRole::IdExpression_qualifier));

diff  --git a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
index fd858dfba91f..2692c1f6ff58 100644
--- a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -491,19 +491,20 @@ struct X {
   operator int();
 };
 void test(X x) {
-  // TODO: Expose `id-expression` from `MemberExpr`
   [[x.operator int()]];
 }
 )cpp",
       {R"txt(
 UnknownExpression
-|-UnknownExpression
+|-MemberExpression
 | |-IdExpression
 | | `-UnqualifiedId
 | |   `-x
 | |-.
-| |-operator
-| `-int
+| `-IdExpression
+|   `-UnqualifiedId
+|     |-operator
+|     `-int
 |-(
 `-)
 )txt"}));
@@ -542,19 +543,20 @@ TEST_P(SyntaxTreeTest, UnqualifiedId_Destructor) {
       R"cpp(
 struct X { };
 void test(X x) {
-  // TODO: Expose `id-expression` from `MemberExpr`
   [[x.~X()]];
 }
 )cpp",
       {R"txt(
 UnknownExpression
-|-UnknownExpression
+|-MemberExpression
 | |-IdExpression
 | | `-UnqualifiedId
 | |   `-x
 | |-.
-| |-~
-| `-X
+| `-IdExpression
+|   `-UnqualifiedId
+|     |-~
+|     `-X
 |-(
 `-)
 )txt"}));
@@ -568,18 +570,23 @@ TEST_P(SyntaxTreeTest, UnqualifiedId_DecltypeDestructor) {
       R"cpp(
 struct X { };
 void test(X x) {
-  // TODO: Expose `id-expression` from `MemberExpr`
+  // FIXME: Make `decltype(x)` a child of `MemberExpression`. It is currently
+  // not because `Expr::getSourceRange()` returns the range of `x.~` for the
+  // `MemberExpr` instead of the expected `x.~decltype(x)`, this is a bug in
+  // clang.
   [[x.~decltype(x)()]];
 }
 )cpp",
       {R"txt(
 UnknownExpression
-|-UnknownExpression
+|-MemberExpression
 | |-IdExpression
 | | `-UnqualifiedId
 | |   `-x
 | |-.
-| `-~
+| `-IdExpression
+|   `-UnqualifiedId
+|     `-~
 |-decltype
 |-(
 |-x
@@ -624,6 +631,9 @@ namespace n {
   struct S { };
 }
 void test() {
+  // FIXME: Remove the `UnknownExpression` wrapping `s1` and `s2`. This
+  // `UnknownExpression` comes from a leaf `CXXConstructExpr` in the
+  // ClangAST. We need to ignore leaf implicit nodes.
   [[::n::S s1]];
   [[n::S s2]];
 }
@@ -1756,6 +1766,9 @@ TEST_P(SyntaxTreeTest, OverloadedOperator_Plus) {
 struct X {
   friend X operator+(X, const X&);
 };
+// FIXME: Remove additional `UnknownExpression` wrapping `x`. For that, ignore
+// implicit copy constructor called on `x`. This should've been ignored already,
+// as we `IgnoreImplicit` when traversing an `Stmt`.
 void test(X x, X y) {
   [[x + y]];
 }
@@ -1961,6 +1974,366 @@ PostfixUnaryOperatorExpression
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, MemberExpression_SimpleWithDot) {
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  int a;
+};
+void test(struct S s) {
+  [[s.a]];
+}
+)cpp",
+      {R"txt(
+MemberExpression
+|-IdExpression
+| `-UnqualifiedId
+|   `-s
+|-.
+`-IdExpression
+  `-UnqualifiedId
+    `-a
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_StaticDataMember) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  static int a;
+};
+void test(S s) {
+  [[s.a]];
+}
+)cpp",
+      {R"txt(
+MemberExpression
+|-IdExpression
+| `-UnqualifiedId
+|   `-s
+|-.
+`-IdExpression
+  `-UnqualifiedId
+    `-a
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_SimpleWithArrow) {
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  int a;
+};
+void test(struct S* sp) {
+  [[sp->a]];
+}
+)cpp",
+      {R"txt(
+MemberExpression
+|-IdExpression
+| `-UnqualifiedId
+|   `-sp
+|-->
+`-IdExpression
+  `-UnqualifiedId
+    `-a
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_Chaining) {
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  struct S* next;
+};
+void test(struct S s){
+  [[s.next->next]];
+}
+)cpp",
+      {R"txt(
+MemberExpression
+|-MemberExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-s
+| |-.
+| `-IdExpression
+|   `-UnqualifiedId
+|     `-next
+|-->
+`-IdExpression
+  `-UnqualifiedId
+    `-next
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_OperatorFunction) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  bool operator!();
+};
+void test(S s) {
+  [[s.operator!()]];
+}
+)cpp",
+      {R"txt(
+UnknownExpression
+|-MemberExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-s
+| |-.
+| `-IdExpression
+|   `-UnqualifiedId
+|     |-operator
+|     `-!
+|-(
+`-)
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_Implicit) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  int a;
+  int test(){
+    // FIXME: Remove the `UnknownExpression` wrapping `a`. This
+    // `UnknownExpression` comes from an implicit leaf `CXXThisExpr`.
+    [[a]];
+  }
+};
+)cpp",
+      {R"txt(
+IdExpression
+`-UnqualifiedId
+  `-UnknownExpression
+    `-a
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_VariableTemplate) {
+  if (!GetParam().isCXX14OrLater()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  template<typename T>
+  static constexpr T x = 42;
+};
+// FIXME: `<int>` should be a child of `MemberExpression` and `;` of
+// `ExpressionStatement`. This is a bug in clang, in `getSourceRange` methods.
+void test(S s) [[{
+  s.x<int>;
+}]]
+)cpp",
+      {R"txt(
+CompoundStatement
+|-{
+|-ExpressionStatement
+| `-MemberExpression
+|   |-IdExpression
+|   | `-UnqualifiedId
+|   |   `-s
+|   |-.
+|   `-IdExpression
+|     `-UnqualifiedId
+|       `-x
+|-<
+|-int
+|->
+|-;
+`-}
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_FunctionTemplate) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  template<typename T>
+  T f();
+};
+void test(S* sp){
+  [[sp->f<int>()]];
+}
+)cpp",
+      {R"txt(
+UnknownExpression
+|-MemberExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-sp
+| |-->
+| `-IdExpression
+|   `-UnqualifiedId
+|     |-f
+|     |-<
+|     |-int
+|     `->
+|-(
+`-)
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_FunctionTemplateWithTemplateKeyword) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  template<typename T>
+  T f();
+};
+void test(S s){
+  [[s.template f<int>()]];
+}
+)cpp",
+      {R"txt(
+UnknownExpression
+|-MemberExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-s
+| |-.
+| |-template
+| `-IdExpression
+|   `-UnqualifiedId
+|     |-f
+|     |-<
+|     |-int
+|     `->
+|-(
+`-)
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_WithQualifier) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct Base {
+  void f();
+};
+struct S : public Base {};
+void test(S s){
+  [[s.Base::f()]];
+  [[s.::S::~S()]];
+}
+)cpp",
+      {R"txt(
+UnknownExpression
+|-MemberExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-s
+| |-.
+| `-IdExpression
+|   |-NestedNameSpecifier
+|   | |-IdentifierNameSpecifier
+|   | | `-Base
+|   | `-::
+|   `-UnqualifiedId
+|     `-f
+|-(
+`-)
+      )txt",
+       R"txt(
+UnknownExpression
+|-MemberExpression
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-s
+| |-.
+| `-IdExpression
+|   |-NestedNameSpecifier
+|   | |-::
+|   | |-IdentifierNameSpecifier
+|   | | `-S
+|   | `-::
+|   `-UnqualifiedId
+|     |-~
+|     `-S
+|-(
+`-)
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, MemberExpression_Complex) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+template<typename T>
+struct U {
+  template<typename U>
+  U f();
+};
+struct S {
+  U<int> getU();
+};
+void test(S* sp) {
+  // FIXME: The first 'template' keyword is a child of `NestedNameSpecifier`,
+  // but it should be a child of `MemberExpression` according to the grammar.
+  // However one might argue that the 'template' keyword fits better inside
+  // `NestedNameSpecifier` because if we change `U<int>` to `UI` we would like
+  // equally to change the `NameSpecifier` `template U<int>` to just `UI`.
+  [[sp->getU().template U<int>::template f<int>()]];
+}
+)cpp",
+      {R"txt(
+UnknownExpression
+|-MemberExpression
+| |-UnknownExpression
+| | |-MemberExpression
+| | | |-IdExpression
+| | | | `-UnqualifiedId
+| | | |   `-sp
+| | | |-->
+| | | `-IdExpression
+| | |   `-UnqualifiedId
+| | |     `-getU
+| | |-(
+| | `-)
+| |-.
+| `-IdExpression
+|   |-NestedNameSpecifier
+|   | |-SimpleTemplateNameSpecifier
+|   | | |-template
+|   | | |-U
+|   | | |-<
+|   | | |-int
+|   | | `->
+|   | `-::
+|   |-template
+|   `-UnqualifiedId
+|     |-f
+|     |-<
+|     |-int
+|     `->
+|-(
+`-)
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
       R"cpp(


        


More information about the cfe-commits mailing list