[PATCH] D136293: [IncludeCleaner] Add public API

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:27
+  Symbol(Decl &D) : Storage(&D) {}
+  Symbol(tooling::stdlib::Symbol S) : Storage(S) {}
+
----------------
Doh, I was skimming and missed that you removed the `Location` concept and folded stdlib handling into `Symbol`.

`tooling::stdlib::Symbol` only captures top-level symbols, so we've thrown away the identity of the actual symbol being accessed. This is not a complete disaster, but I think it causes problems, and I think we should be very deliberate and consistent about what information we preserve vs throw away.

Some examples of how this might bite us:
 - distinguishing between macros/decls is non-uniform (need to check separately for decl vs macro vs stdlib-decl vs stdlib-macro)
 - can't treat different types of symbols e.g. constructors vs classes differently (you suggested elsewhere we could check the kind of targetdecl to implement external policies)
 - can't treat nested classes differently (i.e. must treat `std::vector` and `decltype(myvec)::iterator` the same). 
 - can't print the name, e.g. the fixit hint for `auto x = TimePoint<>::min()` becomes `include header <chrono> for 'std::chrono::time_point'` instead of `include header <chrono> for 'std::chrono::time_point<...>::min()'`
 - can't insert forward declarations with a code action (legal for c symbols)
 - harder to understand/debug (is this an implicit reference to a copy constructor, move constructor, conversion operator, or the type itself? where *are* all the redecls? is this misclassified user code, such as a trait specialization)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136293/new/

https://reviews.llvm.org/D136293