Skip to content

Commit a6fc5a1

Browse files
authored
[clang-tidy][NFC] Refactor fuchsia-multiple-inheritance (#171059)
1 parent ce73cbb commit a6fc5a1

File tree

2 files changed

+41
-89
lines changed

2 files changed

+41
-89
lines changed

clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp

Lines changed: 40 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -17,106 +17,61 @@ namespace clang::tidy::fuchsia {
1717

1818
namespace {
1919
AST_MATCHER(CXXRecordDecl, hasBases) {
20-
if (Node.hasDefinition())
21-
return Node.getNumBases() > 0;
22-
return false;
20+
return Node.hasDefinition() && Node.getNumBases() > 0;
2321
}
2422
} // namespace
2523

26-
// Adds a node to the interface map, if it was not present in the map
27-
// previously.
28-
void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
29-
bool IsInterface) {
30-
InterfaceMap.try_emplace(Node, IsInterface);
31-
}
32-
33-
// Returns "true" if the boolean "isInterface" has been set to the
34-
// interface status of the current Node. Return "false" if the
35-
// interface status for the current node is not yet known.
36-
bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
37-
bool &IsInterface) const {
38-
auto Pair = InterfaceMap.find(Node);
39-
if (Pair == InterfaceMap.end())
40-
return false;
41-
IsInterface = Pair->second;
42-
return true;
43-
}
24+
bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
25+
const CXXRecordDecl *const Node = Base.getType()->getAsCXXRecordDecl();
26+
if (!Node)
27+
return true;
4428

45-
bool MultipleInheritanceCheck::isCurrentClassInterface(
46-
const CXXRecordDecl *Node) const {
47-
// Interfaces should have no fields.
48-
if (!Node->field_empty())
49-
return false;
50-
51-
// Interfaces should have exclusively pure methods.
52-
return llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
53-
return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
54-
});
55-
}
29+
assert(Node->isCompleteDefinition());
5630

57-
bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) {
5831
// Short circuit the lookup if we have analyzed this record before.
59-
bool PreviousIsInterfaceResult = false;
60-
if (getInterfaceStatus(Node, PreviousIsInterfaceResult))
61-
return PreviousIsInterfaceResult;
62-
63-
// To be an interface, all base classes must be interfaces as well.
64-
for (const auto &I : Node->bases()) {
65-
if (I.isVirtual())
66-
continue;
67-
const auto *Base = I.getType()->getAsCXXRecordDecl();
68-
if (!Base)
69-
continue;
70-
assert(Base->isCompleteDefinition());
71-
if (!isInterface(Base)) {
72-
addNodeToInterfaceMap(Node, false);
73-
return false;
74-
}
75-
}
76-
77-
const bool CurrentClassIsInterface = isCurrentClassInterface(Node);
78-
addNodeToInterfaceMap(Node, CurrentClassIsInterface);
32+
if (const auto CachedValue = InterfaceMap.find(Node);
33+
CachedValue != InterfaceMap.end())
34+
return CachedValue->second;
35+
36+
// To be an interface, a class must have...
37+
const bool CurrentClassIsInterface =
38+
// ...no bases that aren't interfaces...
39+
llvm::none_of(Node->bases(),
40+
[&](const CXXBaseSpecifier &I) {
41+
return !I.isVirtual() && !isInterface(I);
42+
}) &&
43+
// ...no fields, and...
44+
Node->field_empty() &&
45+
// ...no methods that aren't pure virtual.
46+
llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) {
47+
return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
48+
});
49+
50+
InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
7951
return CurrentClassIsInterface;
8052
}
8153

8254
void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) {
83-
// Match declarations which have bases.
8455
Finder->addMatcher(cxxRecordDecl(hasBases(), isDefinition()).bind("decl"),
8556
this);
8657
}
8758

8859
void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
89-
if (const auto *D = Result.Nodes.getNodeAs<CXXRecordDecl>("decl")) {
90-
// Check against map to see if the class inherits from multiple
91-
// concrete classes
92-
unsigned NumConcrete = 0;
93-
for (const auto &I : D->bases()) {
94-
if (I.isVirtual())
95-
continue;
96-
const auto *Base = I.getType()->getAsCXXRecordDecl();
97-
if (!Base)
98-
continue;
99-
assert(Base->isCompleteDefinition());
100-
if (!isInterface(Base))
101-
NumConcrete++;
102-
}
103-
104-
// Check virtual bases to see if there is more than one concrete
105-
// non-virtual base.
106-
for (const auto &V : D->vbases()) {
107-
const auto *Base = V.getType()->getAsCXXRecordDecl();
108-
if (!Base)
109-
continue;
110-
assert(Base->isCompleteDefinition());
111-
if (!isInterface(Base))
112-
NumConcrete++;
113-
}
114-
115-
if (NumConcrete > 1) {
116-
diag(D->getBeginLoc(), "inheriting multiple classes that aren't "
117-
"pure virtual is discouraged");
118-
}
119-
}
60+
const auto &D = *Result.Nodes.getNodeAs<CXXRecordDecl>("decl");
61+
// Check to see if the class inherits from multiple concrete classes.
62+
unsigned NumConcrete =
63+
llvm::count_if(D.bases(), [&](const CXXBaseSpecifier &I) {
64+
return !I.isVirtual() && !isInterface(I);
65+
});
66+
67+
// Check virtual bases to see if there is more than one concrete
68+
// non-virtual base.
69+
NumConcrete += llvm::count_if(
70+
D.vbases(), [&](const CXXBaseSpecifier &V) { return !isInterface(V); });
71+
72+
if (NumConcrete > 1)
73+
diag(D.getBeginLoc(), "inheriting multiple classes that aren't "
74+
"pure virtual is discouraged");
12075
}
12176

12277
} // namespace clang::tidy::fuchsia

clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@ class MultipleInheritanceCheck : public ClangTidyCheck {
3030
void onEndOfTranslationUnit() override { InterfaceMap.clear(); }
3131

3232
private:
33-
void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool IsInterface);
34-
bool getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const;
35-
bool isCurrentClassInterface(const CXXRecordDecl *Node) const;
36-
bool isInterface(const CXXRecordDecl *Node);
33+
bool isInterface(const CXXBaseSpecifier &Base);
3734

3835
// Contains the identity of each named CXXRecord as an interface. This is
3936
// used to memoize lookup speeds and improve performance from O(N^2) to O(N),

0 commit comments

Comments
 (0)