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 OverloadedOperatorCheck.cpp StaticallyConstructedObjectsCheck.cpp TrailingReturnCheck.cpp 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" #include "OverloadedOperatorCheck.h" #include "StaticallyConstructedObjectsCheck.h" #include "TrailingReturnCheck.h" @@ -28,6 +29,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "fuchsia-default-arguments"); + CheckFactories.registerCheck( + "fuchsia-multiple-inheritance"); CheckFactories.registerCheck( "fuchsia-overloaded-operator"); CheckFactories.registerCheck( Index: clang-tidy/fuchsia/MultipleInheritanceCheck.h =================================================================== --- clang-tidy/fuchsia/MultipleInheritanceCheck.h +++ clang-tidy/fuchsia/MultipleInheritanceCheck.h @@ -0,0 +1,48 @@ +//===--- 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 implementation inheritance is discouraged. +/// +/// 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 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); + + // 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 =================================================================== --- clang-tidy/fuchsia/MultipleInheritanceCheck.cpp +++ clang-tidy/fuchsia/MultipleInheritanceCheck.cpp @@ -0,0 +1,125 @@ +//===--- 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, hasBases) { + if (Node.hasDefinition()) + return Node.getNumBases() > 0; + return false; +} + +// 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(); + 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) const { + StringRef Name = Node->getIdentifier()->getName(); + llvm::StringMapConstIterator Pair = InterfaceMap.find(Name); + if (Pair == InterfaceMap.end()) + return false; + isInterface = Pair->second; + 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->isPure() && !M->isStatic(); + }); +} + +bool MultipleInheritanceCheck::isInterface(const CXXRecordDecl *Node) { + // Short circuit the lookup if we have analyzed this record before. + bool PreviousIsInterfaceResult; + 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 *Ty = I.getType()->getAs(); + assert(Ty && "RecordType of base class is unknown"); + const RecordDecl *D = Ty->getDecl()->getDefinition(); + if (!D) continue; + const auto *Base = cast(D); + if (!isInterface(Base)) { + addNodeToInterfaceMap(Node, false); + return false; + } + } + + bool CurrentClassIsInterface = isCurrentClassInterface(Node); + addNodeToInterfaceMap(Node, CurrentClassIsInterface); + return CurrentClassIsInterface; +} + +void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) { + // Requires C++. + if (!getLangOpts().CPlusPlus) + return; + + // Match declarations which have bases. + Finder->addMatcher(cxxRecordDecl(hasBases()).bind("decl"), this); +} + +void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *D = Result.Nodes.getNodeAs("decl")) { + // Check against map to see if if the class inherits from multiple + // concrete classes + unsigned NumConcrete = 0; + for (const auto &I : D->bases()) { + if (I.isVirtual()) continue; + const auto *Ty = I.getType()->getAs(); + assert(Ty && "RecordType of base class is unknown"); + const auto *Base = cast(Ty->getDecl()->getDefinition()); + 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 *Ty = V.getType()->getAs(); + assert(Ty && "RecordType of base class is unknown"); + const auto *Base = cast(Ty->getDecl()->getDefinition()); + if (!isInterface(Base)) NumConcrete++; + } + + if (NumConcrete > 1) { + diag(D->getLocStart(), + "inheriting mulitple classes that aren't " + "pure virtual is discouraged"); + } + } +} + +} // namespace fuchsia +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -64,6 +64,11 @@ with looping constructs. Every backward jump is rejected. Forward jumps are only allowed in nested loops. +- New `fuchsia-multiple-inheritance + `_ check + + Warns if a class inherits from multiple classes that are not pure virtual. + - New `fuchsia-statically-constructed-objects `_ check Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst =================================================================== --- docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst +++ 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 @@ -70,6 +70,7 @@ cppcoreguidelines-slicing cppcoreguidelines-special-member-functions fuchsia-default-arguments + fuchsia-multiple-inheritance fuchsia-overloaded-operator fuchsia-statically-constructed-objects fuchsia-trailing-return Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp =================================================================== --- test/clang-tidy/fuchsia-multiple-inheritance.cpp +++ test/clang-tidy/fuchsia-multiple-inheritance.cpp @@ -0,0 +1,131 @@ +// 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 that aren't pure virtual is discouraged [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 that aren't pure virtual is discouraged [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 that aren't pure virtual is discouraged [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; } +}; + +struct B1 { int x; }; +struct B2 { int x;}; +// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes that aren't pure virtual is discouraged [fuchsia-multiple-inheritance] +// CHECK-NEXT: struct D : B1, B2 {}; +struct D1 : B1, B2 {}; + +struct Base1 { virtual void foo() = 0; }; +struct V1 : virtual Base1 {}; +struct V2 : virtual Base1 {}; +struct D2 : V1, V2 {}; + +struct Base2 { virtual void foo(); }; +struct V3 : virtual Base2 {}; +struct V4 : virtual Base2 {}; +struct D3 : V3, V4 {}; + +struct Base3 {}; +struct V5 : virtual Base3 { virtual void f(); }; +struct V6 : virtual Base3 { virtual void g(); }; +// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes that aren't pure virtual is discouraged [fuchsia-multiple-inheritance] +// CHECK-NEXT: struct D4 : V5, V6 {}; +struct D4 : V5, V6 {}; + +struct Base4 {}; +struct V7 : virtual Base4 { virtual void f() = 0; }; +struct V8 : virtual Base4 { virtual void g() = 0; }; +struct D5 : V7, V8 {}; + +struct Base5 { virtual void f() = 0; }; +struct V9 : virtual Base5 { virtual void f(); }; +struct V10 : virtual Base5 { virtual void g() = 0; }; +struct D6 : V9, V10 {}; + +struct Base6 { virtual void f(); }; +struct Base7 { virtual void g(); }; +struct V15 : virtual Base6 { virtual void f() = 0; }; +struct V16 : virtual Base7 { virtual void g() = 0; }; +// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes that aren't pure virtual is discouraged [fuchsia-multiple-inheritance] +// CHECK-NEXT: struct D9 : V15, V16 {}; +struct D9 : V15, V16 {}; + +struct Static_Base { static void foo(); }; +struct V11 : virtual Static_Base {}; +struct V12 : virtual Static_Base {}; +struct D7 : V11, V12 {}; + +struct Static_Base_2 {}; +struct V13 : virtual Static_Base_2 { static void f(); }; +struct V14 : virtual Static_Base_2 { static void g(); }; +struct D8 : V13, V14 {}; +