[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

[Younan Zhang via Phabricator via cfe-commits]

zyounan added inline comments.


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3577
       // containing all of the arguments up to the first deducible argument.
+      // Or, if this isn't a call, emit all the template arguments
+      // to disambiguate the (potential) overloads.
----------------
nridge wrote:
> zyounan wrote:
> > nridge wrote:
> > > 1. If this is not a call, we can avoid running the `Sema::MarkDeducedTemplateParameters` operation altogether.
> > > 
> > > 2. A future improvement we could consider: if this is not a call, try to detect cases where the template parameters can be deduced from the surrounding context (["Deducing template arguments taking the address of a function template "](https://eel.is/c++draft/temp.deduct.funcaddr)). Maybe add a FIXME for this?
> > > avoid running the Sema::MarkDeducedTemplateParameters operation altogether
> > 
> > I think doing so could cause too many adjustments to the flow, and I'm afraid that the `Sema::MarkDeducedTemplateParameters` would be required again when we decide to implement point 2.
> > 
> > I'm adding a FIXME anyway but leave the first intact. However, I'd be happy to rearrange the logic flow if you insist.
> I realized there's actually an open question here: if `FunctionCanBeCall == false`, do we want to include **all** the template parameters, or just the non-deducible ones?
> 
> Let's consider an example:
> 
> ```
> class Foo {
>   template <typename T, typename U>
>   T convertTo(U from);
> };
> 
> void bar() {
>   Foo::^
> }
> ```
> 
> Here, `U` can be deduced but `T` cannot.
> 
> The current behaviour is to complete `convertTo<T>`. That seems appropriate for a **call**, since `U` will be deduced from the call. But since this is not a call, wouldn't it be  better to complete `convertTo<T, U>`?
> But since this is not a call, wouldn't it be better to complete convertTo<T, U>?

(Ugh, this is exactly this patch's initial intention, but how...?)

Oh, I made a mistake here: I presumed `LastDeducibleArgument` would always be 0 if we're in a non-callable context, but this is wrong! This variable is calculated based on the function parameters, which ought to be equal to the number of template parameters deducible in function parameters.

( I used to duplicate this `if` block for the `!FunctionCanBeCall` case but finally, I merged these two erroneously. :-( )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156605