aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:428 +of our new checks. + +.. code-block:: console ---------------- We should also document our expectations explicitly regarding things like ABI and API stability. e.g., remind users that there is none and so the plugin must be compiled against the version of clang-tidy that will be loading the plugin. (We should also make sure we're correct about that; I don't know if Clang 13 and Clang 13.0.1 can share plugins or not.) Another thing we should document is whether the plugins can or cannot use threads or TLS, or other details along those lines that developers need to be aware of. ================ Comment at: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp:24-25 + + //void registerMatchers(ast_matchers::MatchFinder *Finder) override; + //void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + ---------------- vtjnash wrote: > aaron.ballman wrote: > > Shouldn't these functions be overloaded? We don't need it to be > > particularly functional, but the plugin should demonstrate that it works > > and can be run by clang-tidy (not just loaded and listed as a check). > I figured that is guaranteed by the C++ linker, if it can successfully list > the check, so I didn't think it seemed essential to test for that also. I put > these here mostly just to help anyone copying the file to getting started > with adopting it to their use case. I think it's important to demonstrate that the functionality works, and I think it's even more important to ensure that plugin support is maintained as new features are added to clang-tidy. "It links" is insufficient to tell us that. One possible approach to this is to write and maintain a simple check as a plugin, then test it using the normal infrastructure (additional command line arg to load the plugin notwithstanding). We could either make it a functional check that users can optionally use (perhaps the reason it's a plugin is because it relies on a third-party library that may not always be available on all users' systems), or we could make it a toy check that only exists to test tidy features like plugins, plugin config options, etc. The kinds of things I want to verify work are things like: does the plugin name get properly emitted as part of the diagnostics from the check? Are config options properly loaded for the plugin? Can a plugin alias a builtin check and provides a different set of config options? We should also verify the tool gracefully handles a plugin that isn't valid (e.g., ask to load an arbitrary shared library as a plugin, ensure that tidy doesn't crash). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111100/new/ https://reviews.llvm.org/D111100 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
