sammccall added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:70
QualType Type = E->getBase()->IgnoreImpCasts()->getType();
report(E->getMemberLoc(), resolveType(Type));
return true;
----------------
hokein wrote:
> sammccall wrote:
> > only tangentially related, but should these be implicit references?
> >
> > I don't think we actually want to insert headers based on them, right? Just
> > allow people to keep the ones that they have inserted that are required for
> > compilation?
> >
> > This would also make it less severe if our heuristic above goes wrong.
> > I don't think we actually want to insert headers based on them, right?
>
> I think no? We want to insert headers, below is the motivated case
>
> ```
> // all.h
> #include "foo.h"
> #include "declare.h"
>
> // declare.h
> class Foo;
> Foo& getFoo();
>
> // in main.cc
> #include "all.h"
> void k() {
> getFoo().member;
> }
> ```
>
> After the cleanup, we expect: `all.h` is removed, `declare.h`, and `foo.h`
> are inserted, right? I think this is the case we should care about, it is a
> simple version of protobuf case.
I would expect `declare.h` to be removed and `foo.h` to be inserted, and then
the user to have to insert `Foo.h`.
The problem with inserting `"foo.h"` here is that we'll do it even if
`declare.h` includes `foo.h`, which is a false positive according to the IWYS
principle.
> I think this is the case we should care about, it is a simple version of
> protobuf case.
I think the plan was to have only limited support for forward-declarations
(because we can't reason about whether a full declaration is needed), and
consider special-casing protos later because we do understand in that case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140095/new/
https://reviews.llvm.org/D140095
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
