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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
