Minimize the number of include directives

GoogleCodeExporter commented 9 years ago

The current state of iwyu (3.3) in case of a big library like glib or gtk+, is 
that it will propose to remove the single #include <main_lib_header.h>, and 
replace it with a whole lot of #includes for more specific headers 
<lib_inc_dir/specific1.h> to <lib_inc_dir/specific100.h>. In one sense, that is 
what iwyu is about, I know.

But I would be interested in a mode which wouldn't propose to remove a single 
#include line and replace it with many. It would just detect useless includes.

I know the information I want is inside the current behaviour, but it's hidden 
by the huge number of suggestions.

Original issue reported on code.google.com by julien.puydt on 8 Sep 2013 at 11:29

GoogleCodeExporter commented 9 years ago

Here is a small example showing the issue. There we see iwyu suggest removing a 
useless include (good!) but also removing a single good include to replace it 
by five others!

It might also be problematic to follow the suggestion, since the code wouldn't 
#include <stdio.h> anymore... but that might be another bug!

Original comment by julien.puydt on 8 Sep 2013 at 12:25

Added labels: ****
Removed labels: ****

Attachments:

too_many_includes.tar.bz2

GoogleCodeExporter commented 9 years ago

To be more explicit about the example:
- there are five include files biglib/feature1.h to biglib/feature5.h, each 
defining a #FEATUREN printf("Feature N\n")
- there is an empy useless.h
- there is a biglib.h which #includes <stdio.h> and the five biglib/featureN.h
- test.c #includes useless.h and biglist.h

If you ask for iwyu's opinion, it will ask to ditch the two #includes, and 
replace them with five ones to the biglib/featureN.h, so this one example:

- shows that iwyu should give an option to just suggest to remove the useless 
includes (and not go on suggesting a 1-to-5 change) ; this is a wishlist-level 
problem ;

- shows that iwyu will happily suggest removing a much-needed inclusion of 
stdio.h, and following its advice will break the compilation ; this is a 
grave-level problem.

Original comment by julien.puydt on 8 Sep 2013 at 12:38

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

It looks like IWYU is doing what it's supposed to.

