logan-5 marked 2 inline comments as done. logan-5 added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:29 +public: + RenamerClangTidyCheck(StringRef CheckName, ClangTidyContext *Context); + ~RenamerClangTidyCheck(); ---------------- logan-5 wrote: > njames93 wrote: > > Should this be protected as this class should never be instantiated > > directly. Also the definition could be moved inline as its just a > > delegating constructor > The constructor (and destructor) can't be inline, since they need to be able > to see the specialization of `DenseMapInfo<NamingCheckId>` in the cpp. > > I could change it to `protected` to clean up the interface -- though it won't > strictly change anything, since the class already has pure virtual functions > so it's not instantiable. Searched the LLVM codebase for other abstract base classes with explicitly defined constructors (e.g. `MakeSmartPtrCheck` in clang-tidy), and their constructors seem to be public. I think I'll keep this one public too for consistency. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72284/new/ https://reviews.llvm.org/D72284 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
