This is an archive of the discontinued LLVM Phabricator instance.

[clang] Make signature help work with dependent args
ClosedPublic

Authored by kadircet on Aug 12 2020, 3:33 AM.

Diff Detail

Event Timeline

kadircet created this revision.Aug 12 2020, 3:33 AM
kadircet requested review of this revision.Aug 12 2020, 3:33 AM

This looks like a nice improvement.

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
1212

These tests are nice.

But we're changing Sema in this patch, I think we'd better to have these in clang, /test/CodeCompletion/call.cpp seems to be the place.

clang/lib/Sema/SemaCodeComplete.cpp
5582

possible.

5584

I'm a bit nervous to show all overloads regardlessly, I think we could do some smarter things like

  1. if there are some prefix non-type-dependent arguments, we could use these type information to filter out some unrelated candidates;
  2. different overloads may have different number of parameters, this is an important signal, e.g. foo(t, ^t) should not give void foo(int) result;

2 seems quite critical and trivial to implement, maybe we can do it in this patch.

kadircet updated this revision to Diff 285116.Aug 12 2020, 9:34 AM
  • Add tests into clang-lit
  • Make sure current number of args is less than overloads param count.
kadircet updated this revision to Diff 285117.Aug 12 2020, 9:35 AM
kadircet marked 2 inline comments as done.
  • Fix typo in comment
kadircet marked an inline comment as done.Aug 12 2020, 9:35 AM
kadircet added inline comments.Aug 12 2020, 9:59 AM
clang/lib/Sema/SemaCodeComplete.cpp
5584

if there are some prefix non-type-dependent arguments, we could use these type information to filter out some unrelated candidates;

well actually now that I think about it, this is actually not that hard. possibly even easier than current version. changing the logic surface every candidate using the prefix, and do a post-filtering instead of trying to mock all the results.

kadircet updated this revision to Diff 285141.Aug 12 2020, 11:07 AM
  • Change logic to find all signatures without any dependent args and post filter non-viable overloads using the argument counts.
nridge added a subscriber: nridge.Aug 16 2020, 11:32 PM
hokein accepted this revision.Aug 17 2020, 1:01 AM

thanks, looks better.

clang/lib/Sema/SemaCodeComplete.cpp
5513–5525

nit: use early return.

if (!Candidate.Function) {
  continue;
}
...
This revision is now accepted and ready to land.Aug 17 2020, 1:01 AM
kadircet added inline comments.Aug 17 2020, 1:06 AM
clang/lib/Sema/SemaCodeComplete.cpp
5513–5525

i wanted to do that, but it changes the semantics. as push_back below executes even when Candidate.Function is nullptr :/

This revision was landed with ongoing or failed builds.Aug 17 2020, 1:15 AM
This revision was automatically updated to reflect the committed changes.