<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 29, 2018 at 5:31 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Any chance of making this a really low priority completion?</div></blockquote><div>(Just for clarity - this is a postfilter in clangd only, we haven't changed the behavior of the Sema or libclang APIs)</div><div><br></div><div>We certainly have considered downranking such items, maybe it's the right thing, but it's not without issues on the UX side:</div><div> - It's far from the only candidate, other things that people want "at the bottom" include inaccessible and unavailable members, injected type names, operators... it gets crowded, so you still have to decide how to rank them.</div><div> - LSP doesn't (currently) have any affordance to "grey out" results and show users "this is the last good result, the ones below here are unlikely to be useful". So the important end-of-list signal is lost.</div><div>There's also a bit of implementation complexity in having second/third/fourth "tiers" of results - it adds invariants to the scoring logic that make it harder to understand and modify.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">(maybe just leaving in a FIXME or expected-fail check of some kind if it's not practical to implement it right now) For those handful of times when you actually want to do this.</div></blockquote><div>re: practical to implement, there are a few problems with it, apart from being rarely useful: </div><div> - It completes after "foo.", but not "foo.~F", I guess because the parser is in the wrong state</div><div> - it completes ~basic_string, but not ~string</div><div><br></div><div>We do indirectly test that destructor completions are turned off in clangd. I don't really know where to add the FIXMEs. I can add XFAIL tests to c-index-test I think, is anyone likely to go looking for them?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div dir="ltr">On Mon, Jan 22, 2018 at 1:06 PM Sam McCall via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@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">Author: sammccall<br>
Date: Mon Jan 22 13:05:00 2018<br>
New Revision: 323149<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=323149&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=323149&view=rev</a><br>
Log:<br>
[clangd] Drop ~destructor completions - rarely helpful and work inconsistently<br>
<br>
Modified:<br>
clang-tools-extra/trunk/<wbr>clangd/CodeComplete.cpp<br>
clang-tools-extra/trunk/<wbr>unittests/clangd/<wbr>CodeCompleteTests.cpp<br>
<br>
Modified: clang-tools-extra/trunk/<wbr>clangd/CodeComplete.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=323149&r1=323148&r2=323149&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/clangd/CodeComplete.cpp?<wbr>rev=323149&r1=323148&r2=<wbr>323149&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/<wbr>clangd/CodeComplete.cpp (original)<br>
+++ clang-tools-extra/trunk/<wbr>clangd/CodeComplete.cpp Mon Jan 22 13:05:00 2018<br>
@@ -361,6 +361,10 @@ struct CompletionRecorder : public CodeC<br>
(Result.Availability == CXAvailability_NotAvailable ||<br>
Result.Availability == CXAvailability_NotAccessible))<br>
continue;<br>
+ // Destructor completion is rarely useful, and works inconsistently.<br>
+ // (s.^ completes ~string, but s.~st^ is an error).<br>
+ if (dyn_cast_or_null<<wbr>CXXDestructorDecl>(Result.<wbr>Declaration))<br>
+ continue;<br>
Results.push_back(Result);<br>
}<br>
}<br>
<br>
Modified: clang-tools-extra/trunk/<wbr>unittests/clangd/<wbr>CodeCompleteTests.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=323149&r1=323148&r2=323149&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/unittests/clangd/<wbr>CodeCompleteTests.cpp?rev=<wbr>323149&r1=323148&r2=323149&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/<wbr>unittests/clangd/<wbr>CodeCompleteTests.cpp (original)<br>
+++ clang-tools-extra/trunk/<wbr>unittests/clangd/<wbr>CodeCompleteTests.cpp Mon Jan 22 13:05:00 2018<br>
@@ -461,7 +461,7 @@ TEST(CompletionTest, NoDuplicates) {<br>
{cls("Adapter")});<br>
<br>
// Make sure there are no duplicate entries of 'Adapter'.<br>
- EXPECT_THAT(Results.items, ElementsAre(Named("Adapter"), Named("~Adapter")));<br>
+ EXPECT_THAT(Results.items, ElementsAre(Named("Adapter")))<wbr>;<br>
}<br>
<br>
TEST(CompletionTest, ScopedNoIndex) {<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</blockquote></div><br></div></div>