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
