sammccall accepted this revision. sammccall added a comment. LG, thanks!
================ Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:33 + for (auto &Loc : locateSymbol(S)) + // FIXME: findHeaders in theory returns the same result for all source + // locations in the same FileID. Have a cache for that, since we can have ---------------- nit: I think this would be better as a vaguer comment at a higher level (in walkUsed) like "it might be useful to cache some parts of this computation to save repeated work" ================ Comment at: clang-tools-extra/include-cleaner/test/multiple-providers.cpp:1 +// RUN: clang-include-cleaner --print=changes %s -- -I %S/Inputs | FileCheck \ +// RUN: --allow-empty %s ---------------- nit: splitting RUN lines over lines makes copy/pasting/debugging lit tests harder and ends up being net-negative IMO. I've added a `.clang-format` file so that >80 col lines won't get split Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138200/new/ https://reviews.llvm.org/D138200 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits