Fair - maybe just a comment in the test case (and/or the source code that handles this) saying "hey, let's make sure dtors aren't here - but if/when there's support for a low priority completion group, these should appear there" or the like.
On Mon, Jan 29, 2018 at 9:49 AM Sam McCall <sam.mcc...@gmail.com> wrote: > 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
