Page MenuHomePhabricator

[clangd] allow extracting to variable for complete lambda expressions
Needs RevisionPublic

Authored by 5chmidti on Jan 14 2023, 5:12 AM.



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

Diff Detail

Event Timeline

5chmidti created this revision.Jan 14 2023, 5:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 14 2023, 5:12 AM
5chmidti requested review of this revision.Jan 14 2023, 5:12 AM
5chmidti updated this revision to Diff 489232.Jan 14 2023, 5:16 AM

Add change to release notes

LGTM, thanks!


Maybe add a "Code Actions" section for this?

kadircet accepted this revision.Jan 23 2023, 12:05 AM

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)


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



This revision is now accepted and ready to land.Jan 23 2023, 12:05 AM
5chmidti updated this revision to Diff 491319.Jan 23 2023, 5:25 AM
  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.

  1. 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.

5chmidti marked 3 inline comments as done.Jan 23 2023, 5:25 AM
kadircet requested changes to this revision.Jan 23 2023, 9:09 AM

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

This revision now requires changes to proceed.Jan 23 2023, 9:09 AM
nridge added inline comments.Feb 21 2023, 1:20 AM

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 added inline comments.Feb 21 2023, 1:29 AM

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.

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.