[PATCH] D65526: [Clangd] Initial prototype version of ExtractFunction

Fri Aug 2 06:58:20 PDT 2019

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