[PATCH] D93227: [clangd] Smarter hover on auto and decltype

Fri Dec 18 03:41:09 PST 2020

qchateau added a comment.

You can land this if it is still fine after my final update.

email: quentin.chateau at gmail.com

Thanks !

================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:2452
 // In namespace ns
 ret_type foo(params) {})",
       },
----------------
I've also changed this raw string to a normal string (and another one line 2729) because they include whitespace at the end of the line. Git complains about it and my editor automatically trims trailing whitespace. I assume I'm not the only one using this setting and it annoyed me more than it should.

================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:999
+      "decltype(au^to) x = 0;",
+      R"cpp(// Lambda auto parameter. Nothing (Not useful).
+            auto lamb = [](a^uto){};
----------------
sammccall wrote:
> qchateau wrote:
> > sammccall wrote:
> > > (not convinced this is fundamentally not useful - the fact that it's a template parameter means it's probably worth having a hover card for it at some point. But I agree with suppressing it for now)
> > As a user I'd prefer the hover to work over the whole `decltype(auto)` expression. But that does not seem quite compatible with the way tokens are parsed.
> > 
> > Are you suggesting I remove the test case or should I add a `FIXME` comment ?
> Sorry, I think we're talking about different examples.
> 
> (I agree decltype(auto) should ideally be a single thing and we should support hover on all of it, no need to address in this patch, FIXME would be nice).
> 
> But I was talking about the lambda auto parameter, where your comment says "not useful". Anyway, nothing to do here either except maybe soften the comment to "not supported at the moment".
Ahah yes my bad, I copy pasted the comment x)
I fixed it

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93227