hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land.
Thanks, looks great! ================ Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h:22 + +class IncludeSpeller { + ---------------- would be nice to add some documentation for this interface, something like ``` IncludeSpeller provides an extension point to allow clients implement custom include spelling strategies applications for physical headers. ``` ================ Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h:23 +class IncludeSpeller { + +public: ---------------- nit: remove the extra blank line. ================ Comment at: clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp:24 +class DefaultIncludeSpeller : public IncludeSpeller { + + std::string operator()(const Input &Input) const override { ---------------- add a `public:`. ================ Comment at: clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp:33 + +std::string mapHeader(const IncludeSpeller::Input &Input) { + static auto Spellers = [] { ---------------- nit: maybe name it `spellPhysicalHeader`( I'd avoid using `map`, it reminds me too much on the stdlib mapping). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits