kbobyrev added inline comments.
================ Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:52-55 + const syntax::Token *FirstToken = Tree->findFirstLeaf()->getToken(), + *LastToken = Tree->findLastLeaf()->getToken(); + assert(FirstToken->kind() == tok::TokenKind::l_brace); + assert(LastToken->kind() == tok::TokenKind::r_brace); ---------------- sammccall wrote: > eduucaldas wrote: > > Take a look at `clang/include/clang/Tooling/Syntax/Nodes.h`, syntax > > constructs usually have nice classes with accessors. > > > > For instance `CompoundStatement` has the accessors `getLbrace` and > > `getRbrace` that seem to be exactly what you want. > > > > However these might not give exactly the first leaf and last leaf in the > > case of syntactically incorrect code. > I think we should treat all bracket-like things generically. Today this is > just CompoundStmt, but we want to handle init lists, function calls, parens > around for-loop conditions, template parameter and arg lists etc in the same > way. > > This sort of use is why the `OpenParen`/`CloseParen` NodeRoles are generic - > we can have one set of logic to handle all of these. (No opinion on whether > that should live here or in the syntax trees library, but putting it here for > now seems fine). > > So in the end I think checking the class name and then grabbing the braces by > role (not kind) is the right thing here. > We definitely want to avoid asserting that the code looks the way we expect > though. > So in the end I think checking the class name and then grabbing the braces by > role (not kind) is the right thing here. > We definitely want to avoid asserting that the code looks the way we expect > though. Can you elaborate a bit on how this would work? Is your proposal to iterate through `CompoundStatement` first-level children and grab the `OpenParen` + `CloseParen` roles? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88553/new/ https://reviews.llvm.org/D88553 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
