[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
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?=
Date: Thu, 15 May 2025 17:41:16 +0200
Subject: [PATCH 1/9] [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(
"bugprone-forwarding-reference-overload");
+CheckFactories.registerCheck(
+"bugprone-function-visibility-change");
CheckFactories.registerCheck(
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck(
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 0..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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
=?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= Message-ID: In-Reply-To: @@ -1,41 +1,55 @@ -//===--- FunctionVisibilityChangeCheck.cpp - clang-tidy ---===// +//===--- VisibilityChangeToVirtualFunctionCheck.cpp - clang-tidy +//---===// EugeneZelenko wrote: Please merge with previous line. https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
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?=
Date: Thu, 15 May 2025 17:41:16 +0200
Subject: [PATCH 1/8] [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(
"bugprone-forwarding-reference-overload");
+CheckFactories.registerCheck(
+"bugprone-function-visibility-change");
CheckFactories.registerCheck(
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck(
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 0..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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
balazske wrote: > > In my opinion, it should not be bugprone because I don't find any potential > > bug in this code style. Maybe it more like a readability issue? @PiotrZSL > > @EugeneZelenko what do you think? > > Sorry, I don't have strong opinion on this matter. I think the "misc" module would be the best place for this check. https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?=
Message-ID:
In-Reply-To:
@@ -0,0 +1,136 @@
+//===--- 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 "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy {
+
+template <>
+struct OptionEnumMapping {
+ static llvm::ArrayRef<
+ std::pair>
+ 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)),
+ 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")),
+ unless(IgnoredDecl))
+.bind("base_func")))
+ .bind("func"),
+ this);
+}
+
+void FunctionVisibilityChangeCheck::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("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;
HerrCai0907 wrote:
something like `auto M = CheckDestructors ? () : decl();`
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -0,0 +1,136 @@
+//===--- 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 "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy {
+
+template <>
+struct OptionEnumMapping {
+ static llvm::ArrayRef<
+ std::pair>
+ 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)),
+ 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")),
+ unless(IgnoredDecl))
+.bind("base_func")))
+ .bind("func"),
+ this);
+}
+
+void FunctionVisibilityChangeCheck::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("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;
balazske wrote:
I did not find a much better way to check the condition, a conditionally added
matcher can be used but I did not see this pattern in other checker code.
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
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?=
Date: Thu, 15 May 2025 17:41:16 +0200
Subject: [PATCH 1/6] [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(
"bugprone-forwarding-reference-overload");
+CheckFactories.registerCheck(
+"bugprone-function-visibility-change");
CheckFactories.registerCheck(
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck(
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 0..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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
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?=
Date: Thu, 15 May 2025 17:41:16 +0200
Subject: [PATCH 1/5] [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(
"bugprone-forwarding-reference-overload");
+CheckFactories.registerCheck(
+"bugprone-function-visibility-change");
CheckFactories.registerCheck(
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck(
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 0..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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
=?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= Message-ID: In-Reply-To: https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
=?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= Message-ID: In-Reply-To: https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?=
Message-ID:
In-Reply-To:
@@ -0,0 +1,136 @@
+//===--- 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 "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy {
+
+template <>
+struct OptionEnumMapping {
+ static llvm::ArrayRef<
+ std::pair>
+ 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)),
+ 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")),
+ unless(IgnoredDecl))
+.bind("base_func")))
+ .bind("func"),
+ this);
+}
+
+void FunctionVisibilityChangeCheck::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *MatchedFunction = Result.Nodes.getNodeAs("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;
vbvictor wrote:
This can possibly go into the ast-matchers part. You can look for checks that
use options inside mathers with "? unless", "? is" or "? has" strings.
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
=?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= Message-ID: In-Reply-To: @@ -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 + ` check. + + Checks function visibility changes in subclasses. vbvictor wrote: Nit suggestion: make this doc line similar to already existing checks in ReleaseNotes, it should start with "Finds..." Please sync this also with check .rst doc and comment in check header file. https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
balazske wrote: I have renamed the check, if OK the class name will be changed too (in a new commit). https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
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?=
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(
"bugprone-forwarding-reference-overload");
+CheckFactories.registerCheck(
+"bugprone-function-visibility-change");
CheckFactories.registerCheck(
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck(
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 0..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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
balazske wrote: The current code is not finished. I want to check for false positives and probably add options to disable check at destructors and operators. https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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)
balazske wrote:
fixed
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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(
balazske wrote:
fixed (inheritance from system code in non-system code is detected now)
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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) {
balazske wrote:
fixed
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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) {
balazske wrote:
Option is added now to set the type of change.
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
=?utf-8?q?Balázs_Kéri?= Message-ID: In-Reply-To: @@ -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 EugeneZelenko wrote: ```suggestion 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 ``` https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
=?utf-8?q?Balázs_Kéri?=
Message-ID:
In-Reply-To:
@@ -232,3 +238,52 @@ C 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
+};
+
+}
EugeneZelenko wrote:
Please add newline.
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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")
balazske wrote:
Will this not make the message too long? (The message is already located in the
class %4.)
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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 balazske wrote: fixed https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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 + ` check. + + Check function visibility changes in subclasses. balazske wrote: fixed https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
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?=
Date: Thu, 15 May 2025 17:41:16 +0200
Subject: [PATCH 1/2] [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(
"bugprone-forwarding-reference-overload");
+CheckFactories.registerCheck(
+"bugprone-function-visibility-change");
CheckFactories.registerCheck(
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck(
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 0..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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("base");
+
+ if (!MatchedFunction->isCanonicalDecl())
+return;
vbvictor wrote:
nit: Consider moving this lines before `const auto *ParentClass` declaration as
not to declare unused variables
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
https://github.com/vbvictor commented: I think we should make change the name of the check to include "method" or "member-function" words in it. It will make clear that we check for methods visibility and not some visibility attributes on functions. Possible names: bugprone-member-function-visibility-change bugprone-virtual-member-function-visibility-change bugprone-visibility-change-to-virtual-member-function (imho the best) https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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) {
vbvictor wrote:
We should favor explicit type over `auto`
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
https://github.com/PiotrZSL requested changes to this pull request. Idea is ok. Missing some tests (scenarios not covered). Maybe some options to deal with big number of false-positives would be needed. Maybe some option to allow changing visibility to less visible would be needed. https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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
+ };
+
PiotrZSL wrote:
what about when user changes visibility of the function with "using
declaration" ?
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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;
+ }
PiotrZSL wrote:
Consider adding some option to exclude some methods by name, for example if I
would have some generic method like "process_event" that I change visibility on
prupose, then maybe would be good to have option for that, or option to allow
of changing visibility for some base classes.
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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. PiotrZSL wrote: Source code of the check does not verify against operators, and destructors. Probably tests are missing. https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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)
PiotrZSL wrote:
maybe better "%0 is inherited as %1 here".
I do not like that "this" at the begining
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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 PiotrZSL wrote: And avoid "The check", documentation is in scope of check. https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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_f
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -98,6 +98,7 @@ Clang-Tidy Checks :doc:`bugprone-fold-init-type `, :doc:`bugprone-forward-declaration-namespace `, :doc:`bugprone-forwarding-reference-overload `, + :doc:`bugprone-function-visibility-change `, "Yes" PiotrZSL wrote: check does not provide fixes, remove "Yes" https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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")
PiotrZSL wrote:
consider adding "to %3 in class %4"
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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) {
PiotrZSL wrote:
Consider allowing changing access from wider to narrower, maybe some config
option for that.
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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) {
PiotrZSL wrote:
```suggestion
for (const auto& Elem : Elem : Path) {
```
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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.
PiotrZSL wrote:
Proxy functions are out of date, in current days "using declaration" can be
used for that.
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
https://github.com/NagyDonat approved this pull request. LGTM if you implement the changes suggested by EugeneZelenko. https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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.
+
EugeneZelenko wrote:
Excessive trailing newlines.
https://github.com/llvm/llvm-project/pull/140086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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 EugeneZelenko wrote: ```suggestion Checks changes in visibility of C++ member functions in subclasses. The check ``` https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
@@ -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 + ` check. + + Check function visibility changes in subclasses. EugeneZelenko wrote: ```suggestion Checks function visibility changes in subclasses. ``` https://github.com/llvm/llvm-project/pull/140086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-clang-tools-extra
Author: Balázs Kéri (balazske)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/140086.diff
8 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3)
- (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1)
- (added)
clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.cpp (+74)
- (added) clang-tools-extra/clang-tidy/bugprone/FunctionVisibilityChangeCheck.h
(+33)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
- (added)
clang-tools-extra/docs/clang-tidy/checks/bugprone/function-visibility-change.rst
(+43)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
- (added)
clang-tools-extra/test/clang-tidy/checkers/bugprone/function-visibility-change.cpp
(+234)
``diff
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(
"bugprone-forwarding-reference-overload");
+CheckFactories.registerCheck(
+"bugprone-function-visibility-change");
CheckFactories.registerCheck(
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck(
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 0..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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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 "
+
[clang-tools-extra] [clang-tidy] Added check 'bugprone-function-visibility-change' (PR #140086)
https://github.com/balazske created
https://github.com/llvm/llvm-project/pull/140086
None
From 65d44a4eb9621e49a96f1ac43e5a1bbd6691dc13 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Thu, 15 May 2025 17:41:16 +0200
Subject: [PATCH] [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(
"bugprone-forwarding-reference-overload");
+CheckFactories.registerCheck(
+"bugprone-function-visibility-change");
CheckFactories.registerCheck(
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck(
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 0..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("func");
+ const auto *ParentClass = Result.Nodes.getNodeAs("class");
+ const auto *OverriddenFunction =
+ Result.Nodes.getNodeAs("base_func");
+ const auto *BaseClass = Result.Nodes.getNodeAs("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 = El
