[clangd-dev] additional TextEdits on code completion

[Sam McCall via clangd-dev]

On Mon, Apr 8, 2019 at 8:36 AM Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>
wrote:

> Sam McCall <sammccall at google.com> writes:
>
> > Sorry about that :-( +Eric Liu <ioeric at google.com> who may have
> thoughts.
> > I don't think we have an option to disable includes today, maybe we
> should
> > add one...
> >
> > But we don't expect the inserted includes to create compile errors -
> *this
> > is probably a bug, it'd be great if you could provide more details* (is
> it
> > the right header but spelled wrong, or the wrong header entirely, etc).
> It
> > may be that you'd like this feature if it worked properly.
> >
>
> Sorry for the delay in response. This mostly is due to the location where
> the #include line is added. With company-lsp (emacs), it gets added at
> the beginning of the file and the dependency across headers results in
> build failures.

If you have time, it'd be great to see a breakdown of such an example
(which files and symbols are involved, how the compile error comes about).

Currently we're assuming:
1) each symbol has a single header where its "main" declaration is found
2) if you're using a symbol in a file, that header should be included (or
the symbol should be forward-declared)
3) it's safe and useful to directly include exactly the directly required
headers, rather than relying on transitive includes
This style is called include-what-you-use. Its main advantages: it tends
not to break code when #include structure changes, and it's easy to decide
what should be #included.

Personally, my advice would be to follow this style. If inserting an
#include header breaks your compile, it's likely that:
 - that header is not #including one of its dependencies
 - you have a circular dependency, which can be resolved with a forward
declaration
 - header guards are missing somewhere
etc

However I do think it might make sense to offer a way to disable include
insertion entirely for projects that are not IWYU-style and don't want to
be.

I do have a pending patch to never insert #includes of files that don't
have recognized header guards (#ifdef/#define/#endif).
https://reviews.llvm.org/D60316
This avoids triggering the feature in *some* cases where it's not safe.
Happy to look at other heuristics if they're feasible to implement.

One important thing to note is, I am able to build without any
> error even without the new #include line. Hence not sure why we should add
> the
> extra #include when completing function names.

1) Often the relevant header is *transitively* included already, but not
directly included. If file A needs symbol C, relying on a transitive A.c ->
B.h -> C.h include means that if B stops depending on C, or A stops
depending on B, then A will break.
2) There are lots of possible behaviors here and they all have downsides,
we have to pick one (at least as default).
  a) never inserting headers: breaks code after many completions
  b) inserting only when the symbol isn't declared in a transitively
included header: unacceptable performance penalty to deserialize all
declarations from the preamble, problems with incomplete types
  c) inserting only when the primary header isn't transitively included:
unpredictable behavior in large codebases, still can break compiles in the
same way
  d) IWYU: causes problems in codebases that are not IWYU-clean.

Cheers, Sam


>
> -aneesh
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/clangd-dev/attachments/20190408/c3f40748/attachment.html>