[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

[Nathan Ridge via Phabricator via cfe-commits]

nridge requested changes to this revision.
nridge added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:187
+        // Allow expressions, but only allow completely selected lambda
+        // expressions or unselected lambda expressions that are the parent of
+        // the originally selected node, not partially selected lambda
----------------
nridge wrote:
> 5chmidti wrote:
> > nridge wrote:
> > > I think it's worth adding to the comment *why* we allow unselected lambda expressions (to allow extracting from capture initializers), as it's not obvious
> > I changed the previous return of `!isa<LambdaExpr>(Stmt) || InsertionPoint->Selected != SelectionTree::Partial;` to a simple `return true;`. None of my partial selection tests fail and I can't think of a case where the lambda would be partially selected.
> Hmm, so what actually happens in these testcases?
> 
> ```
>       // lambdas: partially selected
>       [][[(){}]];
>       []()[[{}]];
>       [ [[ ](){}]];
> ```
I checked, and in these cases we disallow the extraction [here](https://searchfox.org/llvm/rev/c2bee1ed26a3355d164c92f1eb70ebf88804560d/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp#426-432) before we get to creating an `ExtractionContext`.

If the testcases are modified as follows:

```
template <typename T> void sink(T) {}
...
void f() {
  // lambdas: partially selected
  sink([][[(){}]]);
  sink([]()[[{}]]);
  sink([ [[ ](){}]]);
}
```

now they fail, unless the check for partial selection here is restored.


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

https://reviews.llvm.org/D141757