kadircet added a comment.
hi Serge, sorry for the silence here. we'll be moving this towards a production
ready version over the next months (also releasing some comments i've forgotten
to send, we've already discussed these offline so no need for action).
we'll probably be leaving some todos/fixmes as we go, if you want to help it'd
be great to keep an eye on changes going into the
clang-tools-extra/include-cleaner directory (i'll also try to add you as a
subscriber to them) and mention things you'd be interested in picking up.
================
Comment at: clang-tools-extra/clang-tidy/misc/UnusedIncludesCheck.cpp:60
+ });
+ for (const auto &I : RecordedPP->Includes.all()) {
+ if (!Used.contains(&I)) {
----------------
i guess one annyoing thing we're missing here is communication of `IWYU export
/ keep` like pragmas. It feels like handling of such will now be distributed
across all users, which feels unfortunate.
maybe we can address that by either completely deleting such headers from
`RecordedPP->Includes.all` or having some helper like `shouldKeep` in
RecordedPP->Includes (similar to match).
================
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:50
+ Symbol(NamedDecl *ND) : Target(ND) {}
+ Symbol(const DefinedMacro *M) : Target(M) {}
+
----------------
i think we might want to have some pointers to AnalysisContext owning these
DefinedMacros, it's pretty surprising for the user otherwise.
================
Comment at: clang-tools-extra/include-cleaner/lib/Headers.cpp:37
+ case Location::StandardLibrary:
+ // FIXME: some symbols are provided by multiple stdlib headers:
+ // - for historical reasons, like size_t
----------------
feels like we might need some language-based hints here. like stdio.h is C-like.
for the multiple header cases, i suppose it's fine to put them in an order that
we think is "common", while still providing the others. so that unused includes
usecase won't annoy folks and missing includes can provide sane defaults.
i am not sure if we care much about "guaranteed umbrella headers" case. but in
such a scenario, i guess we'd want a policy, that way missing includes can be
suppressed but unused includes can still fire (basically a flip that returns
only the specific header vs both specific and umbrella header).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122677/new/
https://reviews.llvm.org/D122677
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
