additional TextEdits on code completion

Hi All,

I am finding that with clangd after a function name code completion it
adds a header to the top of the file. This almost always results in
compilation error with my project and I would like to disable that
header file addition to the .c file. I don't see a command line option
that will help disable the feature. Any option i can use to disable?
this?

-aneesh

Sorry about that :frowning: +Eric Liu 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.

Sam McCall <sammccall@google.com> writes:

Sorry about that :frowning: +Eric Liu <ioeric@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. 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.

-aneesh

Sam McCall <sammccall@google.com> writes:

Sorry about that :frowning: +Eric Liu <ioeric@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

I would appreciate a compiler option to entirely disable this feature.
I agree with the include-what-you-use style, but my LSP editor plugin
has a bug where when a new line is inserted at the top of the file,
the cursor stays on the same line number, so it moves up a line.

~Theodore

I would appreciate a compiler option to entirely disable this feature.
I agree with the include-what-you-use style, but my LSP editor plugin
has a bug where when a new line is inserted at the top of the file,
the cursor stays on the same line number, so it moves up a line.

Ouch :frowning: Which editor/plugin is that?
That makes sense, we need to never insert includes during code completion in that case, and a flag is a reasonable way to configure it (it’s not codebase-specific at all).

Clangd also adds include-insertion fixes to “unknown symbol” diagnostics, so you can still get assistance with #includes that way. (This didn’t make the cut for clangd 8 though).

It's LanguageClient-neovim. I've also run into a clangd assertion
failure when using that plugin that I haven't seen in VSCode (reported
at https://bugs.llvm.org/show_bug.cgi?id=41091).

~Theodore