[clangd] allow extracting to variable for lambda expressions

[5chmidti]

Summary

Support for extracting lambda expressions, e.g. extracting a lambda from a callexpr (e.g. algorithms/ranges) to a named variable.

[5chmidti]

Add change to release notes

[nridge]

LGTM, thanks!

clang-tools-extra/docs/ReleaseNotes.rst

86

Maybe add a "Code Actions" section for this?

[kadircet]

branch cut is upon us, so it'd be great if you can land this soon (or let us know if you don't have commit access)

clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

177–193

nit: can we re-write this as: !isa<LambdaExpr>(Stmt) || InsertionPoint->Selected == SelectionTree::Complete ?

clang-tools-extra/docs/ReleaseNotes.rst

86

+1

[5chmidti]

1. addressed comments

2 bugfix (sorry, should've said something)
Local variables inside the lambda were previously added to the ReferencedDecls vector and would block the action inside of exprIsValidOutside (declarations are inside of the lambda).
Now only consider DeclRefExprs in the lambda captures.

3. I don't have commit access, someone else would need to commit this.

I don't know if you want to delay this patch due to the bugfix or not, either way is fine with me.

[kadircet]

sorry i'll need to take a look at this again.

[nridge]

clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

104

What about variables referenced implicitly via a capture-default?

For example, consider:

int main() {
  if (int a = 1)
    if ([[ [&](){ return a + 1; } ]]() == 4)
      a = a + 1;
}

Here, the attempted extraction point is before the outer if, which is before the declaration of a, and the fact that the lambda references a should prevent that.

[nridge]

clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

106

On the other hand, the exclusion of local variables declared inside lambdas should also apply to lambdas which are subexpressions of the selected expression. Consider:

template <typename T>
auto call(T t) { return t(); }

int main() {
  return [[ call([](){ int a = 1; return a + 1; }) ]] + 5;
}

Here, the presence of the local variable a inside the lambda should not interfere with the ability to extract the call(...) expression.

[5chmidti]

You're right, I'll incorporate the logic for lambdas into FindDeclRefsVisitor and change the docs/comments/commit message to reflect this. I will try to think of more edge cases to test as well. Though I'll be busy until mid march with uni, so there will be no new revisions until then.

[5chmidti]

I took a little break, but here are the changes/fixes:

• moved the logic for variables referenced in captures into the visitor
  • short circuiting the TraverseLambdaExpression and using TraverseLambdaCapture to handle variable captures and the initialization fo init-captures
  • fixes both problems mentioned by nridge
• fix for immediately invoked lambda expressions
  • the selection of an IIL would mark the call operator as a referenced decl (fixed in VisitDeclRefExpr)
• fix condition in CanExtractOutside to allow an unselected parent that is a lambda expression
  • allows for the extraction of an initializer for init-capture variables
• block default arguments from prarameters of a lambda from being extracted to a function-local scope (fixed in CanExtractOutside)
• added more tests

[5chmidti]

clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

178

Noticed that this can be removed because extracting from captures is allowed. Will change in the next update

182–184

This is supposed to stop the following invalid code from happening:

void foo() {
  int placeholder = 42;
  [](int x = placeholder {};
  extern void bar(int x = placeholder);
}

clangd does not seem to support extracting from the initializers of defaulted parameters, should I keep the condition as is, or should I do something different here? It is legal to have default arguments in global scope (examples below).

The following insertions could exist (out of scope for this patch):

int placeholder = 42;
void foo() {
  [](int x = placeholder {};
  extern void bar(int x = placeholder);
}

struct X {
  static inline int placeholder = 42;
  void foo(int x = placeholder) {}
};

Either way, I'll have to adjust the comment because this is not just to stop default parameter initializers in lambdas from being extracted to a local scope.