On Mon, Jan 29, 2018 at 5:31 PM, David Blaikie <dblai...@gmail.com> wrote:
> Any chance of making this a really low priority completion? > (Just for clarity - this is a postfilter in clangd only, we haven't changed the behavior of the Sema or libclang APIs) We certainly have considered downranking such items, maybe it's the right thing, but it's not without issues on the UX side: - 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. - 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. 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. (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. > re: practical to implement, there are a few problems with it, apart from being rarely useful: - It completes after "foo.", but not "foo.~F", I guess because the parser is in the wrong state - it completes ~basic_string, but not ~string 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? On Mon, Jan 22, 2018 at 1:06 PM Sam McCall via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: sammccall >> Date: Mon Jan 22 13:05:00 2018 >> New Revision: 323149 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=323149&view=rev >> Log: >> [clangd] Drop ~destructor completions - rarely helpful and work >> inconsistently >> >> Modified: >> clang-tools-extra/trunk/clangd/CodeComplete.cpp >> clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp >> >> Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/clangd/CodeComplete.cpp?rev=323149&r1=323148&r2=323149&view=diff >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) >> +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon Jan 22 13:05:00 >> 2018 >> @@ -361,6 +361,10 @@ struct CompletionRecorder : public CodeC >> (Result.Availability == CXAvailability_NotAvailable || >> Result.Availability == CXAvailability_NotAccessible)) >> continue; >> + // Destructor completion is rarely useful, and works >> inconsistently. >> + // (s.^ completes ~string, but s.~st^ is an error). >> + if (dyn_cast_or_null<CXXDestructorDecl>(Result.Declaration)) >> + continue; >> Results.push_back(Result); >> } >> } >> >> Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp >> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/ >> trunk/unittests/clangd/CodeCompleteTests.cpp?rev= >> 323149&r1=323148&r2=323149&view=diff >> ============================================================ >> ================== >> --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp >> (original) >> +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon >> Jan 22 13:05:00 2018 >> @@ -461,7 +461,7 @@ TEST(CompletionTest, NoDuplicates) { >> {cls("Adapter")}); >> >> // Make sure there are no duplicate entries of 'Adapter'. >> - EXPECT_THAT(Results.items, ElementsAre(Named("Adapter"), >> Named("~Adapter"))); >> + EXPECT_THAT(Results.items, ElementsAre(Named("Adapter"))); >> } >> >> TEST(CompletionTest, ScopedNoIndex) { >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
