Disabling automatic insertion of include

Hi!

In some cases, I find myself fighting against clangd automatically adding includes when choosing a completion item. Is there a way to disable this feature? If not, is there any opposition to adding one?

One such example is when using glib. The users of glib are supposed to only include glib.h, trying to include the more specific header files (such as glib/gscanner.h) results in an error:

/usr/include/glib-2.0/glib/gscanner.h:29:2: error: #error "Only <glib.h> can be included directly."
  #error "Only <glib.h> can be included directly."
   ^~~~~

When I choose a completion item, such as the GTokenType type, clangd automatically adds an include of "glib/gscanner.h", which I have to constantly remove before building.

Ideally, this feature would always do the right thing in all situations, but since it's unlikely that clangd will ever be 100% right about this, I think it would be useful to be able to disable it.

Thanks!

Simon

Sam has go/phab/D60409 ready to land. It will provide an option to disable the automatic insertion.

Alternatively, clangd respects IWYU private pragma 0 which can be used by library owners to specify alternative (public) include headers for index symbols in private headers.

Awesome!

Didn't know about the IWYU pragmas, thanks for the reference.

Simon

I’m a little hesitant to mention this, as it’s a bit of a pandora’s box, but…
If it’s hard to get prominent libraries like glib patched, we could have a lookup table, and pretend we saw such directives…

FWIW, having a mapping for libraries like glib seems useful even if they do get patched to add IWYU pragmas.
There always a rather large amount of older versions floating around…

IWYU seems to support the equivalent of pragmas, but specified outside the headers,
and calls that "Mappings":

https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUMappings.md

It would probably be possible to write such a mapping file for a specific version of the glib
library. Then, it would be a matter of making clangd read and respect those, like it would
respect the pragmas. But it would be the user's responsibility to configure that properly.

As sugar on top, clangd could always ship with some built-in IWYU mapping files contributed
by the community (is there a repository of such files for various libraries somewhere?). I
am not sure though how it would identify that a certain library is in use (and which version)
to load the appropriate mapping file.

Simon

We actually have something very similar implemented in the code (“clangd/CanonicalIncludes.h”), but we don’t support the IWYU mapping file format.
I’m not sure how much work implementing that is, though.

Can we at least make it less aggressive? I’m working on a GTK app and it keeps inserting includes when I already have the symbols available to me through a different one. Or is that the intent?

Doug.

That’s the intent - we’ve been discussing this recently.
We now have two policies for -header-insertion:
iwyu (default): insert a #include to the “best” header providing the symbol, if it’s not directly included
never: never insert includes
there are other possible policies:
transitive: insert a #include to the best header, if it’s not transitively included (some implementation complexity)
include-what-you-must: insert a #include to the best header, if no decl is available. (I have only a vague idea how to implement this in the presence of index+preamble)

All of these policies (except “never”) are reliant on the “best header” concept, if that’s the thing we’re getting wrong then one option is to improve those heuristics.

Well I have to blame GTK for much of the issue. They have a horrible paradigm enforced in their header files.

#if !defined (GTK_H_INSIDE) && !defined (GTK_COMPILATION)
#error “Only <gtk/gtk.h> can be included directly.”
#endif

I guess the best header in this case is the one that doesn’t cause a compile error :relaxed:.

Thanks!
Doug.

Right!
So we can

  1. say that gtk should have IWYU directives
  2. accept that it won’t, and ship them ourselves (or blacklist GTK somehow)
  3. decide that tracking libraries’ header practices is a losing battle, and use a less aggressive policy than IWYU

The one i called “transitive” would work ok in practice here - with no #includes you’d get the wrong one inserted, but once the umbrella include is added by hand, we’re set for that file.

Problem is, giving up on IWYU for everything because one dependency doesn’t like it is a bit sad. Itd be better if we could detect the right strategy when indexing these headers.

I vote for 3. IWYU is not universal and therefore should not be the default.

~Theodore

~Theodore

Bit of an update here:

I vote for 3. IWYU is not universal and therefore should not be the default.
Nothing is universal (except death and taxes?). All the available options cause problems in some situations.“insert if not transitively included” will exhibit the same bugs as IWYU, but less often. In addition, it tends to create code health problems over time.
As style goes, some codebases prefer relying on transitive includes, and others ban it.

It’s not clear it would be a better default, though it might be! We’d need to implement it as an option first.
Patches definitely welcome. I do think the team will get to it, but it might not be a priority until we have more info on the cases where it’s the right policy.