https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/140086
From 65d44a4eb9621e49a96f1ac43e5a1bbd6691dc13 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Thu, 15 May 2025 17:41:16 +0200
Subject: [PATCH 1/3] [clang-tidy] Added check
'bugprone-function-visibility-change'
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
.../FunctionVisibilityChangeCheck.cpp | 74 ++++++
.../bugprone/FunctionVisibilityChangeCheck.h | 33 +++
clang-tools-extra/docs/ReleaseNotes.rst | 5 +
.../bugprone/function-visibility-change.rst | 43 ++++
.../docs/clang-tidy/checks/list.rst | 1 +
.../bugprone/function-visibility-change.cpp | 234 ++++++++++++++++++
8 files changed, 394 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index b780a85bdf3fe..7cf58c5218969 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/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>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e310ea9c94543..b4f7ba76f4cee 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/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
diff --git
a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
new file mode 100644
index 0000000000000..7ea4ed20705ed
--- /dev/null
+++ b/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
diff --git
a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
new file mode 100644
index 0000000000000..ec7152fb63acb
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 579fca54924d5..52f61a78fa3b7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/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.
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
new file mode 100644
index 0000000000000..3b7940e5e584e
--- /dev/null
+++
b/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.
+
+
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 18f1467285fab..59653a2059e64 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/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>`,
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
new file mode 100644
index 0000000000000..eb4532374267b
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
@@ -0,0 +1,234 @@
+// RUN: %check_clang_tidy %s bugprone-function-visibility-change %t
+
+class A {
+public:
+ virtual void pub_foo1() {}
+ virtual void pub_foo2() {}
+ virtual void pub_foo3() {}
+protected:
+ virtual void prot_foo1();
+ virtual void prot_foo2();
+ virtual void prot_foo3();
+private:
+ virtual void priv_foo1() {}
+ virtual void priv_foo2() {}
+ virtual void priv_foo3() {}
+};
+
+void A::prot_foo1() {}
+void A::prot_foo2() {}
+void A::prot_foo3() {}
+
+namespace test1 {
+
+class B: public A {
+public:
+ void pub_foo1() override {}
+ void prot_foo1() override {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from protected in class 'A' to public
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :9:16: note: function declared here as protected
+ void priv_foo1() override {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo1' is changed from private in class 'A' to public
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :13:16: note: function declared here as private
+protected:
+ void pub_foo2() override {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo2' is changed from public in class 'A' to protected
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :6:16: note: function declared here as public
+ void prot_foo2() override {}
+ void priv_foo2() override {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo2' is changed from private in class 'A' to protected
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :14:16: note: function declared here as private
+private:
+ void pub_foo3() override {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo3' is changed from public in class 'A' to private
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :7:16: note: function declared here as public
+ void prot_foo3() override {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo3' is changed from protected in class 'A' to private
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :11:16: note: function declared here as protected
+ void priv_foo3() override {}
+};
+
+class C: public B {
+public:
+ void pub_foo1() override;
+protected:
+ void prot_foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from public in class 'B' to protected
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :27:8: note: function declared here as public
+private:
+ void priv_foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo1' is changed from public in class 'B' to private
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :30:8: note: function declared here as public
+};
+
+void C::prot_foo1() {}
+void C::priv_foo1() {}
+
+}
+
+namespace test2 {
+
+class B: public A {
+public:
+ void pub_foo1() override;
+protected:
+ void prot_foo1() override;
+private:
+ void priv_foo1() override;
+};
+
+class C: public B {
+public:
+ void pub_foo1() override;
+ void prot_foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from protected in class 'B' to public
+ // CHECK-MESSAGES: :75:8: note: function declared here as protected
+ void priv_foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo1' is changed from private in class 'B' to public
+ // CHECK-MESSAGES: :77:8: note: function declared here as private
+
+ void pub_foo2() override;
+ void prot_foo2() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo2' is changed from protected in class 'A' to public
+ // CHECK-MESSAGES: :10:16: note: function declared here as protected
+ void priv_foo2() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo2' is changed from private in class 'A' to public
+ // CHECK-MESSAGES: :14:16: note: function declared here as private
+};
+
+}
+
+namespace test3 {
+
+class B: private A {
+public:
+ void pub_foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo1' is changed from private (through private inheritance of class 'A')
to public
+ // CHECK-MESSAGES: :103:10: note: this inheritance would make 'pub_foo1'
private
+ // CHECK-MESSAGES: :5:16: note: function declared here as public
+protected:
+ void prot_foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from private (through private inheritance of class 'A')
to protected
+ // CHECK-MESSAGES: :103:10: note: this inheritance would make 'prot_foo1'
private
+ // CHECK-MESSAGES: :9:16: note: function declared here as protected
+private:
+ void priv_foo1() override;
+
+public:
+ void prot_foo2() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo2' is changed from private (through private inheritance of class 'A')
to public
+ // CHECK-MESSAGES: :103:10: note: this inheritance would make 'prot_foo2'
private
+ // CHECK-MESSAGES: :10:16: note: function declared here as protected
+ void priv_foo2() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo2' is changed from private in class 'A' to public
+ // CHECK-MESSAGES: :14:16: note: function declared here as private
+
+private:
+ void pub_foo3() override;
+ void prot_foo3() override;
+};
+
+class C: private A {
+};
+
+class D: public C {
+public:
+ void pub_foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo1' is changed from private (through private inheritance of class 'A')
to public
+ // CHECK-MESSAGES: :131:10: note: this inheritance would make 'pub_foo1'
private
+ // CHECK-MESSAGES: :5:16: note: function declared here as public
+};
+
+
+}
+
+namespace test4 {
+
+struct Base1 {
+public:
+ virtual void foo1();
+private:
+ virtual void foo2();
+};
+
+struct Base2 {
+public:
+ virtual void foo2();
+private:
+ virtual void foo1();
+};
+
+struct A : public Base1, public Base2 {
+protected:
+ void foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function 'foo1' is
changed from private in class 'Base2' to protected
+ // CHECK-MESSAGES: :158:16: note: function declared here as private
+ // CHECK-MESSAGES: :[[@LINE-3]]:8: warning: visibility of function 'foo1' is
changed from public in class 'Base1' to protected
+ // CHECK-MESSAGES: :149:16: note: function declared here as public
+private:
+ void foo2() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function 'foo2' is
changed from public in class 'Base2' to private
+ // CHECK-MESSAGES: :156:16: note: function declared here as public
+};
+
+}
+
+namespace test5 {
+
+struct B1: virtual public A {};
+struct B2: virtual private A {};
+struct B: public B1, public B2 {
+public:
+ void pub_foo1() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo1' is changed from private (through private inheritance of class 'A')
to public
+ // CHECK-MESSAGES: :179:12: note: this inheritance would make 'pub_foo1'
private
+ // CHECK-MESSAGES: :5:16: note: function declared here as public
+};
+
+}
+
+namespace test6 {
+class A {
+private:
+ A(int);
+};
+class B: public A {
+public:
+ using A::A;
+};
+}
+
+namespace test7 {
+
+template <typename T>
+class A {
+protected:
+ virtual T foo();
+};
+
+template <typename T>
+class B: public A<T> {
+private:
+ T foo() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: visibility of function 'foo' is
changed from protected in class 'A<int>' to private
+ // CHECK-MESSAGES: :206:13: note: function declared here as protected
+};
+
+template <typename T>
+class C: private A<T> {
+public:
+ T foo() override;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: visibility of function 'foo' is
changed from private (through private inheritance of class 'A<int>') to public
+ // CHECK-MESSAGES: :218:10: note: this inheritance would make 'foo' private
+ // CHECK-MESSAGES: :206:13: note: function declared here as protected
+};
+
+B<int> fB() {
+ return B<int>{};
+}
+
+C<int> fC() {
+ return C<int>{};
+}
+
+}
From f19ab2e4cd0eee1b261767486cf7826f8c7b7ff6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Mon, 19 May 2025 18:11:40 +0200
Subject: [PATCH 2/3] code and doc fixes, added option
---
.../FunctionVisibilityChangeCheck.cpp | 60 +++++++++++---
.../bugprone/FunctionVisibilityChangeCheck.h | 11 ++-
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
.../bugprone/function-visibility-change.rst | 20 ++---
.../docs/clang-tidy/checks/list.rst | 2 +-
.../test-system-header.h | 14 ++++
.../bugprone/function-visibility-change.cpp | 79 ++++++++++++++++---
7 files changed, 153 insertions(+), 35 deletions(-)
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/function-visibility-change/test-system-header.h
diff --git
a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
index 7ea4ed20705ed..f5cfd0c37d4c4 100644
--- a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
@@ -11,12 +11,45 @@
using namespace clang::ast_matchers;
-namespace clang::tidy::bugprone {
+namespace clang::tidy {
+
+template <>
+struct OptionEnumMapping<bugprone::FunctionVisibilityChangeCheck::ChangeKind> {
+ static llvm::ArrayRef<
+ std::pair<bugprone::FunctionVisibilityChangeCheck::ChangeKind,
StringRef>>
+ getEnumMapping() {
+ static constexpr std::pair<
+ bugprone::FunctionVisibilityChangeCheck::ChangeKind, StringRef>
+ Mapping[] = {
+ {bugprone::FunctionVisibilityChangeCheck::ChangeKind::Any, "any"},
+ {bugprone::FunctionVisibilityChangeCheck::ChangeKind::Widening,
+ "widening"},
+ {bugprone::FunctionVisibilityChangeCheck::ChangeKind::Narrowing,
+ "narrowing"},
+ };
+ return {Mapping};
+ }
+};
+
+namespace bugprone {
+
+FunctionVisibilityChangeCheck::FunctionVisibilityChangeCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ DetectVisibilityChange(
+ Options.get("DisallowedVisibilityChange", ChangeKind::Any)) {}
+
+void FunctionVisibilityChangeCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "DisallowedVisibilityChange", DetectVisibilityChange);
+}
void FunctionVisibilityChangeCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
cxxMethodDecl(
- ofClass(cxxRecordDecl().bind("class")),
+ isVirtual(),
+ ofClass(
+
cxxRecordDecl(unless(isExpansionInSystemHeader())).bind("class")),
forEachOverridden(cxxMethodDecl(ofClass(cxxRecordDecl().bind("base")))
.bind("base_func")))
.bind("func"),
@@ -26,14 +59,14 @@ void
FunctionVisibilityChangeCheck::registerMatchers(MatchFinder *Finder) {
void FunctionVisibilityChangeCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *MatchedFunction = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ if (!MatchedFunction->isCanonicalDecl())
+ return;
+
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();
@@ -42,7 +75,7 @@ void FunctionVisibilityChangeCheck::check(
return;
const CXXBaseSpecifier *InheritanceWithStrictVisibility = nullptr;
for (const CXXBasePath &Path : Paths) {
- for (auto Elem : Path) {
+ for (const CXXBasePathElement &Elem : Path) {
if (Elem.Base->getAccessSpecifier() > OverriddenAccess) {
OverriddenAccess = Elem.Base->getAccessSpecifier();
InheritanceWithStrictVisibility = Elem.Base;
@@ -51,6 +84,13 @@ void FunctionVisibilityChangeCheck::check(
}
if (ActualAccess != OverriddenAccess) {
+ if (DetectVisibilityChange == ChangeKind::Widening &&
+ ActualAccess > OverriddenAccess)
+ return;
+ if (DetectVisibilityChange == ChangeKind::Narrowing &&
+ ActualAccess < OverriddenAccess)
+ return;
+
if (InheritanceWithStrictVisibility) {
diag(MatchedFunction->getLocation(),
"visibility of function %0 is changed from %1 (through %1 "
@@ -58,8 +98,8 @@ void FunctionVisibilityChangeCheck::check(
<< MatchedFunction << OverriddenAccess
<< InheritanceWithStrictVisibility->getType() << ActualAccess;
diag(InheritanceWithStrictVisibility->getBeginLoc(),
- "this inheritance would make %0 %1", DiagnosticIDs::Note)
- << MatchedFunction << OverriddenAccess;
+ "%0 is inherited as %1 here", DiagnosticIDs::Note)
+ << InheritanceWithStrictVisibility->getType() << OverriddenAccess;
} else {
diag(MatchedFunction->getLocation(),
"visibility of function %0 is changed from %1 in class %2 to %3")
@@ -71,4 +111,6 @@ void FunctionVisibilityChangeCheck::check(
}
}
-} // namespace clang::tidy::bugprone
+} // namespace bugprone
+
+} // namespace clang::tidy
diff --git
a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
index ec7152fb63acb..9d4b701d51314 100644
--- a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
@@ -13,19 +13,24 @@
namespace clang::tidy::bugprone {
-/// Check function visibility changes in subclasses.
+/// Checks 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) {}
+ enum class ChangeKind { Any, Widening, Narrowing };
+
+ FunctionVisibilityChangeCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
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;
}
+
+private:
+ ChangeKind DetectVisibilityChange;
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 52f61a78fa3b7..6b752359c712d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,7 +127,7 @@ New checks
- New :doc:`bugprone-function-visibility-change
<clang-tidy/checks/bugprone/function-visibility-change>` check.
- Check function visibility changes in subclasses.
+ Checks function visibility changes in subclasses.
- New :doc:`bugprone-unintended-char-ostream-output
<clang-tidy/checks/bugprone/unintended-char-ostream-output>` check.
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
index 3b7940e5e584e..4e0744eede128 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
@@ -3,10 +3,13 @@
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.
+Checks changes in visibility of C++ member functions in subclasses. This
+includes for example if a virtual function declared as `private` is overridden
+and declared as `public` in a subclass. The detected change is the modification
+of visibility resulting from keywords `public`, `protected`, `private` at
+overridden virtual functions. Use of the `using` keyword is not considered by
+this check. The check applies to any normal virtual function and optionally to
+destructors or operators.
.. code-block:: c++
@@ -36,8 +39,7 @@ operators, conversions or other special functions.
// 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.
-
-
+If the visibility is changed in this way, it can indicate bad design or
+programming error. If the change is necessary, it can be achieved by the
+``using`` keyword in a more safe way (this has no effect on the visibility
+in further subclasses).
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 59653a2059e64..c5b8a73f3af65 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -98,7 +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-function-visibility-change
<bugprone/function-visibility-change>`,
: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>`,
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/function-visibility-change/test-system-header.h
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/function-visibility-change/test-system-header.h
new file mode 100644
index 0000000000000..e64e1924a1708
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/function-visibility-change/test-system-header.h
@@ -0,0 +1,14 @@
+#pragma clang system_header
+
+namespace sys {
+
+struct Base {
+ virtual void publicF();
+};
+
+struct Derived: public Base {
+private:
+ void publicF() override;
+};
+
+}
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
index eb4532374267b..b2279d45fe47e 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy %s bugprone-function-visibility-change %t
-
+// RUN: %check_clang_tidy %s bugprone-function-visibility-change %t -- -- -I
%S/Inputs/function-visibility-change
+#include <test-system-header.h>
class A {
public:
virtual void pub_foo1() {}
@@ -104,12 +104,12 @@ class B: private A {
public:
void pub_foo1() override;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo1' is changed from private (through private inheritance of class 'A')
to public
- // CHECK-MESSAGES: :103:10: note: this inheritance would make 'pub_foo1'
private
+ // CHECK-MESSAGES: :103:10: note: 'A' is inherited as private here
// CHECK-MESSAGES: :5:16: note: function declared here as public
protected:
void prot_foo1() override;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from private (through private inheritance of class 'A')
to protected
- // CHECK-MESSAGES: :103:10: note: this inheritance would make 'prot_foo1'
private
+ // CHECK-MESSAGES: :103:10: note: 'A' is inherited as private here
// CHECK-MESSAGES: :9:16: note: function declared here as protected
private:
void priv_foo1() override;
@@ -117,7 +117,7 @@ class B: private A {
public:
void prot_foo2() override;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo2' is changed from private (through private inheritance of class 'A')
to public
- // CHECK-MESSAGES: :103:10: note: this inheritance would make 'prot_foo2'
private
+ // CHECK-MESSAGES: :103:10: note: 'A' is inherited as private here
// CHECK-MESSAGES: :10:16: note: function declared here as protected
void priv_foo2() override;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo2' is changed from private in class 'A' to public
@@ -135,7 +135,7 @@ class D: public C {
public:
void pub_foo1() override;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo1' is changed from private (through private inheritance of class 'A')
to public
- // CHECK-MESSAGES: :131:10: note: this inheritance would make 'pub_foo1'
private
+ // CHECK-MESSAGES: :131:10: note: 'A' is inherited as private here
// CHECK-MESSAGES: :5:16: note: function declared here as public
};
@@ -181,24 +181,30 @@ struct B: public B1, public B2 {
public:
void pub_foo1() override;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo1' is changed from private (through private inheritance of class 'A')
to public
- // CHECK-MESSAGES: :179:12: note: this inheritance would make 'pub_foo1'
private
+ // CHECK-MESSAGES: :179:12: note: 'A' is inherited as private here
// CHECK-MESSAGES: :5:16: note: function declared here as public
};
}
-namespace test6 {
+namespace test_using {
+
class A {
private:
A(int);
+protected:
+ virtual void f();
};
+
class B: public A {
public:
using A::A;
+ using A::f;
};
+
}
-namespace test7 {
+namespace test_template {
template <typename T>
class A {
@@ -211,7 +217,7 @@ class B: public A<T> {
private:
T foo() override;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: visibility of function 'foo' is
changed from protected in class 'A<int>' to private
- // CHECK-MESSAGES: :206:13: note: function declared here as protected
+ // CHECK-MESSAGES: :[[@LINE-8]]:13: note: function declared here as protected
};
template <typename T>
@@ -219,8 +225,8 @@ class C: private A<T> {
public:
T foo() override;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: visibility of function 'foo' is
changed from private (through private inheritance of class 'A<int>') to public
- // CHECK-MESSAGES: :218:10: note: this inheritance would make 'foo' private
- // CHECK-MESSAGES: :206:13: note: function declared here as protected
+ // CHECK-MESSAGES: :[[@LINE-4]]:10: note: 'A<int>' is inherited as private
here
+ // CHECK-MESSAGES: :[[@LINE-17]]:13: note: function declared here as
protected
};
B<int> fB() {
@@ -232,3 +238,52 @@ C<int> fC() {
}
}
+
+namespace test_system_header {
+
+struct SysDerived: public sys::Base {
+private:
+ void publicF();
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function 'publicF'
is changed from public in class 'Base' to private
+};
+
+}
+
+namespace test_destructor {
+
+class A {
+public:
+ virtual ~A();
+};
+
+class B: public A {
+protected:
+ ~B();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: visibility of function '~B'
+ // CHECK-MESSAGES: :[[@LINE-7]]:11: note: function declared here
+};
+
+}
+
+namespace test_operator {
+
+class A {
+ virtual int operator()(int);
+ virtual A& operator++();
+ virtual operator double() const;
+};
+
+class B: public A {
+protected:
+ int operator()(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: visibility of function
'operator()'
+ // CHECK-MESSAGES: :[[@LINE-9]]:15: note: function declared here
+ A& operator++();
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: visibility of function
'operator++'
+ // CHECK-MESSAGES: :[[@LINE-11]]:14: note: function declared here
+ operator double() const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: visibility of function 'operator
double'
+ // CHECK-MESSAGES: :[[@LINE-13]]:11: note: function declared here
+};
+
+}
\ No newline at end of file
From 8f1b20eb1531d21d65e74e42965b37acea1a15c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Wed, 21 May 2025 12:55:47 +0200
Subject: [PATCH 3/3] improvements, added options, check rename
---
.../bugprone/BugproneTidyModule.cpp | 4 +-
.../FunctionVisibilityChangeCheck.cpp | 24 +++++-
.../bugprone/FunctionVisibilityChangeCheck.h | 5 +-
.../bugprone/function-visibility-change.rst | 45 ----------
.../visibility-change-to-virtual-function.rst | 86 +++++++++++++++++++
.../docs/clang-tidy/checks/list.rst | 2 +-
.../test-system-header.h | 0
...lity-change-to-virtual-function-ignore.cpp | 60 +++++++++++++
...ity-change-to-virtual-function-options.cpp | 75 ++++++++++++++++
...visibility-change-to-virtual-function.cpp} | 20 ++---
10 files changed, 260 insertions(+), 61 deletions(-)
delete mode 100644
clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/bugprone/visibility-change-to-virtual-function.rst
rename
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/{function-visibility-change
=> visibility-change-to-virtual-function}/test-system-header.h (100%)
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function-ignore.cpp
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function-options.cpp
rename
clang-tools-extra/test/clang-tidy/checkers/bugprone/{function-visibility-change.cpp
=> visibility-change-to-virtual-function.cpp} (92%)
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 7cf58c5218969..bef6b876f6960 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -144,8 +144,6 @@ 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>(
@@ -280,6 +278,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck<UseAfterMoveCheck>("bugprone-use-after-move");
CheckFactories.registerCheck<VirtualNearMissCheck>(
"bugprone-virtual-near-miss");
+ CheckFactories.registerCheck<FunctionVisibilityChangeCheck>(
+ "bugprone-visibility-change-to-virtual-function");
}
};
diff --git
a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
index f5cfd0c37d4c4..5ba2f51eb3dd9 100644
--- a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "FunctionVisibilityChangeCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
using namespace clang::ast_matchers;
@@ -37,20 +39,31 @@
FunctionVisibilityChangeCheck::FunctionVisibilityChangeCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
DetectVisibilityChange(
- Options.get("DisallowedVisibilityChange", ChangeKind::Any)) {}
+ Options.get("DisallowedVisibilityChange", ChangeKind::Any)),
+ CheckDestructors(Options.get("CheckDestructors", false)),
+ CheckOperators(Options.get("CheckOperators", false)),
+ IgnoredFunctions(utils::options::parseStringList(
+ Options.get("IgnoredFunctions", ""))) {}
void FunctionVisibilityChangeCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "DisallowedVisibilityChange", DetectVisibilityChange);
+ Options.store(Opts, "CheckDestructors", CheckDestructors);
+ Options.store(Opts, "CheckOperators", CheckOperators);
+ Options.store(Opts, "IgnoredFunctions",
+ utils::options::serializeStringList(IgnoredFunctions));
}
void FunctionVisibilityChangeCheck::registerMatchers(MatchFinder *Finder) {
+ auto IgnoredDecl =
+ namedDecl(matchers::matchesAnyListedName(IgnoredFunctions));
Finder->addMatcher(
cxxMethodDecl(
isVirtual(),
ofClass(
cxxRecordDecl(unless(isExpansionInSystemHeader())).bind("class")),
-
forEachOverridden(cxxMethodDecl(ofClass(cxxRecordDecl().bind("base")))
+
forEachOverridden(cxxMethodDecl(ofClass(cxxRecordDecl().bind("base")),
+ unless(IgnoredDecl))
.bind("base_func")))
.bind("func"),
this);
@@ -61,6 +74,13 @@ void FunctionVisibilityChangeCheck::check(
const auto *MatchedFunction = Result.Nodes.getNodeAs<FunctionDecl>("func");
if (!MatchedFunction->isCanonicalDecl())
return;
+ DeclarationName::NameKind NK = MatchedFunction->getDeclName().getNameKind();
+ if (!CheckDestructors && NK == DeclarationName::CXXDestructorName)
+ return;
+ if (!CheckOperators && NK != DeclarationName::Identifier &&
+ NK != DeclarationName::CXXConstructorName &&
+ NK != DeclarationName::CXXDestructorName)
+ return;
const auto *ParentClass = Result.Nodes.getNodeAs<CXXRecordDecl>("class");
const auto *OverriddenFunction =
diff --git
a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
index 9d4b701d51314..2c397217faddd 100644
--- a/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
@@ -16,7 +16,7 @@ namespace clang::tidy::bugprone {
/// Checks function visibility changes in subclasses.
///
/// For the user-facing documentation see:
-///
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/function-visibility-change.html
+///
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/visibility-change-to-virtual-function.html
class FunctionVisibilityChangeCheck : public ClangTidyCheck {
public:
enum class ChangeKind { Any, Widening, Narrowing };
@@ -31,6 +31,9 @@ class FunctionVisibilityChangeCheck : public ClangTidyCheck {
private:
ChangeKind DetectVisibilityChange;
+ bool CheckDestructors;
+ bool CheckOperators;
+ std::vector<llvm::StringRef> IgnoredFunctions;
};
} // namespace clang::tidy::bugprone
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
deleted file mode 100644
index 4e0744eede128..0000000000000
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
+++ /dev/null
@@ -1,45 +0,0 @@
-.. title:: clang-tidy - bugprone-function-visibility-change
-
-bugprone-function-visibility-change
-===================================
-
-Checks changes in visibility of C++ member functions in subclasses. This
-includes for example if a virtual function declared as `private` is overridden
-and declared as `public` in a subclass. The detected change is the modification
-of visibility resulting from keywords `public`, `protected`, `private` at
-overridden virtual functions. Use of the `using` keyword is not considered by
-this check. The check applies to any normal virtual function and optionally to
-destructors or operators.
-
-.. 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
- };
-
-If the visibility is changed in this way, it can indicate bad design or
-programming error. If the change is necessary, it can be achieved by the
-``using`` keyword in a more safe way (this has no effect on the visibility
-in further subclasses).
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/visibility-change-to-virtual-function.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/visibility-change-to-virtual-function.rst
new file mode 100644
index 0000000000000..afd215d199f4a
--- /dev/null
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/visibility-change-to-virtual-function.rst
@@ -0,0 +1,86 @@
+.. title:: clang-tidy - bugprone-visibility-change-to-virtual-function
+
+bugprone-visibility-change-to-virtual-function
+==============================================
+
+Checks changes in visibility of C++ member functions in subclasses. This
+includes for example if a virtual function declared as ``private`` is
overridden
+and declared as ``public`` in a subclass. The detected change is the
+modification of visibility resulting from keywords ``public``, ``protected``,
+``private`` at overridden virtual functions. Use of the ``using`` keyword is
not
+considered by this check. The check applies to any normal virtual function and
+optionally to destructors or operators.
+
+.. 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
+ };
+
+If the visibility is changed in this way, it can indicate bad design or
+programming error.
+
+If a virtual function is private in a subclass but public in the base class, it
+can still be accessed from a pointer to the subclass if the pointer is
converted
+to the base type. Probably private inheritance can be used instead.
+
+A protected virtual function that is made public in a subclass may have valid
+use cases but similar (not exactly same) effect can be achieved with the
+``using`` keyword.
+
+Options
+-------
+
+.. option:: DisallowedVisibilityChange
+
+ Controls what kind of change to the visibility will be detected by the check.
+ Possible values are `any`, `widening`, `narrowing`. For example the
+ `widening` option will produce warning only if the visibility is changed
+ from more restrictive (``private``) to less restrictive (``public``).
+ Default value is `any`.
+
+.. option:: CheckDestructors
+
+ If turned on, the check does apply to destructors too. Otherwise destructors
+ are ignored by the check.
+ Default value is false.
+
+.. option:: CheckOperators
+
+ If turned on, the check does apply to overloaded C++ operators (as virtual
+ member functions) too. This includes other special member functions (like
+ conversions) too. This option is probably useful only in rare cases because
+ operators and conversions are not often virtual functions.
+ Default value is false.
+
+.. option:: IgnoredFunctions
+
+ This option can be used to ignore the check at specific functions.
+ To configure this option, a semicolon-separated list of function names
+ should be provided. The list can contain regular expressions, in this way it
+ is possible to select all functions of a specific class (like `MyClass::.*`)
+ or a specific function of any class (like `my_function` or
+ `::.*::my_function`). The function names are matched at the base class.
+ Default value: empty string.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c5b8a73f3af65..2b39b0b7f5934 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -98,7 +98,6 @@ 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>`,
: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>`,
@@ -168,6 +167,7 @@ Clang-Tidy Checks
:doc:`bugprone-unused-return-value <bugprone/unused-return-value>`,
:doc:`bugprone-use-after-move <bugprone/use-after-move>`,
:doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes"
+ :doc:`bugprone-visibility-change-to-virtual-function
<bugprone/visibility-change-to-virtual-function>`,
:doc:`cert-dcl50-cpp <cert/dcl50-cpp>`,
:doc:`cert-dcl58-cpp <cert/dcl58-cpp>`,
:doc:`cert-env33-c <cert/env33-c>`,
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/function-visibility-change/test-system-header.h
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/visibility-change-to-virtual-function/test-system-header.h
similarity index 100%
rename from
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/function-visibility-change/test-system-header.h
rename to
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/visibility-change-to-virtual-function/test-system-header.h
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function-ignore.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function-ignore.cpp
new file mode 100644
index 0000000000000..324fae5382219
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function-ignore.cpp
@@ -0,0 +1,60 @@
+// RUN: %check_clang_tidy %s bugprone-visibility-change-to-virtual-function %t
-- \
+// RUN: -config="{CheckOptions:
{bugprone-visibility-change-to-virtual-function.IgnoredFunctions:
'IgnoreAlways::.*;::a::IgnoreSelected::.*;IgnoreFunctions::f1;ignored_f'}}"
+
+class IgnoreAlways {
+ virtual void f();
+};
+
+class IgnoreSelected {
+ virtual void f();
+};
+
+namespace a {
+class IgnoreAlways {
+ virtual void f();
+};
+class IgnoreSelected {
+ virtual void f();
+};
+}
+
+namespace ignore_always {
+class Test1: public IgnoreAlways {
+public:
+ void f();
+ void ignored_f(int);
+};
+class Test2: public a::IgnoreAlways {
+public:
+ void f();
+};
+}
+
+namespace ignore_selected {
+class Test1: public IgnoreSelected {
+public:
+ void f();
+ // CHECK-NOTES: :[[@LINE-1]]:8: warning: visibility of function 'f'
+ // CHECK-NOTES: :9:16: note: function declared here
+ void ignored_f(int);
+};
+class Test2: public a::IgnoreSelected {
+public:
+ void f();
+};
+}
+
+class IgnoreFunctions {
+ virtual void f1();
+ virtual void f2();
+ virtual void ignored_f();
+};
+
+class IgnoreFunctionsTest: public IgnoreFunctions {
+public:
+ void f1();
+ void f2();
+ // CHECK-NOTES: :[[@LINE-1]]:8: warning: visibility of function 'f2'
+ // CHECK-NOTES: :[[@LINE-9]]:16: note: function declared here
+ void ignored_f();
+};
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function-options.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function-options.cpp
new file mode 100644
index 0000000000000..a74476849d1fa
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function-options.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy -check-suffixes=DTORS,WIDENING,NARROWING %s
bugprone-visibility-change-to-virtual-function %t -- \
+// RUN: -config="{CheckOptions:
{bugprone-visibility-change-to-virtual-function.CheckDestructors: true}}"
+
+// RUN: %check_clang_tidy -check-suffixes=OPS,WIDENING,NARROWING %s
bugprone-visibility-change-to-virtual-function %t -- \
+// RUN: -config="{CheckOptions:
{bugprone-visibility-change-to-virtual-function.CheckOperators: true}}"
+
+// RUN: %check_clang_tidy -check-suffixes=WIDENING %s
bugprone-visibility-change-to-virtual-function %t -- \
+// RUN: -config="{CheckOptions:
{bugprone-visibility-change-to-virtual-function.DisallowedVisibilityChange:
'widening'}}"
+
+// RUN: %check_clang_tidy -check-suffixes=NARROWING %s
bugprone-visibility-change-to-virtual-function %t -- \
+// RUN: -config="{CheckOptions:
{bugprone-visibility-change-to-virtual-function.DisallowedVisibilityChange:
'narrowing'}}"
+
+namespace test_change {
+
+class A {
+protected:
+ virtual void f1();
+ virtual void f2();
+};
+
+class B: public A {
+public:
+ void f1();
+ // CHECK-NOTES-WIDENING: :[[@LINE-1]]:8: warning: visibility of function 'f1'
+ // CHECK-NOTES-WIDENING: :[[@LINE-8]]:16: note: function declared here
+private:
+ void f2();
+ // CHECK-NOTES-NARROWING: :[[@LINE-1]]:8: warning: visibility of function
'f2'
+ // CHECK-NOTES-NARROWING: :[[@LINE-11]]:16: note: function declared here
+};
+
+}
+
+namespace test_destructor {
+
+class A {
+public:
+ virtual ~A();
+};
+
+class B: public A {
+protected:
+ ~B();
+ // CHECK-NOTES-DTORS: :[[@LINE-1]]:3: warning: visibility of function '~B'
+ // CHECK-NOTES-DTORS: :[[@LINE-7]]:11: note: function declared here
+};
+
+}
+
+namespace test_operator {
+
+class A {
+ virtual A& operator=(const A&);
+ virtual A& operator++();
+ virtual int operator()(int);
+ virtual operator double() const;
+};
+
+class B: public A {
+protected:
+ A& operator=(const A&);
+ // CHECK-NOTES-OPS: :[[@LINE-1]]:6: warning: visibility of function
'operator='
+ // CHECK-NOTES-OPS: :[[@LINE-10]]:14: note: function declared here
+ A& operator++();
+ // CHECK-NOTES-OPS: :[[@LINE-1]]:6: warning: visibility of function
'operator++'
+ // CHECK-NOTES-OPS: :[[@LINE-12]]:14: note: function declared here
+ int operator()(int);
+ // CHECK-NOTES-OPS: :[[@LINE-1]]:7: warning: visibility of function
'operator()'
+ // CHECK-NOTES-OPS: :[[@LINE-14]]:15: note: function declared here
+ operator double() const;
+ // CHECK-NOTES-OPS: :[[@LINE-1]]:3: warning: visibility of function
'operator double'
+ // CHECK-NOTES-OPS: :[[@LINE-16]]:11: note: function declared here
+};
+
+}
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function.cpp
similarity index 92%
rename from
clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
rename to
clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function.cpp
index b2279d45fe47e..ce31ea7c56239 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/visibility-change-to-virtual-function.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-function-visibility-change %t -- -- -I
%S/Inputs/function-visibility-change
+// RUN: %check_clang_tidy %s bugprone-visibility-change-to-virtual-function %t
-- -config="{CheckOptions:
{bugprone-visibility-change-to-virtual-function.CheckDestructors:
true,bugprone-visibility-change-to-virtual-function.CheckOperators: true}}" --
-I %S/Inputs/visibility-change-to-virtual-function
#include <test-system-header.h>
class A {
public:
@@ -25,25 +25,25 @@ class B: public A {
public:
void pub_foo1() override {}
void prot_foo1() override {}
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from protected in class 'A' to public
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from protected in class 'A' to public
[bugprone-visibility-change-to-virtual-function]
// CHECK-MESSAGES: :9:16: note: function declared here as protected
void priv_foo1() override {}
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo1' is changed from private in class 'A' to public
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo1' is changed from private in class 'A' to public
[bugprone-visibility-change-to-virtual-function]
// CHECK-MESSAGES: :13:16: note: function declared here as private
protected:
void pub_foo2() override {}
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo2' is changed from public in class 'A' to protected
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo2' is changed from public in class 'A' to protected
[bugprone-visibility-change-to-virtual-function]
// CHECK-MESSAGES: :6:16: note: function declared here as public
void prot_foo2() override {}
void priv_foo2() override {}
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo2' is changed from private in class 'A' to protected
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo2' is changed from private in class 'A' to protected
[bugprone-visibility-change-to-virtual-function]
// CHECK-MESSAGES: :14:16: note: function declared here as private
private:
void pub_foo3() override {}
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo3' is changed from public in class 'A' to private
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'pub_foo3' is changed from public in class 'A' to private
[bugprone-visibility-change-to-virtual-function]
// CHECK-MESSAGES: :7:16: note: function declared here as public
void prot_foo3() override {}
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo3' is changed from protected in class 'A' to private
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo3' is changed from protected in class 'A' to private
[bugprone-visibility-change-to-virtual-function]
// CHECK-MESSAGES: :11:16: note: function declared here as protected
void priv_foo3() override {}
};
@@ -53,11 +53,11 @@ class C: public B {
void pub_foo1() override;
protected:
void prot_foo1() override;
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from public in class 'B' to protected
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'prot_foo1' is changed from public in class 'B' to protected
[bugprone-visibility-change-to-virtual-function]
// CHECK-MESSAGES: :27:8: note: function declared here as public
private:
void priv_foo1() override;
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo1' is changed from public in class 'B' to private
[bugprone-function-visibility-change]
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: visibility of function
'priv_foo1' is changed from public in class 'B' to private
[bugprone-visibility-change-to-virtual-function]
// CHECK-MESSAGES: :30:8: note: function declared here as public
};
@@ -286,4 +286,4 @@ class B: public A {
// CHECK-MESSAGES: :[[@LINE-13]]:11: note: function declared here
};
-}
\ No newline at end of file
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
