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

[Jan Korous via clangd-dev]

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 <https://reviews.llvm.org/D52308> [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

[2] https://reviews.llvm.org/D52495 <https://reviews.llvm.org/D52495> [WIP][Sema][CodeCompletion] Add base type for double colon member completion results
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/clangd-dev/attachments/20180925/86c4c3d3/attachment.html>