[PATCH] D86227: [SyntaxTree] Add support for `MemberExpression`

[Dmitri Gribenko via Phabricator via cfe-commits]

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