SureYeaah added inline comments.
================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:119 + case SelectionTree::Selection::Partial: + // Treat Partially selected VarDecl as completely selected since + // SelectionTree doesn't always select VarDecls correctly. ---------------- sammccall wrote: > D66872 has the fix if you'd rather avoid these workarounds Will remove this once D66872 is committed. ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:150 + } + bool isRootStmt(const Stmt *S) const; + // The last root statement is important to decide where we need to insert a ---------------- kadircet wrote: > it seems like you are rather using it to decide whether a statement is inside > the zone or not? > > Could you rather rename it to reflect that and add some comments? if extracting ``` for(;;) int x = 0; ``` the DeclStmt is inside the zone but not a rootstmt. ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:172 + // FIXME: Support extraction from templated functions. + if (CurNode->Parent->ASTNode.get<FunctionTemplateDecl>()) + return nullptr; ---------------- kadircet wrote: > nit: > ``` > if(isa<FunctionTemplateDecl>(Func)) > ``` Using `Func->isTemplated()` ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:346 + ZoneRelative DeclaredIn; + // index of the declaration or first reference + unsigned DeclIndex; ---------------- kadircet wrote: > i think you mean `index of the first reference to this decl` ? It can be the index of declaration or the first reference. Since the visitor Visits Decls as well, we will encounter a reference before the Decl only if the Decl is outside the enclosing function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65526/new/ https://reviews.llvm.org/D65526 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
