Index: clang-tidy/fuchsia/CMakeLists.txt =================================================================== --- clang-tidy/fuchsia/CMakeLists.txt +++ clang-tidy/fuchsia/CMakeLists.txt @@ -3,6 +3,7 @@ add_clang_library(clangTidyFuchsiaModule DefaultArgumentsCheck.cpp FuchsiaTidyModule.cpp + MultipleInheritanceCheck.cpp LINK_LIBS clangAST Index: clang-tidy/fuchsia/FuchsiaTidyModule.cpp =================================================================== --- clang-tidy/fuchsia/FuchsiaTidyModule.cpp +++ clang-tidy/fuchsia/FuchsiaTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "DefaultArgumentsCheck.h" +#include "MultipleInheritanceCheck.h" using namespace clang::ast_matchers; @@ -24,6 +25,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "fuchsia-default-arguments"); + CheckFactories.registerCheck( + "fuchsia-multiple-inheritance"); } }; // Register the FuchsiaTidyModule using this statically initialized variable. Index: clang-tidy/fuchsia/MultipleInheritanceCheck.h =================================================================== --- /dev/null +++ clang-tidy/fuchsia/MultipleInheritanceCheck.h @@ -0,0 +1,51 @@ +//===--- MultipleInheritanceCheck.h - clang-tidy-----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_MULTIPLE_INHERITANCE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_MULTIPLE_INHERITANCE_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace fuchsia { + +/// Mulitple inheritance is disallowed. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-multiple-inheritance.html +class MultipleInheritanceCheck : public ClangTidyCheck { +public: + MultipleInheritanceCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + + void onStartOfTranslationUnit() override { + InterfaceMap = new llvm::StringMap; + } + void onEndOfTranslationUnit() override { delete InterfaceMap; } + + void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool isInterface); + bool getInterfaceStatus(const CXXRecordDecl *Node, bool *isInterface); + bool isCurrentClassInterface(const CXXRecordDecl *Node); + bool isInterface(const CXXRecordDecl *Node); + +private: + // 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), + // where N is the number of classes. + llvm::StringMap *InterfaceMap; +}; + +} // namespace fuchsia +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_MULTIPLE_INHERITANCE_H Index: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/fuchsia/MultipleInheritanceCheck.cpp @@ -0,0 +1,120 @@ +//===--- MultipleInheritanceCheck.cpp - clang-tidy-------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "MultipleInheritanceCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang; +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace fuchsia { + +AST_MATCHER(CXXRecordDecl, hasDefinition) { + if (!Node.hasDefinition()) + return false; + return true; +} + +// Adds a node (by name) to the interface map, if it was not present in the map +// previously. +void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node, + bool isInterface) { + StringRef Name = Node->getIdentifier()->getName(); + MultipleInheritanceCheck::InterfaceMap->insert( + std::make_pair(Name, 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) { + StringRef Name = Node->getIdentifier()->getName(); + if (MultipleInheritanceCheck::InterfaceMap->count(Name)) { + *isInterface = MultipleInheritanceCheck::InterfaceMap->lookup(Name); + return true; + } + return false; +} + +// TODO(smklein): Make an exception for 'Dispatcher' -- consider it an +// interface, even though it isn't. +bool MultipleInheritanceCheck::isCurrentClassInterface( + const CXXRecordDecl *Node) { + // Interfaces should have no fields. + if (!Node->field_empty()) { + return false; + } + // Interfaces should have exclusively pure methods. + for (const auto method : Node->methods()) { + if (method->isUserProvided() && !method->isPure()) { + return false; + } + } + return true; +} + +bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) { + // Short circuit the lookup if we have analyzed this record before. + bool PreviousIsInterfaceResult; + if (MultipleInheritanceCheck::getInterfaceStatus(Node, + &PreviousIsInterfaceResult)) { + return PreviousIsInterfaceResult; + } + + // To be an interface, all base classes must be interfaces as well. + for (const auto &I : Node->bases()) { + const RecordType *Ty = I.getType()->getAs(); + assert(Ty && "RecordType of base class is unknown"); + const auto *Base = cast(Ty->getDecl()->getDefinition()); + if (!MultipleInheritanceCheck::isInterface(Base)) { + MultipleInheritanceCheck::addNodeToInterfaceMap(Node, false); + return false; + } + } + + bool CurrentClassIsInterface = + MultipleInheritanceCheck::isCurrentClassInterface(Node); + MultipleInheritanceCheck::addNodeToInterfaceMap(Node, + CurrentClassIsInterface); + return CurrentClassIsInterface; +} + +void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) { + // Match declarations which have definitions. + Finder->addMatcher(cxxRecordDecl(hasDefinition()).bind("decl"), this); +} + +void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) { + if (const CXXRecordDecl *D = + Result.Nodes.getNodeAs("decl")) { + // Check against map to see if if the class inherits from multiple + // concrete classes + int NumConcrete = 0; + for (const auto &I : D->bases()) { + const RecordType *Ty = I.getType()->getAs(); + assert(Ty && "RecordType of base class is unknown"); + CXXRecordDecl *Base = cast(Ty->getDecl()->getDefinition()); + if (!MultipleInheritanceCheck::isInterface(Base)) + NumConcrete++; + } + + if (NumConcrete > 1) { + diag(D->getLocStart(), "inheriting mulitple classes which aren't " + "pure virtual is disallowed"); + } + } +} + +} // namespace fuchsia +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -114,6 +114,11 @@ `_ check Warns if a function or method is declared or called with default arguments. + +- New `fuchsia-multiple-inheritance + `_ check + + Warns if a class inherits from multiple classes that are not pure virtual. - New `google-avoid-throwing-objc-exception `_ check Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst @@ -0,0 +1,46 @@ +.. title:: clang-tidy - fuchsia-multiple-inheritance + +fuchsia-multiple-inheritance +============================ + +Warns if a class inherits from multiple classes that are not pure virtual. + +For example, declaring a class that inherits from multiple concrete classes is +disallowed: + +.. code-block:: c++ + + class Base_A { + public: + virtual int foo() { return 0; } + }; + + class Base_B { + public: + virtual int bar() { return 0; } + }; + + // Warning + class Bad_Child1 : public Base_A, Base_B {}; + +A class that inherits from a pure virtual is allowed: + +.. code-block:: c++ + + class Interface_A { + public: + virtual int foo() = 0; + }; + + class Interface_B { + public: + virtual int bar() = 0; + }; + + // No warning + class Good_Child1 : public Interface_A, Interface_B { + virtual int foo() override { return 0; } + virtual int bar() override { return 0; } + }; + +See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -69,6 +69,7 @@ cppcoreguidelines-slicing cppcoreguidelines-special-member-functions fuchsia-default-arguments + fuchsia-multiple-inheritance google-build-explicit-make-pair google-build-namespaces google-build-using-namespace Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp =================================================================== --- /dev/null +++ test/clang-tidy/fuchsia-multiple-inheritance.cpp @@ -0,0 +1,91 @@ +// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t + +class Base_A { +public: + virtual int foo() { return 0; } +}; + +class Base_B { +public: + virtual int bar() { return 0; } +}; + +class Base_A_child : public Base_A { +public: + virtual int baz() { return 0; } +}; + +class Interface_A { +public: + virtual int foo() = 0; +}; + +class Interface_B { +public: + virtual int bar() = 0; +}; + +class Interface_C { +public: + virtual int blat() = 0; +}; + +class Interface_A_with_member { +public: + virtual int foo() = 0; + int val = 0; +}; + +class Interface_with_A_Parent : public Base_A { +public: + virtual int baz() = 0; +}; + +// Inherits from multiple concrete classes. +// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance] +// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {}; +class Bad_Child1 : public Base_A, Base_B {}; + +// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance] +class Bad_Child2 : public Base_A, Interface_A_with_member { + virtual int foo() override { return 0; } +}; + +// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance] +// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B { +class Bad_Child3 : public Interface_with_A_Parent, Base_B { + virtual int baz() override { return 0; } +}; + +// Easy cases of single inheritance +class Simple_Child1 : public Base_A {}; +class Simple_Child2 : public Interface_A { + virtual int foo() override { return 0; } +}; + +// Valid uses of multiple inheritance +class Good_Child1 : public Interface_A, Interface_B { + virtual int foo() override { return 0; } + virtual int bar() override { return 0; } +}; + +class Good_Child2 : public Base_A, Interface_B { + virtual int bar() override { return 0; } +}; + +class Good_Child3 : public Base_A_child, Interface_C, Interface_B { + virtual int bar() override { return 0; } + virtual int blat() override { return 0; } +}; + +int main(void) { + Bad_Child1 a; + Bad_Child2 b; + Bad_Child3 c; + Simple_Child1 d; + Simple_Child2 e; + Good_Child1 f; + Good_Child2 g; + Good_Child3 h; + return 0; +}