SureYeaah marked 2 inline comments as done. SureYeaah added a comment. In D65526#1612117 <https://reviews.llvm.org/D65526#1612117>, @sammccall wrote:
> I guess you're looking for design review at this point. > > But the most important part of that is the documentation of the high-level > structure, which is entirely missing. I can't really infer it from the code > because most of the design is (presumably) not implemented at this point :-) Aaah. Yes. Sorry. Makes sense. ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:110 + // FIXME: Bail out when it's partial selected. Wait for selectiontree + // semicolon fix. + if (CommonAnc->Selected == SelectionTree::Selection::Partial && ---------------- sammccall wrote: > what is the fix you're waiting for? The semicolon fix that has already landed now. ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:211 + // Note that DRELocType is never OUTSIDECONTEXT. + if (DeclLocType + 1 != DRELocType) + return; ---------------- sammccall wrote: > this line seems very suspect, what are you trying to do and why? > This just checks whether (DeclLocType, DRELocType) is either (PRETARGET, TARGET) or (TARGET, POSTTARGET). Could explicitly write it if needed. 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
