[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 6 03:10:19 PST 2022


kadircet marked 5 inline comments as done.
kadircet added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/test/html.cpp:7
 int n = foo();
+// CHECK: Symbol{{.*}}foo
+// CHECK-NEXT: int foo()
----------------
sammccall wrote:
> I'd like to keep a very simple smoke test of the HTML report.
> 
> If you're aiming to test something specific here about the HTML report, I'd suggest creating a new test describing what it is. (Though generally speaking, the HTML test has poor test coverage of its details, which I think is unfortunately the right tradeoff)
> 
> If you're aiming to provide an integration test (these locations are taken into account in making overall decisions), I think this should be a test using `-print=changes` rather than -html.
thanks makes sense. `print-changes` didn't exist back in the day when i was putting together this patch and i forgot to update this piece.

i wasn't trying to test HTMLReport behaviour specifically, and it's probably not worth adding that anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138200



More information about the cfe-commits mailing list