[PATCH] D153198: [clang-tidy] Refactor common code from the Noexcept*Checks into `NoexceptFunctionCheck`

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Overall good, my main concern is base class name & location.



================
Comment at: clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h:23
 /// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-destructor.html
-class NoexceptDestructorCheck : public ClangTidyCheck {
+class NoexceptDestructorCheck : public utils::NoexceptFunctionCheck {
 public:
----------------
Consider putting "Base" into a name, so that it woudn't be considered a full check, and it can be in this module, no need to put it into utils


================
Comment at: clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h:25-26
 public:
   NoexceptDestructorCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : NoexceptFunctionCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
----------------
we got c++17, so delegate constructor.
using utils::NoexceptFunctionCheck::NoexceptFunctionCheck;


================
Comment at: clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h:28-30
   std::optional<TraversalKind> getCheckTraversalKind() const override {
     return TK_IgnoreUnlessSpelledInSource;
   }
----------------
consider moving this into base class


================
Comment at: clang-tools-extra/clang-tidy/utils/NoexceptFunctionCheck.h:34-38
+  virtual DiagnosticBuilder
+  reportMissingNoexcept(const FunctionDecl *FuncDecl) = 0;
+
+  virtual void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
+                                              const Expr *NoexceptExpr) = 0;
----------------
you can move those into protected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153198/new/

https://reviews.llvm.org/D153198