Closed GoogleCodeExporter closed 9 years ago
I don't agree.
"there is no possible way that B.h will ever not include A.h unless the include
hierarchy changes"
Unfortunately, class hierarchies *do* change. B might change from being a
subclass of A (is-a) to containing a reference of A (has-a); then B.h does not
need to include A.h because a forward reference is enough. If test.cpp only
included B.h, it would suddenly fail to compile.
The point of iwyu is precisely to make code resilient in face of change.
Original comment by rcs...@gmail.com
on 11 Jul 2012 at 10:33
Okay, yes, it's possible in some cases. But in some cases the inheritance is
fundamental to the class' purpose, and using human judgment we can figure that
it will never change -- or if it does, the changes needed to users would be
enough that changing a few includes is no big deal. E.g., I can tell you that
as long as the classes "nsIDOMNode" and "nsIDOMElement" exist in Mozilla, the
latter will inherit from the former!
Would it be possible to identify constructs that *require* that one class
derive from another? For instance, suppose I did:
A* a = new B();
or:
B* b = static_cast<B*>instanceOfA;
Then the file won't compile anyway if the class hierarchy changes. I suspect
this would be a large fraction of the cases I'm looking at. But maybe this
would be overly complicated to implement, I dunno.
Anyway, I see your point, but also don't think unnecessary includes should be
viewed as without cost -- even if it's just an aesthetic thing. At a minimum,
maybe there should be a way to manually annotate certain includes as "assume
that this file will always be included here".
Also -- are these redundant includes really free? Or can it add noticeably to
compilation time to have to re-include the same large header files many times
per compilation unit?
Original comment by a...@aryeh.name
on 11 Jul 2012 at 11:17
> At a minimum, maybe there should be a way to manually
> annotate certain includes as "assume that this file
> will always be included here".
You should be able to do this in B.h with IWYU pragma: export.
B.h:
"""
#include "A.h" // IWYU pragma: export
class B : public A {};
"""
test.cpp:
"""
#include "B.h"
int main() {
A a; // will be satisified by B.h
B b;
return &a && &b ? 123 : -123;
}
"""
Hope that helps,
- Kim
Original comment by kim.gras...@gmail.com
on 13 Jul 2012 at 6:28
Thanks! I didn't find this in the docs before, but now I see
<http://code.google.com/p/include-what-you-use/wiki/InstructionsForUsers> links
to <http://code.google.com/p/include-what-you-use/wiki/IWYUPragmas>, which
describes how to use these pragmas. So this is fine by me; the issue can be
closed as far as I'm concerned.
Original comment by a...@aryeh.name
on 15 Jul 2012 at 6:38
Yes, I added it last night :-)
Could you verify that it solves the problem before we close the issue?
Thanks!
Original comment by kim.gras...@gmail.com
on 15 Jul 2012 at 7:08
There are a few cases when IWYU considers indirect includes to be correct. For
example,
A.h:
"""
class A {};
"""
B.h:
"""
#include "A.h"
class B : public A {
void foo();
};
"""
B.cpp:
"""
#include "B.h"
void B::foo() {}
"""
And b.cpp has correct #includes/fwd-decls.
So before closing the issue I'd like to look at the code and consider if
indirect include is acceptable.
Original comment by vsap...@gmail.com
on 17 Jul 2012 at 9:07
I verify that "// IWYU pragma: export" fixes the issue. Thanks!
Original comment by a...@aryeh.name
on 25 Jul 2012 at 9:20
@Volodymyr, the reason IWYU accepts that is because B.h/B.cpp are considered a
unit -- a header named the same as the .cpp file we're processing is its
associated header, and it makes sense to treat them as one.
We've discussed running IWYU in different modes before (e.g. issue 77) but I
think for the current makefile-driven mode, this is the right behavior.
Original comment by kim.gras...@gmail.com
on 10 Dec 2012 at 1:29
OK, then closing the issue.
Original comment by vsap...@gmail.com
on 11 Dec 2012 at 2:36
Original issue reported on code.google.com by
a...@aryeh.name
on 11 Jul 2012 at 8:15