[clangd-dev] RFC: Constructor/destructor code completion

[Ilya Biryukov via clangd-dev]

If we ignore the implementation details for a moment and think of what the
results should be in those cases, my thoughts would be:

For (2) neither of the results make sense, the completion results should be
empty.
We are completing on top-level, so we try to complete either (a) a type of
the declaration or (b) constructor/destructor name.
There are no types inside 'foo', so (a) should not produce any results
either.
There are no user-defined ctors and dtors, so (b) should not produce any
results.

Note that (b) describes a form of completion that we do not have, i.e. the
completion for out-of-line definitions of methods inside a class.
It is different from normal member completions after dot (e.g. `foo().^`):
- Should only include methods that do not have a definition.
- Should not have any argument placeholders in snippets.
- Should include the return type.
- Should probably complete the braces for method body `{}` too.

For (1), we sometimes (very rarely) want destructor, but it's so rare that
typing '~' and repeating completion seems to be fine in those cases.
For (3), the struct and destructor names are almost indistinguishable, the
only possible visible difference is the presence of function argument
snippet (i.e. `foo()` vs `foo`).
This does not seem to be a major UX concern, either seems fine. Reiterating
(2): without user-defined dtor we should not show dtor in those completion
results.

Don't have a strong opinion, but if we do choose to have destructors in
code completion, not showing void as return type for destructors (and no
return type for constructors constructors) seems totally reasonable.

On Tue, Sep 25, 2018 at 5:17 PM Jan Korous via clangd-dev <
clangd-dev at lists.llvm.org> wrote:

> Hi,
>
> As a follow-up to discussion about destructor return type in code
> completion [1] I investigated status quo of constructor and destructor code
> completion in clangd.
>
> I was using this trivial lit-test case:
>
>
> {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
> ---
>
> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"
> test:///main.cpp","languageId":"cpp","version":1,"text":"struct foo {
> bool b; }; void f() { foo a; a."}}}
> ---
>
> {"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"
> test:///main.cpp"},"position":{"line":0,"character":44}}}
>
>
> These are my observations.
>
> 1. We currently skip constructors and destructors in code completion after
> . and -> member access in clangd.
>
> struct foo { bool b; }; void f() { foo a; a.^
> // produces single result - bool b
>
> This seems fine.
>
> 2. We currently skip constructors but include destructors, assignment
> operators and struct/class name after double colon.
>
> struct foo {}; foo::^
> // produces 4 results - struct foo itself, copy assignment, move
> assignment, destructor
>
> - It seems to me that by including struct/class name we effectively
> include constructor code completion which makes sense in this situation but
> we shouldn’t put it’s kind as struct/class.
> - What seems odd to me in this context is that we use void as return type
> of destructor (and had we not skipped constructor, it’s return type might
> be unexpected too). We previously discussed destructor return type with Sam
> in [1] and I agreed that it’s fine under false impression that we aren’t
> displaying destructors anyway (off-by-one error in my test case).
>
> I suggest that we don’t include struct/class name but include both
> constructor (without any return type == no “detail” in LSP) and destructor
> (without any return type == no “detail” in LSP) when code completion
> context is "struct/class name followed by double colon".
>
>
> 3. We currently skip both constructors and destructors but include
> struct/class name after double colon and tilde but also include lot of
> other results.
>
> struct foo {}; foo::~^
> // produces 66 results - including keywords and probably built-in macros
> and struct/class name but not constructor or destructor
>
> This seems wrong to me. I suggest we skip unrelated keywords and
> struct/class name but we should include destructor (with proper insertText
> to not repeat the leading tilde).
>
>
> I’d like to know other people’s thoughts on this before I work on patches.
> I just created one WIP patch when I was investigating why we don’t skip
> destructor in double colon case - anyone interested please see [2].
>
> Thank you.
>
> Cheers,
>
> Jan
>
>
>
> [1] https://reviews.llvm.org/D52308 [Sema][CodeCompletion] Fix return
> type of C++ destructors in code completion
>
> [2] https://reviews.llvm.org/D52495 [WIP][Sema][CodeCompletion] Add base
> type for double colon member completion results
> _______________________________________________
> clangd-dev mailing list
> clangd-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev
>


-- 
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/clangd-dev/attachments/20180925/16d2fe3f/attachment.html>