If you want to solve the facade/detail problem (your wishlist-level one), you 
can use IWYU pragmas 
(https://code.google.com/p/include-what-you-use/wiki/IWYUPragmas) or IWYU 
mapping files 
(https://code.google.com/p/include-what-you-use/wiki/IWYUMappings).

As for the stdio.h problem, I think IWYU should be suggesting to move #include 
<stdio.h> to the individual feature headers; they are the ones using printf, 
and they are essentially re-exporting printf under a different name. If it 
doesn't, I would consider that a bug.

Original comment by kim.gras...@gmail.com on 8 Sep 2013 at 2:56

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

It doesn't suggest to move the #include <stdio.h> into the individual headers ; 
in fact it doesn't mention it at all... but the suggestions have as consequence 
to make it disappear, which is pretty bad.

The mappings feature looks like something I could use ; it will be annoying 
that the include directives don't look like it's possible/easy to use 
wildcards, but I think some python script magic will help there.

Original comment by julien.puydt on 10 Sep 2013 at 6:51

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

I've forgotten to document it, but you should be able to use @-prefixed regexes 
in the from-mappings, e.g.

  { include: ["@<internal/.*>", "private", "<public>", "public"] }

Not sure how far that gets you.

Original comment by kim.gras...@gmail.com on 10 Sep 2013 at 7:12

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

That doesn't bring me as far as I wanted: I created an ekiga.imp which 
references glib.imp and gtk.imp, where I use regexes as explained. But the 
project is autotools-based ; I was testing things like this:
  make CC=iwyu CXX=iwyu V=1 > /tmp/iwyu.log 2>&1
but if I try to add the "--mapping_file=..." in there (either in CC/CXX or 
through CFLAGS/CXXFLAGS), the autotools take the argument as being for them!

Is there a way I can send an option to iwyu directly (either an environment 
variable or a configuration file)?

Original comment by julien.puydt on 11 Sep 2013 at 6:45

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

I don't know anything about autotools, but there must be a way to funnel 
arguments through to $CXX, right?

Unix is not my primary platform, but I'd try to add mappings to CXXFLAGS. Don't 
forget to prefix with -Xiwyu, e.g.

  CXXFLAGS=-Xiwyu --mapping_file=...

Sorry I can't help more, maybe there are other autotools users out there.

Original comment by kim.gras...@gmail.com on 11 Sep 2013 at 6:53

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

Ah, indeed -Xiwyu was missing in my tests. Now I get it to run correctly, but:
 { include: ["@<glib/.*>", private, "<glib.h>", public]},
still makes iwyu suggest to add "glib/gmacros.h" and "glib/gtypes.h", and 
remove <glib.h>...

Original comment by julien.puydt on 11 Sep 2013 at 7:20

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

Note: if you want to document better how one can use iwyu with autotools, I 
compile using:
 make CFLAGS="-Xiwyu --mapping_file=/home/jpuydt/Logiciels/Ekiga/ekiga.imp" CXXFLAGS="-Xiwyu --mapping_file=/home/jpuydt/Logiciels/Ekiga/ekiga.imp" CC=iwyu CXX=iwyu V=1 -k > /tmp/iwyu.log 2>&1

Original comment by julien.puydt on 11 Sep 2013 at 7:22

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

The quotes are significant (though I'm not sure why, that decision predates me 
:-)), so you may have better luck with:

 { include: ["@\"glib/.*\"", private, "<glib.h>", public]},

or whatever the escape syntax for quotes is in JSON.

Thanks for the concrete example of autotools use, I'll try and add it to the 
HowToUse wiki page.

Original comment by kim.gras...@gmail.com on 11 Sep 2013 at 8:06

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

That's better indeed, but now I find situations where iwyu tells me to remove a 
#include, and tells me it is good where it is!

See:
../../ekiga.git/lib/platform/platform.c should add these lines:
#include <stddef.h>                     // for NULL

../../ekiga.git/lib/platform/platform.c should remove these lines:
- #include <gtk/gtk.h>  // lines 38-38

The full include-list for ../../ekiga.git/lib/platform/platform.c:
#include "platform.h"
#include <gtk/gtk.h>                    // for gtk_show_uri, etc
#include <stddef.h>                     // for NULL

Original comment by julien.puydt on 11 Sep 2013 at 11:56

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

Without more context, I can imagine two things:

1. gtk/gtk.h has been included twice, and IWYU removes one of them
2. "// for ..." comments are added, and therefore the #include is rewritten

I don't think IWYU does (2), so I'm leaning towards (1).

Original comment by kim.gras...@gmail.com on 11 Sep 2013 at 6:50

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

I just realized I hadn't tested your attached example, I was disracted by the 
follow-on discussion.

> - shows that iwyu should give an option to just suggest to remove the useless
> includes (and not go on suggesting a 1-to-5 change) ; this is a wishlist-level
> problem ;

I think we covered this, the inlining of biglib feature headers is by design. 
Without metadata for a library, we don't know which headers are private and 
which are public. Mappings should help here (even if they're inconvenient.)

> - shows that iwyu will happily suggest removing a much-needed inclusion
> of stdio.h, and following its advice will break the compilation ; this
> is a grave-level problem.

Ah, so IWYU losing stdio is a consequence of inlining the feature headers?

I think that's because IWYU does not modify headers by default (other than the 
'associated' header, i.e. the one with the same basename as the .c file being 
processed.)

You can let IWYU know which headers it it allows to change in its analysis 
using the --check_also switch:

$ include-what-you-use.exe test.c -I. -Ibiglib -Xiwyu 
--check_also=biglib/feature*.h

When I add that, IWYU suggests that #include <stdio.h> be added in all feature 
headers, and then inlines biglib.h and removes useless.h.

That's one of the challenges with IWYU, that it can't do a big-bang fix of an 
entire codebase. It works translation unit by translation unit, and shared 
headers are not implicitly fixed (because it would run the risk of conflicts). 
The user has to explicitly --check_also in the context of one translation unit. 
IWYU for larger projects would have to run several passes to fix #include 
dependencies inside-out.

Original comment by kim.gras...@gmail.com on 6 Oct 2013 at 7:36

Added labels: ****
Removed labels: ****

GoogleCodeExporter commented 9 years ago

I'd love to hear more about the problem in comment #11, though.

Can you run IWYU for platform.c with -Xiwyu --verbose=7 and upload the log 
output?

Original comment by kim.gras...@gmail.com on 6 Oct 2013 at 7:39

Added labels: ****
Removed labels: ****

HaveF / include-what-you-use

Minimize the number of include directives #108