[PATCH] D52273: [clangd] Initial implementation of expected types

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 24 11:35:32 PDT 2018


sammccall added a comment.

Happy to speculate about what might work here, but I strongly believe the path forward here is to build the simplest version of this feature, without conversions, and try to avoid complicated conversion logic if we can get most of the benefit in simpler ways.

In https://reviews.llvm.org/D52273#1243652, @ilya-biryukov wrote:

> > This seems very clever, but extremely complicated - you've implemented much of C++'s conversion logic, it's not clear to me which parts are actually necessary to completion quality.
>
> Clearly the model that supports C++ conversions is something that **will** improve code completion quality.


It's not clear that will be significant. This isn't hard to measure, so I'm not sure why we should guess. And I'm not sure why it all has to go in the first patch.

> I do agree it's not trivial, but would argue we at least want:
> 
> - qualification conversions (i.e. adding const)

Another approach here is just always dropping const. (And refs, and so on). This will create some false positives, but maybe they don't hurt much. This handles some true cases too, like invoking copy constructors.

> - user-defined conversions (e.g. operator bool is commonly useful think)

My **guess** is you're not going to measure a difference here, bool has lots of false positives and others are rare.

> - derived-to-base conversions (Derived* should convert to Base*)

Yes, probably. If this ends up being the only "chain" we have to follow, we're probably in good shape complexity-wise.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52273





More information about the cfe-commits mailing list