[PATCH] D137040: [clangd] Add heuristic for dropping snippet when completing member function pointer

[Tom Praschan via Phabricator via cfe-commits]

tom-anders added a comment.

In D137040#3907497 <https://reviews.llvm.org/D137040#3907497>, @nridge wrote:

> Thanks for the patch!
>
> The test looks good to me.
>
> As for the fix, I wonder if it would make sense to implement it in the Sema layer rather than in clangd. For example, we could give `clang::CodeCompletionResult` a new flag <https://searchfox.org/llvm/rev/f97639ce13754e78e26f8d7f564830ddfe4f727c/clang/include/clang/Sema/CodeCompleteConsumer.h#851> called something like `CanBeCall`, which, for a completion result that refers to a function, is set to true (by the code that creates the CodeCompletionResult, in SemaCodeComplete.cpp) if the use of the function may be a call, and false if it cannot.
>
> Then clangd can check this flag, and clear the snippet suffix (or even better, not build it in the first place) if the value is false.
>
> I think this approach could have a couple of advantages:
>
> 1. By placing the logic into libSema, other consumers of the Sema completion layer, besides clangd, could benefit from it as well. (A couple of other consumers that I'm aware of are libclang <https://clang.llvm.org/doxygen/group__CINDEX__CODE__COMPLET.html> and cling <https://root.cern/cling/>.)
> 2. Because the logic now resides inside `Sema` itself, if we make enhancements to the heuristics for computing `CanBeCall` that require additional state (besides `CurContext`), we can just use them in place rather than having to propagate them into places like clangd's `CodeCompletionBuilder`.
>
> What do you think?

That indeed sounds like a better solution. I haven't looked much into SemaCodeComplete.cpp up to now, but I'll take a look and try to implement the heuristic over there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137040