[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
This revision was automatically updated to reflect the committed changes. Closed by commit rGba32915db2ce: [SyntaxTree] Add support for `MemberExpression` (authored by eduucaldas). Changed prior to commit: https://reviews.llvm.org/D86227?vs=286811=286820#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 Files: 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 Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp === --- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp +++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp @@ -491,19 +491,20 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ )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
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
eduucaldas updated this revision to Diff 286811. eduucaldas marked 2 inline comments as done. eduucaldas added a comment. Unify logic for generating `id-expression` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 Files: 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 Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp === --- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp +++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp @@ -491,19 +491,20 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ )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 +//
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:911 + S->getEndLoc()), + idExpression, nullptr); + eduucaldas wrote: > gribozavr2 wrote: > > This code seems largely identical to WalkUpFromDeclRefExpr. (It is also > > somewhat difficult to track what is happening because we are creating three > > levels of nodes here.) Could we factor out the repeated code? > I agree. > However there are some things that change between semantic nodes that produce > an `IdExpression`. > > * We may or not have a link from `IdExpression` to the AST. > * The `SourceRange` for the `UnqualifiedId` is obtained in an ad-hoc manner. > > Taking into these variables we could write a function `buildIdExpression` > > 1. `IdExpression* buildIdExpression(Expr* E, SourceRange UnqualifiedIdLoc, > bool shouldLinkToAST);` > 2. `IdExpression* buildIdExpression(SourceRange QualifierLoc, SourceRange > TemplateKwLoc, SourceRange UnqualifiedIdLoc, ASTPtr link);` > > The first option takes into account only the differences that we perceived > until now. > The second option provides a more general approach. I'd prefer (2), it is less subtle and more explicit. Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:1999 + +TEST_P(SyntaxTreeTest, MemberExpression_StaticMemberFunction) { + if (!GetParam().isCXX()) { Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
eduucaldas marked an inline comment as done. eduucaldas added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:900 + +auto *unqualifiedId = new (allocator()) syntax::UnqualifiedId; +Builder.foldNode(Builder.getRange(S->getMemberLoc(), S->getEndLoc()), gribozavr2 wrote: > Please make unqualifiedId variable start with the uppercase character. Since > it would clash with the type name, you can do `TheUnqualifiedId`, for example. > > Same for `idExpression` below. I just saw in the style guide the rules for casing. I didn't expect this to exist because it doesn't seem to be obeyed ^^. For instance WalkUpFrom* starts with Upper-case. I'll follow-up with a patch fixing any style problems of this nature. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:911 + S->getEndLoc()), + idExpression, nullptr); + gribozavr2 wrote: > This code seems largely identical to WalkUpFromDeclRefExpr. (It is also > somewhat difficult to track what is happening because we are creating three > levels of nodes here.) Could we factor out the repeated code? I agree. However there are some things that change between semantic nodes that produce an `IdExpression`. * We may or not have a link from `IdExpression` to the AST. * The `SourceRange` for the `UnqualifiedId` is obtained in an ad-hoc manner. Taking into these variables we could write a function `buildIdExpression` 1. `IdExpression* buildIdExpression(Expr* E, SourceRange UnqualifiedIdLoc, bool shouldLinkToAST);` 2. `IdExpression* buildIdExpression(SourceRange QualifierLoc, SourceRange TemplateKwLoc, SourceRange UnqualifiedIdLoc, ASTPtr link);` The first option takes into account only the differences that we perceived until now. The second option provides a more general approach. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
eduucaldas updated this revision to Diff 286799. eduucaldas marked 10 inline comments as done. eduucaldas added a comment. Answering comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 Files: 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 Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp === --- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp +++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp @@ -491,19 +491,20 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ )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_StaticMemberFunction) { + 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
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
eduucaldas updated this revision to Diff 286762. eduucaldas added a comment. Implicit `MemberExpr` generates `id-expression` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 Files: 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 Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp === --- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp +++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp @@ -491,19 +491,20 @@ 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 @@ 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 @@ 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 @@ struct S { }; } void test() { + // FIXME: Remove the terminal `UnknownExpression` wrapping `s1` and `s2`. This + // `UnknownExpression` comes from a terminal `CXXConstructExpr` in the + // ClangAST. We need to ignore terminal implicit nodes. [[::n::S s1]]; [[n::S s2]]; } @@ -1756,6 +1766,9 @@ 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,305 @@ )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_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 geta(){ +// FIXME: Remove the terminal`UnknownExpression` wrapping `a`. This +// `UnknownExpression` comes from a terminal implicit `CXXThisExpr`. +[[a]]; + } +}; +)cpp", + {R"txt( +IdExpression +`-UnqualifiedId + `-UnknownExpression +`-a +)txt"})); +} + +TEST_P(SyntaxTreeTest, MemberExpression_Template) { + if (!GetParam().isCXX()) { +return; + } + EXPECT_TRUE(treeDumpEqualOnAnnotations( + R"cpp( +struct S { + template + T f(); +}; +void test(S* sp){ +
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:180 + MemberExpression_member, + MemberExpression_accessToken, }; Could you order new items in source order? object, access token, member. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:333 +/// expression . template_opt id-expression +/// id-expression +/// e.g. `x.a`, `xp->a` or even just `a` when we have an implicit `this->`. eduucaldas wrote: > We could discuss how to model the implicit member expression, as it has a > totally different syntax. I think the syntax tree should represent only the syntax that was actually present in the source code. IOW, implicit member expression from Clang AST should not map to a member expression in the syntax tree. The syntax tree should represent just the id-expression. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:900 + +auto *unqualifiedId = new (allocator()) syntax::UnqualifiedId; +Builder.foldNode(Builder.getRange(S->getMemberLoc(), S->getEndLoc()), Please make unqualifiedId variable start with the uppercase character. Since it would clash with the type name, you can do `TheUnqualifiedId`, for example. Same for `idExpression` below. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:911 + S->getEndLoc()), + idExpression, nullptr); + This code seems largely identical to WalkUpFromDeclRefExpr. (It is also somewhat difficult to track what is happening because we are creating three levels of nodes here.) Could we factor out the repeated code? Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:634-636 + // FIXME: Remove the terminal `UnknownExpression` wrapping `s1` and `s2`. This + // `UnknownExpression` comes from a terminal `CXXConstructExpr` in the + // ClangAST. We need to ignore terminal implicit nodes. Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2085 + int a; + int geta(){ +// FIXME: Remove the terminal`UnknownExpression` wrapping `a`. This Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2086-2087 + int geta(){ +// FIXME: Remove the terminal`UnknownExpression` wrapping `a`. This +// `UnknownExpression` comes from a terminal implicit `CXXThisExpr`. +[[a]]; eduucaldas wrote: > I'm experimenting with that now, but anyways, I think this should go to the > patch, > > [SyntaxTree] Add support to `CXXThisExpr` Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2101 + +TEST_P(SyntaxTreeTest, MemberExpression_Template) { + if (!GetParam().isCXX()) { Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2133 + +TEST_P(SyntaxTreeTest, MemberExpression_TemplateWithTemplateKeyword) { + if (!GetParam().isCXX()) { Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2165 +} + +TEST_P(SyntaxTreeTest, MemberExpression_WithQualifier) { Please add tests that access static members through member expression syntax. Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2165 +} + +TEST_P(SyntaxTreeTest, MemberExpression_WithQualifier) { gribozavr2 wrote: > Please add tests that access static members through member expression syntax. > Please add a test for variable templates: ``` struct S { template static constexpr T x = 42; }; void f(S s) { s.x; } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
eduucaldas added a reviewer: gribozavr2. eduucaldas added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:333 +/// expression . template_opt id-expression +/// id-expression +/// e.g. `x.a`, `xp->a` or even just `a` when we have an implicit `this->`. We could discuss how to model the implicit member expression, as it has a totally different syntax. Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:1769-1771 +// 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`. Not much progress trying to use other ignores. Perhaps we can treat this properly when adding support for `CXXConstructExpr` Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2086-2087 + int geta(){ +// FIXME: Remove the terminal`UnknownExpression` wrapping `a`. This +// `UnknownExpression` comes from a terminal implicit `CXXThisExpr`. +[[a]]; I'm experimenting with that now, but anyways, I think this should go to the patch, > [SyntaxTree] Add support to `CXXThisExpr` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
eduucaldas updated this revision to Diff 286593. eduucaldas added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86227/new/ https://reviews.llvm.org/D86227 Files: 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 Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp === --- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp +++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp @@ -491,19 +491,20 @@ 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 @@ 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 @@ 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 @@ struct S { }; } void test() { + // FIXME: Remove the terminal `UnknownExpression` wrapping `s1` and `s2`. This + // `UnknownExpression` comes from a terminal `CXXConstructExpr` in the + // ClangAST. We need to ignore terminal implicit nodes. [[::n::S s1]]; [[n::S s2]]; } @@ -1756,6 +1766,9 @@ 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,306 @@ )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_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 geta(){ +// FIXME: Remove the terminal`UnknownExpression` wrapping `a`. This +// `UnknownExpression` comes from a terminal implicit `CXXThisExpr`. +[[a]]; + } +}; +)cpp", + {R"txt( +MemberExpression +`-IdExpression + `-UnqualifiedId +`-UnknownExpression + `-a +)txt"})); +} + +TEST_P(SyntaxTreeTest, MemberExpression_Template) { + if (!GetParam().isCXX()) { +return; + } + EXPECT_TRUE(treeDumpEqualOnAnnotations( + R"cpp( +struct S { + template + T f(); +}; +void test(S* sp){ + [[sp->f()]]; +} +)cpp", +
[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`
eduucaldas created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. eduucaldas requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D86227 Files: 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 Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp === --- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp +++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp @@ -491,19 +491,20 @@ 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 @@ 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 @@ 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 @@ struct S { }; } void test() { + // FIXME: Remove the terminal `UnknownExpression` wrapping `s1` and `s2`. This + // `UnknownExpression` comes from a terminal `CXXConstructExpr` in the + // ClangAST. We need to ignore terminal implicit nodes. [[::n::S s1]]; [[n::S s2]]; } @@ -1756,6 +1766,9 @@ 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,306 @@ )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_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 geta(){ +// FIXME: Remove the terminal`UnknownExpression` wrapping `a`. This +// `UnknownExpression` comes from a terminal implicit `CXXThisExpr`. +[[a]]; + } +}; +)cpp", + {R"txt( +MemberExpression +`-IdExpression + `-UnqualifiedId +`-UnknownExpression + `-a +)txt"})); +} + +TEST_P(SyntaxTreeTest, MemberExpression_Template) { + if (!GetParam().isCXX()) { +return; + } + EXPECT_TRUE(treeDumpEqualOnAnnotations( + R"cpp( +struct S { + template + T f(); +}; +void test(S* sp){ + [[sp->f()]]; +} +)cpp",