<div dir="ltr"><div>If we ignore the implementation details for a moment and think of what the results should be in those cases, my thoughts would be:</div><div><br></div>For (2) neither of the results make sense, the completion results should be empty.<div>We are completing on top-level, so we try to complete either (a) a type of the declaration or (b) constructor/destructor name.<div>There are no types inside 'foo', so (a) should not produce any results either.<br></div><div>There are no user-defined ctors and dtors, so (b) should not produce any results.</div><div><br></div><div>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. </div><div>It is different from normal member completions after dot (e.g. `foo().^`):</div><div>- Should only include methods that do not have a definition.</div><div>- Should not have any argument placeholders in snippets.</div><div>- Should include the return type.</div><div>- Should probably complete the braces for method body `{}` too.</div><div><br></div><div>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.</div><div>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`).</div><div>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.</div></div><div><br></div><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 25, 2018 at 5:17 PM Jan Korous via clangd-dev <<a href="mailto:clangd-dev@lists.llvm.org">clangd-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">Hi,<div><br><div>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.</div></div><div><br></div><div>I was using this trivial lit-test case:</div><div><br></div><div><div>{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}</div><div>---</div><div>{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"<a>test:///main.cpp</a>","languageId":"cpp","version":1,"text":"struct foo { bool b; }; void f() { foo a; a."}}}</div><div>---</div><div>{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"<a>test:///main.cpp</a>"},"position":{"line":0,"character":44}}}</div></div><div><br></div><div><br></div><div>These are my observations.</div><div><br></div><div>1. We currently skip constructors and destructors in code completion after . and -> member access in clangd.</div><div><br></div><div>struct foo { bool b; }; void f() { foo a; a.^</div><div>// produces single result - bool b</div><div><br></div><div>This seems fine.</div><div><br></div><div>2. We currently skip constructors but include destructors, assignment operators and struct/class name after double colon.</div><div><br></div><div>struct foo {}; foo::^</div><div>// produces 4 results - struct foo itself, copy assignment, move assignment, destructor</div><div><br></div><div><div>- 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.</div><div>- 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).</div><div><br></div><div>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".</div></div><div><br></div><div><br></div><div>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.</div><div><br></div><div>struct foo {}; foo::~^</div><div>// produces 66 results - including keywords and probably built-in macros and struct/class name but not constructor or destructor</div><div><br></div><div>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).</div><div><br></div><div><br></div><div>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].</div><div><br></div><div>Thank you.</div><div><br></div><div>Cheers,</div><div><br></div><div>Jan</div><div><br></div><div><br></div><div><br></div><div>[1] <a href="https://reviews.llvm.org/D52308" target="_blank">https://reviews.llvm.org/D52308</a> [Sema][CodeCompletion] Fix return type of C++ destructors in code completion</div><div><br></div><div>[2] <a href="https://reviews.llvm.org/D52495" target="_blank">https://reviews.llvm.org/D52495</a> [WIP][Sema][CodeCompletion] Add base type for double colon member completion results</div></div>_______________________________________________<br>
clangd-dev mailing list<br>
<a href="mailto:clangd-dev@lists.llvm.org" target="_blank">clangd-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>