[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 27 12:01:08 PST 2019

JonasToth added a comment.

Welcome to the LLVM community and thank you for the patch lewmpk!

Please add unit tests for the changes you made to the check. They live in `clang-tools-extra/test/clang-tidy/modernize-....`.
It is common to add a additional test-file to ensure the configuration options do work as expected.
You can get inspiration from other checks that provide configuration capability (e.g. https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-bool-literals.html#options)
for reference.

If you have any questions don't hesitate to ask!

P.S: Usually we upload the patches with full context (https://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch) for the reviewer to see the change in context.

Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:23
+  if (getLangOpts().CPlusPlus11) {
+    if (IgnoreDestructors) {
+      Finder->addMatcher(
Please ellide the braces for the single-statements (this `if` and the `else` branch)

Comment at: clang-tidy/modernize/UseOverrideCheck.h:23
+      : ClangTidyCheck(Name, Context),
+        IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
That requires additional methods for the function for the configuration to function properly. Please see other checks and implement that the same way.

Comment at: clang-tidy/modernize/UseOverrideCheck.h:27
+  const bool IgnoreDestructors;
please use private instead

  rCTE Clang Tools Extra



More information about the cfe-commits mailing list