[clang-tidy] Added check 'misc-visibility-change-to-virtual-function'

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -30,6 +30,7 @@
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
 #include "ForwardingReferenceOverloadCheck.h"
+#include "FunctionVisibilityChangeCheck.h"
 #include "ImplicitWideningOfMultiplicationResultCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "IncDecInConditionsCheck.h"
@@ -143,6 +144,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-forward-declaration-namespace");
     CheckFactories.registerCheck<ForwardingReferenceOverloadCheck>(
         "bugprone-forwarding-reference-overload");
+    CheckFactories.registerCheck<FunctionVisibilityChangeCheck>(
+        "bugprone-function-visibility-change");
     CheckFactories.registerCheck<ImplicitWideningOfMultiplicationResultCheck>(
         "bugprone-implicit-widening-of-multiplication-result");
     CheckFactories.registerCheck<InaccurateEraseCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -26,6 +26,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   FoldInitTypeCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp
   ForwardingReferenceOverloadCheck.cpp
+  FunctionVisibilityChangeCheck.cpp
   ImplicitWideningOfMultiplicationResultCheck.cpp
   InaccurateEraseCheck.cpp
   IncorrectEnableIfCheck.cpp

clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp

@@ -0,0 +1,74 @@
+//===--- FunctionVisibilityChangeCheck.cpp - clang-tidy -------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "FunctionVisibilityChangeCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void FunctionVisibilityChangeCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxMethodDecl(
+          ofClass(cxxRecordDecl().bind("class")),
+          forEachOverridden(cxxMethodDecl(ofClass(cxxRecordDecl().bind("base")))
+                                .bind("base_func")))
+          .bind("func"),
+      this);
+}
+
+void FunctionVisibilityChangeCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedFunction = Result.Nodes.getNodeAs<FunctionDecl>("func");
+  const auto *ParentClass = Result.Nodes.getNodeAs<CXXRecordDecl>("class");
+  const auto *OverriddenFunction =
+      Result.Nodes.getNodeAs<FunctionDecl>("base_func");
+  const auto *BaseClass = Result.Nodes.getNodeAs<CXXRecordDecl>("base");
+
+  if (!MatchedFunction->isCanonicalDecl())
+    return;
+
+  AccessSpecifier ActualAccess = MatchedFunction->getAccess();
+  AccessSpecifier OverriddenAccess = OverriddenFunction->getAccess();
+
+  CXXBasePaths Paths;
+  if (!ParentClass->isDerivedFrom(BaseClass, Paths))
+    return;
+  const CXXBaseSpecifier *InheritanceWithStrictVisibility = nullptr;
+  for (const CXXBasePath &Path : Paths) {
+    for (auto Elem : Path) {
+      if (Elem.Base->getAccessSpecifier() > OverriddenAccess) {
+        OverriddenAccess = Elem.Base->getAccessSpecifier();
+        InheritanceWithStrictVisibility = Elem.Base;
+      }
+    }
+  }
+
+  if (ActualAccess != OverriddenAccess) {
+    if (InheritanceWithStrictVisibility) {
+      diag(MatchedFunction->getLocation(),
+           "visibility of function %0 is changed from %1 (through %1 "
+           "inheritance of class %2) to %3")
+          << MatchedFunction << OverriddenAccess
+          << InheritanceWithStrictVisibility->getType() << ActualAccess;
+      diag(InheritanceWithStrictVisibility->getBeginLoc(),
+           "this inheritance would make %0 %1", DiagnosticIDs::Note)
+          << MatchedFunction << OverriddenAccess;
+    } else {
+      diag(MatchedFunction->getLocation(),
+           "visibility of function %0 is changed from %1 in class %2 to %3")
+          << MatchedFunction << OverriddenAccess << BaseClass << ActualAccess;
+    }
+    diag(OverriddenFunction->getLocation(), "function declared here as %0",
+         DiagnosticIDs::Note)
+        << OverriddenFunction->getAccess();
+  }
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h

@@ -0,0 +1,33 @@
+//===--- FunctionVisibilityChangeCheck.h - clang-tidy -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_FUNCTIONVISIBILITYCHANGECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_FUNCTIONVISIBILITYCHANGECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Check function visibility changes in subclasses.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/function-visibility-change.html
+class FunctionVisibilityChangeCheck : public ClangTidyCheck {
+public:
+  FunctionVisibilityChangeCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_FUNCTIONVISIBILITYCHANGECHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -124,6 +124,11 @@ New checks
   pointer and store it as class members without handle the copy and move
   constructors and the assignments.
 
+- New :doc:`bugprone-function-visibility-change
+  <clang-tidy/checks/bugprone/function-visibility-change>` check.
+
+  Check function visibility changes in subclasses.
+
 - New :doc:`bugprone-unintended-char-ostream-output
   <clang-tidy/checks/bugprone/unintended-char-ostream-output>` check.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst

@@ -0,0 +1,43 @@
+.. title:: clang-tidy - bugprone-function-visibility-change
+
+bugprone-function-visibility-change
+===================================
+
+Check changes in visibility of C++ member functions in subclasses. The check
+detects if a virtual function is overridden with a different visibility than in
+the base class declaration. Only normal functions are detected, no constructors,
+operators, conversions or other special functions.
+
+.. code-block:: c++
+
+  class A {
+  public:
+    virtual void f_pub();
+  private:
+    virtual void f_priv();
+  };
+
+  class B: public A {
+  public:
+    void f_priv(); // warning: changed visibility from private to public
+  private:
+    void f_pub(); // warning: changed visibility from public to private
+  };
+
+  class C: private A {
+    // no warning: f_pub becomes private in this case but this is from the
+    // private inheritance
+  };
+
+  class D: private A {
+  public:
+    void f_pub(); // warning: changed visibility from private to public
+                  // 'f_pub' would have private access but is forced to be
+                  // public
+  };
+
+The changed visibility can be an indicator of bad design or a result of
+coding error or code changes. If it is intentional, it can be avoided by
+adding an additional virtual function with the new access.
+
+

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -98,6 +98,7 @@ Clang-Tidy Checks
    :doc:`bugprone-fold-init-type <bugprone/fold-init-type>`,
    :doc:`bugprone-forward-declaration-namespace <bugprone/forward-declaration-namespace>`,
    :doc:`bugprone-forwarding-reference-overload <bugprone/forwarding-reference-overload>`,
+   :doc:`bugprone-function-visibility-change <bugprone/function-visibility-change>`, "Yes"
    :doc:`bugprone-implicit-widening-of-multiplication-result <bugprone/implicit-widening-of-multiplication-result>`, "Yes"
    :doc:`bugprone-inaccurate-erase <bugprone/inaccurate-erase>`, "Yes"
    :doc:`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions>`,