[PATCH] D62814: [clangd] Treat lambdas as functions when preparing hover response

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 4 01:24:58 PDT 2019


ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

In D62814#1528736 <https://reviews.llvm.org/D62814#1528736>, @kadircet wrote:

> Nice catch! I think it makes sense to show signature in those cases as well.
>  Updating according to that.


Those cases are pretty rare, though, so I wasn't even sure they were forth it. They are pretty cheap to handle, though, so LG.

LGTM with a few last second NITs



================
Comment at: clang-tools-extra/clangd/XRefs.cpp:664
+        if (!DT->getUnderlyingType().isNull())
+          if (const auto *CD = DT->getUnderlyingType()->getAsCXXRecordDecl())
+            return CD->getLambdaCallOperator();
----------------
ilya-biryukov wrote:
> NIT: add extra braces to the inner `if` for more readable code
Uh, I meant the **outer** if, sorry for the confusion. My idea is that only one level of nesting is allowed to omit braces.
```
if (condition1) {
  if (condition2) 
    return true;
}
```

Not a big deal, though, don't want to nit-pick on minor code style issues. Feel free to tailor to your liking


================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:831
+         };
          return HI;
        }},
----------------
Could you add another test with even weirder types where we fail to show the signature? To make sure we don't break when reaching the limitations of the chosen approach and document what those limitations are.

Something like:
```
auto a = [](int a) { return 10; };
auto *b = &a;
auto *c = &b;
```

We would fail to show the signature here, but it's totally ok to ignore it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62814





More information about the cfe-commits mailing list