[PATCH] D70569: [clangd] Allow extract-to-function on regions that always return.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added a comment.

(Thanks! Will address comments, just wanted to ask for a clarification)



================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:180
+      Last = CS->body_back();
+  return llvm::isa<ReturnStmt>(Last);
+}
----------------
kadircet wrote:
> what if there's a goto in the selection :P 
If the goto target is in the selection, that's fine. Otherwise it's broken control flow like break/continue. It doesn't seem to be detected yet indeed.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:611
+  // (Others are possible if there are conversions, but this seems clearest).
+  if (CapturedInfo.HasReturnStmt) {
+    // If the return is conditional, neither replacing the code with
----------------
kadircet wrote:
> nit: early exits
I'm not sure what this comment means, can you elaborate?
(I do mean "sometimes returns and sometimes doesn't". Early-exits aren't a problem, e.g. `if (x) return 1; return 2;` can be extracted.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70569/new/

https://reviews.llvm.org/D70569