janisozaur / include-what-you-use

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

Headers not corresponding to .cpp files are ignored #77

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?  Give the *exact* arguments passed
to include-what-you-use, and attach the input source file that gives the
problem (minimal test-cases are much appreciated!)

A.cpp:
"""
#include "A.h"

int main() {
        A a;
        return a;
}
"""

A.h:
"""
#include "B.h"

class A : public B {};
"""

B.h:

"""
#include "C.h"

class B {
public:
        operator int() { return 42; }
};
"""

C.h is empty.  Command and output:

"""
$ include-what-you-use A.cpp

(A.h has correct #includes/fwd-decls)

(A.cpp has correct #includes/fwd-decls)
"""

What is the expected output? What do you see instead?

B.h is indirectly included, so it should be analyzed, and iwyu should report 
that it unnecessarily includes C.h.

What version of the product are you using? On what operating system?

LLVM r160710, iwyu r367, Ubuntu 12.04.

Please provide any additional information below.

Issue 16 comment 3 suggests this is intentional.  It doesn't explain why, and 
this isn't documented anywhere I see on the wiki.  Mozilla has many abstract 
base classes that are defined in headers with no corresponding .cpp files; I 
would expect iwyu to analyze these files by default.  Either iwyu should be 
updated so it does do this analysis, or the limitation should be documented and 
explained on the wiki.

Original issue reported on code.google.com by a...@aryeh.name on 5 Aug 2012 at 11:05

GoogleCodeExporter commented 9 years ago
I think this is by design. If IWYU were to analyze all #included files, it 
would eventually suggest changing system headers, etc. I assume that's why it 
stops after associated headers (i.e. headers that "belong to" the main 
compilation unit).

I believe this is what the --check_also switch is for; you can specify 
additional headers here. Adding --Xiwyu --check_also="B.h" to the command-line 
reports that the include of C.h should be removed.

I'm not sure how practical this is, though... 

Original comment by kim.gras...@gmail.com on 5 Aug 2012 at 11:32

GoogleCodeExporter commented 9 years ago
Sorry, that should have been: -Xiwyu --check_also=B.h

Original comment by kim.gras...@gmail.com on 5 Aug 2012 at 11:38

GoogleCodeExporter commented 9 years ago
--check_also is not practical for a project like Mozilla that has hundreds or 
thousands of such header files.  To ensure that it only checks headers that are 
part of the current project, I suggest iwyu limit checks to everything below a 
given directory.  It would also probably be a good idea to have it make a note 
somehow of what files it's already inspected, so it doesn't repeatedly inspect 
the same file when invoked on many .cpp's that include it.

Original comment by a...@aryeh.name on 5 Aug 2012 at 12:19

GoogleCodeExporter commented 9 years ago
That sounds like a good, simple fix to me.

It would be interesting to hear from the Google folks why they chose the 
main.cc + associated + check_also route... I can only assume it must fit their 
build system better somehow.

One reservation against a single root dir is projects like LLVM + Clang + IWYU, 
where third parties are checked out into the same source tree. Running IWYU on 
LLVM would include Clang and IWYU if they were checked out. But it feels like 
that would be easy to solve with an exclude-list or something.

I agree with you in principle, but I'd like to hear more opinions on this.

Thanks!

Original comment by kim.gras...@gmail.com on 5 Aug 2012 at 12:57

GoogleCodeExporter commented 9 years ago
I'm guessing it was done this way just because it was easier.  iwyu is invoked 
by the build system on each .cpp.  So it's easy for it to just process the .cpp 
it's given plus the corresponding .h in the same directory (if any).  Trying to 
figure out which other includes to process, and trying to process each only 
once when it's invoked (perhaps in parallel) separately for each .cpp, is 
probably more complicated.

Maybe there's another reason, though.

Original comment by a...@aryeh.name on 5 Aug 2012 at 1:09

GoogleCodeExporter commented 9 years ago
Yes, that occurred to me too after some thinking. I guess it's hard if not 
intractable to keep multiple IWYU runs from clobbering changes to the same 
shared header files.

That said, I think it makes sense to offer the choice -- being able to process 
entire trees in one go can be useful, especially if you do it carefully, 
starting from the leaves of the dependency tree and working inwards. At some 
point, all shared headers would have been fixed, and IWYU won't find anything 
to do for them.

Original comment by kim.gras...@gmail.com on 5 Aug 2012 at 2:45