[PATCH] D85826: [clang] Make signature help work with dependent args

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 12 06:45:02 PDT 2020


hokein added a comment.

This looks like a nice improvement.



================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1212
+      R"cpp(
+        int foo(int a, int b);
+        template <typename T> void bar(T t) {
----------------
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.


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5571
+  Expr *NakedFn = Fn->IgnoreParenCasts();
+  // In presence of dependent args we surface all posible signatures, without
+  // performing any semantic checks on availability. That's to improve user
----------------
possible.


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5573
+  // performing any semantic checks on availability. That's to improve user
+  // experience, it is better to see all overloads rather than none.
+  if (Expr::hasAnyTypeDependentArguments(Args)) {
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85826



More information about the cfe-commits mailing list