sarojvarma / include-what-you-use

Automatically exported from code.google.com/p/include-what-you-use
Other
0 stars 0 forks source link

choose parent #include file when conditionally incuding #56

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Say, file sys/cdefs.h contains:

#ifdef __ELF__
#include <sys/cdefs_elf.h>
#else
#include <sys/cdefs_aout.h>
#endif

iwyu suggests removing the #include for sys/cdefs.h and adding an #include 
<sys/cdefs_elf.h>

If the #include is conditional, iwyu should use the top-most ancestor include 
file that is unconditional.

Original issue reported on code.google.com by MarkoSch...@googlemail.com on 23 Jul 2011 at 3:17

GoogleCodeExporter commented 9 years ago
It's an interesting idea!  I've tried to come up with an example where this 
isn't a good idea, but it's hard for me to.  That's not to say there aren't 
any; I just don't have a feel for how often it occurs.

My basic philosophy for system includes is to not try to come up with rules, 
but to just hard-code knowledge in iwyu_include_picker.cc.  It would certainly 
be easy to add a mapping from cdefs_*.h to cdefs.h, which is how I would try to 
solve this immediate problem.  But I wonder if your idea is a good one to try 
to generalize things.  Feel free to draw up a patch that implements it if you 
like, and we can play around with it.

Original comment by csilv...@gmail.com on 25 Jul 2011 at 9:06

GoogleCodeExporter commented 9 years ago
There's a saying that you should generalize something only after needing it the 
third time...

Original comment by rcs...@gmail.com on 26 Jul 2011 at 6:56

GoogleCodeExporter commented 9 years ago

Original comment by csilv...@gmail.com on 25 Oct 2011 at 1:41

GoogleCodeExporter commented 9 years ago
I must say that this issue prevents iwyu from being used on any real project 
that compiles for multiple platforms.

Original comment by tripleQu...@gmail.com on 26 Jun 2012 at 4:11

GoogleCodeExporter commented 9 years ago
I ran into this issue as well, and am looking forward to a fix.

Original comment by plaztiks...@gmail.com on 16 Jul 2012 at 8:58

GoogleCodeExporter commented 9 years ago
We have a lot of old code that looks like this:

header.h:
#ifndef HEADER_H
...
#endif

top_header.h:
#ifndef HEADER_H
#include "header.h"
#endif

...

In other words, there are external include guards. Old compilers used to be 
very bad at including headers, so it was more efficient to put an additional 
include guard in the file that does the include.

Terrible practice nowadays, but it still happens.
What effect would fixing this bug have on the above?

Original comment by mathias....@gmail.com on 4 Jan 2013 at 9:06

GoogleCodeExporter commented 9 years ago
Mathias; thanks for the data point!

I think about this bug from time to time, but I can't come up with a systematic 
solution. The original report suggests that any conditional #include should be 
considered a private header, and I think that's too strong a policy. Your 
external include guards example reinforces that.

We pulled the mappings from iwyu_include_picker.cc out into external mapping 
files (*.imp), so this should be easy to patch locally. I'll post documentation 
of the mapping files as soon as I can.

That said, if someone can think of a principled solution for this specific 
class of problems I'm all ears!

Original comment by kim.gras...@gmail.com on 4 Jan 2013 at 9:20

GoogleCodeExporter commented 9 years ago
Here's another example (from K&R):

   #if SYSTEM == SYSV
       #define HDR "sysv.h"
   #elif SYSTEM == BSD
       #define HDR "bsd.h"
   #elif SYSTEM == MSDOS
       #define HDR "msdos.h"
   #else
       #define HDR "default.h"
   #endif
   #include HDR

Conditional #include detection would fail here as well, because the #include is 
not in a condition, just the definition of which header to include.

Original comment by kim.gras...@gmail.com on 7 Jan 2013 at 1:17