[PATCH] D138219: [include-cleaner] Show includes matched by refs in HTML report.Demo: https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/ecee6869e37af3db28089b64d8dce806/raw/8736e64c45af411e2c2d72adaed2dfc4410a5b36/ASTTests.html%25202

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks, I like the idea of marking missing-include refs.

I haven't read the code yet, and below are all my findings when I played with the demo html (some of them are unrelated to the current patch, just want to dump all):

1. size_t shows duplicated entries, line 465
2. we are missing refs in some using declarations (line 31, line32), but line 33 does have a link which seems weird
3. UI related:
  - it would be nice to highlight the whole line, if user clicks the line link from the shown-up panel;
  - for `Included path/to/header.h`, I think adding the `""`/`<>` around the spelling string will be nicer;
  - for main-file symbols, showing (`Header  ASTTests.cpp`) is suboptimal, but I don't have better solution;
4. The handling of std symbols seems inconsistent:
  - click on a vector `push_back` will give the mapping `vector` header;
  - click on the type `std::vector` will give the `iosfwd` header;
5. I was confused why the type `Annotation` has the `Included` file, but not the method call `.code()`, since both shown-up panel are showing `Annotations.h` header, then I realized that the `code` is from the base class `llvm::Annotation`. This brings up a policy question (Not sure it has been discussed before): If we have already included the header of a subclass, and we have some base-class method calls via the subclass object, do we need to insert the header of the base class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138219