[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/localspook closed https://github.com/llvm/llvm-project/pull/171059 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
zeyi2 wrote: > So this check now only located in "fuchsia" module. I think we could have a > regular `misc-` check for it instead, so more users can discover and use it. Maybe open another "rename" issue for this check? https://github.com/llvm/llvm-project/pull/171059 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/vbvictor approved this pull request. So this check now only located in "fuchsia" module. I think we could have a regular `misc-` check for it instead, so more users can discover and use it. https://github.com/llvm/llvm-project/pull/171059 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/zeyi2 approved this pull request. https://github.com/llvm/llvm-project/pull/171059 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/localspook updated
https://github.com/llvm/llvm-project/pull/171059
>From c53880940787c9c2f81b0466f82e41831b87b407 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:38:48 -0800
Subject: [PATCH 01/21] Inline `addNodeToInterfaceMap`
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 11 ++-
.../clang-tidy/fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index df40d166bd404..45fea99ece90b 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,13 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Adds a node to the interface map, if it was not present in the map
-// previously.
-void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
- bool IsInterface) {
- InterfaceMap.try_emplace(Node, IsInterface);
-}
-
// Returns "true" if the boolean "isInterface" has been set to the
// interface status of the current Node. Return "false" if the
// interface status for the current node is not yet known.
@@ -69,13 +62,13 @@ bool MultipleInheritanceCheck::isInterface(const
CXXRecordDecl *Node) {
continue;
assert(Base->isCompleteDefinition());
if (!isInterface(Base)) {
- addNodeToInterfaceMap(Node, false);
+ InterfaceMap.try_emplace(Node, false);
return false;
}
}
const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
- addNodeToInterfaceMap(Node, CurrentClassIsInterface);
+ InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index b9055eb58a300..a333b3c67882e 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,6 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool IsInterface);
bool getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const;
bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
bool isInterface(const CXXRecordDecl *Node);
>From 05b7a81142a698e1921a639df352217a363aad06 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:41:21 -0800
Subject: [PATCH 02/21] Inline `getInterfaceStatus`
At the call site, it was just as much code to call
this function as it is to inline it. This also
avoids an out parameter.
---
.../fuchsia/MultipleInheritanceCheck.cpp | 18 +++---
.../fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 45fea99ece90b..71dbf6acf906f 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,18 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Returns "true" if the boolean "isInterface" has been set to the
-// interface status of the current Node. Return "false" if the
-// interface status for the current node is not yet known.
-bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
- bool &IsInterface) const {
- auto Pair = InterfaceMap.find(Node);
- if (Pair == InterfaceMap.end())
-return false;
- IsInterface = Pair->second;
- return true;
-}
-
bool MultipleInheritanceCheck::isCurrentClassInterface(
const CXXRecordDecl *Node) const {
// Interfaces should have no fields.
@@ -49,9 +37,9 @@ bool MultipleInheritanceCheck::isCurrentClassInterface(
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
- bool PreviousIsInterfaceResult = false;
- if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
-return PreviousIsInterfaceResult;
+ const auto CachedValue = InterfaceMap.find(Node);
+ if (CachedValue != InterfaceMap.end())
+return CachedValue->second;
// To be an interface, all base classes must be interfaces as well.
for (const auto &I : Node->bases()) {
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index a333b3c67882e..feb86da4309fb 100644
--- a/clang-tool
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
@@ -17,105 +17,61 @@ namespace clang::tidy::fuchsia {
namespace {
AST_MATCHER(CXXRecordDecl, hasBases) {
- if (Node.hasDefinition())
-return Node.getNumBases() > 0;
- return false;
+ return Node.hasDefinition() && Node.getNumBases() > 0;
}
} // namespace
-// Adds a node to the interface map, if it was not present in the map
-// previously.
-void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
- bool IsInterface) {
- InterfaceMap.try_emplace(Node, IsInterface);
-}
-
-// Returns "true" if the boolean "isInterface" has been set to the
-// interface status of the current Node. Return "false" if the
-// interface status for the current node is not yet known.
-bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
- bool &IsInterface) const {
- auto Pair = InterfaceMap.find(Node);
- if (Pair == InterfaceMap.end())
-return false;
- IsInterface = Pair->second;
- return true;
-}
+bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
+ const CXXRecordDecl *const Node = Base.getType()->getAsCXXRecordDecl();
+ if (!Node)
+return true;
-bool MultipleInheritanceCheck::isCurrentClassInterface(
-const CXXRecordDecl *Node) const {
- // Interfaces should have no fields.
- if (!Node->field_empty())
-return false;
-
- // Interfaces should have exclusively pure methods.
- return llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
-return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
- });
-}
+ assert(Node->isCompleteDefinition());
-bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
- bool PreviousIsInterfaceResult = false;
- if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
-return PreviousIsInterfaceResult;
-
- // To be an interface, all base classes must be interfaces as well.
- for (const auto &I : Node->bases()) {
-if (I.isVirtual())
- continue;
-const auto *Base = I.getType()->getAsCXXRecordDecl();
-if (!Base)
- continue;
-assert(Base->isCompleteDefinition());
-if (!isInterface(Base)) {
- addNodeToInterfaceMap(Node, false);
- return false;
-}
- }
-
- const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
- addNodeToInterfaceMap(Node, CurrentClassIsInterface);
+ const auto CachedValue = InterfaceMap.find(Node);
+ if (CachedValue != InterfaceMap.end())
+return CachedValue->second;
zwuis wrote:
```cpp
if (const auto CachedValue = ...;
CachedValue != ...)
return ...;
```
https://github.com/llvm/llvm-project/pull/171059
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/zwuis approved this pull request. https://github.com/llvm/llvm-project/pull/171059 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
@@ -17,105 +17,61 @@ namespace clang::tidy::fuchsia {
namespace {
AST_MATCHER(CXXRecordDecl, hasBases) {
- if (Node.hasDefinition())
-return Node.getNumBases() > 0;
- return false;
+ return Node.hasDefinition() && Node.getNumBases() > 0;
}
} // namespace
-// Adds a node to the interface map, if it was not present in the map
-// previously.
-void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
- bool IsInterface) {
- InterfaceMap.try_emplace(Node, IsInterface);
-}
-
-// Returns "true" if the boolean "isInterface" has been set to the
-// interface status of the current Node. Return "false" if the
-// interface status for the current node is not yet known.
-bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
- bool &IsInterface) const {
- auto Pair = InterfaceMap.find(Node);
- if (Pair == InterfaceMap.end())
-return false;
- IsInterface = Pair->second;
- return true;
-}
+bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
+ const CXXRecordDecl *const Node = Base.getType()->getAsCXXRecordDecl();
+ if (!Node)
+return true;
-bool MultipleInheritanceCheck::isCurrentClassInterface(
-const CXXRecordDecl *Node) const {
- // Interfaces should have no fields.
- if (!Node->field_empty())
-return false;
-
- // Interfaces should have exclusively pure methods.
- return llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
-return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
- });
-}
+ assert(Node->isCompleteDefinition());
-bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
- bool PreviousIsInterfaceResult = false;
- if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
-return PreviousIsInterfaceResult;
-
- // To be an interface, all base classes must be interfaces as well.
- for (const auto &I : Node->bases()) {
-if (I.isVirtual())
- continue;
-const auto *Base = I.getType()->getAsCXXRecordDecl();
-if (!Base)
- continue;
-assert(Base->isCompleteDefinition());
-if (!isInterface(Base)) {
- addNodeToInterfaceMap(Node, false);
- return false;
-}
- }
-
- const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
- addNodeToInterfaceMap(Node, CurrentClassIsInterface);
+ const auto CachedValue = InterfaceMap.find(Node);
+ if (CachedValue != InterfaceMap.end())
+return CachedValue->second;
+
+ // To be an interface, a class must have...
+ const bool CurrentClassIsInterface =
+ // ...no bases that aren't interfaces...
+ llvm::none_of(Node->bases(),
+[&](const CXXBaseSpecifier &I) {
+ return !I.isVirtual() && !isInterface(I);
+}) &&
+ // ...no fields, and...
+ Node->field_empty() &&
+ // ...no methods that aren't pure virtual.
+ llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
+return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
+ });
+
+ InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) {
- // Match declarations which have bases.
Finder->addMatcher(cxxRecordDecl(hasBases(), isDefinition()).bind("decl"),
this);
}
void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
- if (const auto *D = Result.Nodes.getNodeAs("decl")) {
-// Check against map to see if the class inherits from multiple
-// concrete classes
-unsigned NumConcrete = 0;
-for (const auto &I : D->bases()) {
- if (I.isVirtual())
-continue;
- const auto *Base = I.getType()->getAsCXXRecordDecl();
- if (!Base)
-continue;
- assert(Base->isCompleteDefinition());
- if (!isInterface(Base))
-NumConcrete++;
-}
-
-// Check virtual bases to see if there is more than one concrete
-// non-virtual base.
-for (const auto &V : D->vbases()) {
- const auto *Base = V.getType()->getAsCXXRecordDecl();
- if (!Base)
-continue;
- assert(Base->isCompleteDefinition());
- if (!isInterface(Base))
-NumConcrete++;
-}
-
-if (NumConcrete > 1) {
- diag(D->getBeginLoc(), "inheriting multiple classes that aren't "
- "pure virtual is discouraged");
-}
+ const auto &D = *Result.Nodes.getNodeAs("decl");
+ // Check to see if the class inherits from multiple concrete classes.
+ unsigned NumConcrete =
+ llvm::count_if(D.bases(), [&](const CXXBaseSpecifier &I) {
+return !I.isVirtual() && !isInterface(I);
+ });
+
+ // Check virtual bases to see if there is more than one concrete
+ // non-virtual base.
+
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/zwuis edited https://github.com/llvm/llvm-project/pull/171059 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/localspook updated
https://github.com/llvm/llvm-project/pull/171059
>From c53880940787c9c2f81b0466f82e41831b87b407 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:38:48 -0800
Subject: [PATCH 01/20] Inline `addNodeToInterfaceMap`
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 11 ++-
.../clang-tidy/fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index df40d166bd404..45fea99ece90b 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,13 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Adds a node to the interface map, if it was not present in the map
-// previously.
-void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
- bool IsInterface) {
- InterfaceMap.try_emplace(Node, IsInterface);
-}
-
// Returns "true" if the boolean "isInterface" has been set to the
// interface status of the current Node. Return "false" if the
// interface status for the current node is not yet known.
@@ -69,13 +62,13 @@ bool MultipleInheritanceCheck::isInterface(const
CXXRecordDecl *Node) {
continue;
assert(Base->isCompleteDefinition());
if (!isInterface(Base)) {
- addNodeToInterfaceMap(Node, false);
+ InterfaceMap.try_emplace(Node, false);
return false;
}
}
const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
- addNodeToInterfaceMap(Node, CurrentClassIsInterface);
+ InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index b9055eb58a300..a333b3c67882e 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,6 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool IsInterface);
bool getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const;
bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
bool isInterface(const CXXRecordDecl *Node);
>From 05b7a81142a698e1921a639df352217a363aad06 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:41:21 -0800
Subject: [PATCH 02/20] Inline `getInterfaceStatus`
At the call site, it was just as much code to call
this function as it is to inline it. This also
avoids an out parameter.
---
.../fuchsia/MultipleInheritanceCheck.cpp | 18 +++---
.../fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 45fea99ece90b..71dbf6acf906f 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,18 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Returns "true" if the boolean "isInterface" has been set to the
-// interface status of the current Node. Return "false" if the
-// interface status for the current node is not yet known.
-bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
- bool &IsInterface) const {
- auto Pair = InterfaceMap.find(Node);
- if (Pair == InterfaceMap.end())
-return false;
- IsInterface = Pair->second;
- return true;
-}
-
bool MultipleInheritanceCheck::isCurrentClassInterface(
const CXXRecordDecl *Node) const {
// Interfaces should have no fields.
@@ -49,9 +37,9 @@ bool MultipleInheritanceCheck::isCurrentClassInterface(
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
- bool PreviousIsInterfaceResult = false;
- if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
-return PreviousIsInterfaceResult;
+ const auto CachedValue = InterfaceMap.find(Node);
+ if (CachedValue != InterfaceMap.end())
+return CachedValue->second;
// To be an interface, all base classes must be interfaces as well.
for (const auto &I : Node->bases()) {
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index a333b3c67882e..feb86da4309fb 100644
--- a/clang-tool
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/localspook updated
https://github.com/llvm/llvm-project/pull/171059
>From c53880940787c9c2f81b0466f82e41831b87b407 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:38:48 -0800
Subject: [PATCH 01/19] Inline `addNodeToInterfaceMap`
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 11 ++-
.../clang-tidy/fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index df40d166bd404..45fea99ece90b 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,13 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Adds a node to the interface map, if it was not present in the map
-// previously.
-void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
- bool IsInterface) {
- InterfaceMap.try_emplace(Node, IsInterface);
-}
-
// Returns "true" if the boolean "isInterface" has been set to the
// interface status of the current Node. Return "false" if the
// interface status for the current node is not yet known.
@@ -69,13 +62,13 @@ bool MultipleInheritanceCheck::isInterface(const
CXXRecordDecl *Node) {
continue;
assert(Base->isCompleteDefinition());
if (!isInterface(Base)) {
- addNodeToInterfaceMap(Node, false);
+ InterfaceMap.try_emplace(Node, false);
return false;
}
}
const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
- addNodeToInterfaceMap(Node, CurrentClassIsInterface);
+ InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index b9055eb58a300..a333b3c67882e 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,6 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool IsInterface);
bool getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const;
bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
bool isInterface(const CXXRecordDecl *Node);
>From 05b7a81142a698e1921a639df352217a363aad06 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:41:21 -0800
Subject: [PATCH 02/19] Inline `getInterfaceStatus`
At the call site, it was just as much code to call
this function as it is to inline it. This also
avoids an out parameter.
---
.../fuchsia/MultipleInheritanceCheck.cpp | 18 +++---
.../fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 45fea99ece90b..71dbf6acf906f 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,18 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Returns "true" if the boolean "isInterface" has been set to the
-// interface status of the current Node. Return "false" if the
-// interface status for the current node is not yet known.
-bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
- bool &IsInterface) const {
- auto Pair = InterfaceMap.find(Node);
- if (Pair == InterfaceMap.end())
-return false;
- IsInterface = Pair->second;
- return true;
-}
-
bool MultipleInheritanceCheck::isCurrentClassInterface(
const CXXRecordDecl *Node) const {
// Interfaces should have no fields.
@@ -49,9 +37,9 @@ bool MultipleInheritanceCheck::isCurrentClassInterface(
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
- bool PreviousIsInterfaceResult = false;
- if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
-return PreviousIsInterfaceResult;
+ const auto CachedValue = InterfaceMap.find(Node);
+ if (CachedValue != InterfaceMap.end())
+return CachedValue->second;
// To be an interface, all base classes must be interfaces as well.
for (const auto &I : Node->bases()) {
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index a333b3c67882e..feb86da4309fb 100644
--- a/clang-tool
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/localspook updated
https://github.com/llvm/llvm-project/pull/171059
>From c53880940787c9c2f81b0466f82e41831b87b407 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:38:48 -0800
Subject: [PATCH 01/18] Inline `addNodeToInterfaceMap`
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 11 ++-
.../clang-tidy/fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index df40d166bd404..45fea99ece90b 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,13 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Adds a node to the interface map, if it was not present in the map
-// previously.
-void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
- bool IsInterface) {
- InterfaceMap.try_emplace(Node, IsInterface);
-}
-
// Returns "true" if the boolean "isInterface" has been set to the
// interface status of the current Node. Return "false" if the
// interface status for the current node is not yet known.
@@ -69,13 +62,13 @@ bool MultipleInheritanceCheck::isInterface(const
CXXRecordDecl *Node) {
continue;
assert(Base->isCompleteDefinition());
if (!isInterface(Base)) {
- addNodeToInterfaceMap(Node, false);
+ InterfaceMap.try_emplace(Node, false);
return false;
}
}
const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
- addNodeToInterfaceMap(Node, CurrentClassIsInterface);
+ InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index b9055eb58a300..a333b3c67882e 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,6 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool IsInterface);
bool getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const;
bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
bool isInterface(const CXXRecordDecl *Node);
>From 05b7a81142a698e1921a639df352217a363aad06 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:41:21 -0800
Subject: [PATCH 02/18] Inline `getInterfaceStatus`
At the call site, it was just as much code to call
this function as it is to inline it. This also
avoids an out parameter.
---
.../fuchsia/MultipleInheritanceCheck.cpp | 18 +++---
.../fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 45fea99ece90b..71dbf6acf906f 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,18 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Returns "true" if the boolean "isInterface" has been set to the
-// interface status of the current Node. Return "false" if the
-// interface status for the current node is not yet known.
-bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
- bool &IsInterface) const {
- auto Pair = InterfaceMap.find(Node);
- if (Pair == InterfaceMap.end())
-return false;
- IsInterface = Pair->second;
- return true;
-}
-
bool MultipleInheritanceCheck::isCurrentClassInterface(
const CXXRecordDecl *Node) const {
// Interfaces should have no fields.
@@ -49,9 +37,9 @@ bool MultipleInheritanceCheck::isCurrentClassInterface(
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
- bool PreviousIsInterfaceResult = false;
- if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
-return PreviousIsInterfaceResult;
+ const auto CachedValue = InterfaceMap.find(Node);
+ if (CachedValue != InterfaceMap.end())
+return CachedValue->second;
// To be an interface, all base classes must be interfaces as well.
for (const auto &I : Node->bases()) {
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index a333b3c67882e..feb86da4309fb 100644
--- a/clang-tool
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff origin/main HEAD --extensions cpp,h --
clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
--diff_from_common_commit
``
:warning:
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing `origin/main` to the base branch/commit you want to compare against.
:warning:
View the diff from clang-format here.
``diff
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index eb5c94241..4dcbd0c78 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,7 @@ public:
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- bool isInterface(const CXXBaseSpecifier& Base);
+ bool isInterface(const CXXBaseSpecifier &Base);
// Contains the identity of each named CXXRecord as an interface. This is
// used to memoize lookup speeds and improve performance from O(N^2) to O(N),
``
https://github.com/llvm/llvm-project/pull/171059
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][NFC] Refactor `fuchsia-multiple-inheritance` (PR #171059)
https://github.com/localspook created
https://github.com/llvm/llvm-project/pull/171059
I've structured this PR as a series of independent commits, so it's probably
easiest to review commit-by-commit. There's some subjectivity involved with
refactoring, so if you think any of the changes is a downgrade, I'm happy to
discuss it.
>From c53880940787c9c2f81b0466f82e41831b87b407 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:38:48 -0800
Subject: [PATCH 01/18] Inline `addNodeToInterfaceMap`
---
.../clang-tidy/fuchsia/MultipleInheritanceCheck.cpp | 11 ++-
.../clang-tidy/fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index df40d166bd404..45fea99ece90b 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,13 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Adds a node to the interface map, if it was not present in the map
-// previously.
-void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
- bool IsInterface) {
- InterfaceMap.try_emplace(Node, IsInterface);
-}
-
// Returns "true" if the boolean "isInterface" has been set to the
// interface status of the current Node. Return "false" if the
// interface status for the current node is not yet known.
@@ -69,13 +62,13 @@ bool MultipleInheritanceCheck::isInterface(const
CXXRecordDecl *Node) {
continue;
assert(Base->isCompleteDefinition());
if (!isInterface(Base)) {
- addNodeToInterfaceMap(Node, false);
+ InterfaceMap.try_emplace(Node, false);
return false;
}
}
const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
- addNodeToInterfaceMap(Node, CurrentClassIsInterface);
+ InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
return CurrentClassIsInterface;
}
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
index b9055eb58a300..a333b3c67882e 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h
@@ -30,7 +30,6 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
private:
- void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool IsInterface);
bool getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const;
bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
bool isInterface(const CXXRecordDecl *Node);
>From 32cbb6efa3331d055faffefc354c9d81fc2d967f Mon Sep 17 00:00:00 2001
From: Victor Chernyakin
Date: Sun, 7 Dec 2025 01:41:21 -0800
Subject: [PATCH 02/18] Inline `getInterfaceStatus`
At the call site, it was just as much code to call
this function as it is to inline it. This also
avoids an uninitialized variable.
---
.../fuchsia/MultipleInheritanceCheck.cpp | 18 +++---
.../fuchsia/MultipleInheritanceCheck.h | 1 -
2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
index 45fea99ece90b..71dbf6acf906f 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -23,18 +23,6 @@ AST_MATCHER(CXXRecordDecl, hasBases) {
}
} // namespace
-// Returns "true" if the boolean "isInterface" has been set to the
-// interface status of the current Node. Return "false" if the
-// interface status for the current node is not yet known.
-bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
- bool &IsInterface) const {
- auto Pair = InterfaceMap.find(Node);
- if (Pair == InterfaceMap.end())
-return false;
- IsInterface = Pair->second;
- return true;
-}
-
bool MultipleInheritanceCheck::isCurrentClassInterface(
const CXXRecordDecl *Node) const {
// Interfaces should have no fields.
@@ -49,9 +37,9 @@ bool MultipleInheritanceCheck::isCurrentClassInterface(
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
// Short circuit the lookup if we have analyzed this record before.
- bool PreviousIsInterfaceResult = false;
- if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
-return PreviousIsInterfaceResult;
+ const auto CachedValue = InterfaceMap.find(Node);
+ if (CachedValue != InterfaceMap.end())
+return CachedValue->second;
// To be an interface, all base classes must be interfac
