[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
juliehockett marked 4 inline comments as done.
Closed by commit rCTE323011: [clang-tidy] Adding Fuchsia checker for multiple 
inheritance (authored by juliehockett, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40580?vs=130427&id=130714#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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 : virt

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from a few small nits.




Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:85
+void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) {
+  // Match declarations which have bases.
+  Finder->addMatcher(cxxRecordDecl(hasBases()).bind("decl"), this);

No need to register the matchers for languages other than C++.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:113-114
+  diag(D->getLocStart(),
+   "inheriting mulitple classes which aren't "
+   "pure virtual is discouraged");
+}

s/which/that


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 130427.
juliehockett added a comment.

Rebasing from trunk


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ 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 which 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 which 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 which 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 which 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 which 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 which 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 {};
+
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-

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:60
+  // To be an interface, all base classes must be interfaces as well.
+  for (const auto &I : Node->bases()) {
+const auto *Ty = I.getType()->getAs();

rsmith wrote:
> aaron.ballman wrote:
> > juliehockett wrote:
> > > aaron.ballman wrote:
> > > > What about virtual bases (`Node->vbases()`)? This would also be worth 
> > > > some test cases.
> > > Added test cases for virtual, but aren't virtual bases also included in 
> > > `bases()`?
> > No, they are separate in `CXXRecordDecl`.
> That's not quite right. `bases()` contains all direct bases, regardless of 
> whether or not they're virtual. `vbases()` contains all virtual bases, 
> regardless of whether or not they're direct.
Ah, I must have mis-remembered these APIs. Thanks, @rsmith and sorry for the 
noise @juliehockett!


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:60
+  // To be an interface, all base classes must be interfaces as well.
+  for (const auto &I : Node->bases()) {
+const auto *Ty = I.getType()->getAs();

aaron.ballman wrote:
> juliehockett wrote:
> > aaron.ballman wrote:
> > > What about virtual bases (`Node->vbases()`)? This would also be worth 
> > > some test cases.
> > Added test cases for virtual, but aren't virtual bases also included in 
> > `bases()`?
> No, they are separate in `CXXRecordDecl`.
That's not quite right. `bases()` contains all direct bases, regardless of 
whether or not they're virtual. `vbases()` contains all virtual bases, 
regardless of whether or not they're direct.


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 129498.
juliehockett marked 8 inline comments as done.
juliehockett added a comment.

1. Updating check and tests to address virtual inheritance
2. Rebasing from trunk


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ 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 which 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 which 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 which 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 which 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 which 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 which 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 {};
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/cla

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/clang-tidy/fuchsia-multiple-inheritance.cpp:48
+};
+
+// Inherits from multiple concrete classes.

juliehockett wrote:
> aaron.ballman wrote:
> > The virtual base test cases I was thinking of were:
> > ```
> > struct Base { virtual void foo() = 0; };
> > struct V1 : virtual Base {};
> > struct V2 : virtual Base {};
> > struct D : V1, V2 {}; // Should be fine
> > ---
> > struct Base { virtual void foo(); };
> > struct V1 : virtual Base {};
> > struct V2 : virtual Base {};
> > struct D : V1, V2 {}; // Should be fine (there's only one concrete base)?
> > ---
> > struct Base {};
> > struct V1 : virtual Base { virtual void f(); }
> > struct V2 : virtual Base { virtual void g(); }
> > struct D : V1, V2 {}; // Not okay
> > ---
> > struct Base {};
> > struct V1 : virtual Base { virtual void f() = 0; }
> > struct V2 : virtual Base { virtual void g() = 0; }
> > struct D : V1, V2 {}; // Okay
> > ---
> > struct Base { virtual void f(); };
> > struct V1 : virtual Base { virtual void f(); }
> > struct V2 : virtual base { virtual void g() = 0; }
> > struct D : V1, V2 {}; // Should be okay (V1::f() overrides Base::f() which 
> > is only inherited once)?
> > ```
> Ah okay I see what you mean. We're actually following the Google style guide 
> (see [[ https://google.github.io/styleguide/cppguide.html#Inheritance | here 
> ]]), so virtual inheritance is disallowed. There's another check that covers 
> these cases (see [[ https://reviews.llvm.org/D40813 | D40813 ]]).
What about users who disable that check but leave this one enabled? I think the 
crux of the rule is that multiple inheritance of interface features is bad, and 
so I think there's a sensible answer here for virtual bases as well.


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: test/clang-tidy/fuchsia-multiple-inheritance.cpp:48
+};
+
+// Inherits from multiple concrete classes.

aaron.ballman wrote:
> The virtual base test cases I was thinking of were:
> ```
> struct Base { virtual void foo() = 0; };
> struct V1 : virtual Base {};
> struct V2 : virtual Base {};
> struct D : V1, V2 {}; // Should be fine
> ---
> struct Base { virtual void foo(); };
> struct V1 : virtual Base {};
> struct V2 : virtual Base {};
> struct D : V1, V2 {}; // Should be fine (there's only one concrete base)?
> ---
> struct Base {};
> struct V1 : virtual Base { virtual void f(); }
> struct V2 : virtual Base { virtual void g(); }
> struct D : V1, V2 {}; // Not okay
> ---
> struct Base {};
> struct V1 : virtual Base { virtual void f() = 0; }
> struct V2 : virtual Base { virtual void g() = 0; }
> struct D : V1, V2 {}; // Okay
> ---
> struct Base { virtual void f(); };
> struct V1 : virtual Base { virtual void f(); }
> struct V2 : virtual base { virtual void g() = 0; }
> struct D : V1, V2 {}; // Should be okay (V1::f() overrides Base::f() which is 
> only inherited once)?
> ```
Ah okay I see what you mean. We're actually following the Google style guide 
(see [[ https://google.github.io/styleguide/cppguide.html#Inheritance | here 
]]), so virtual inheritance is disallowed. There's another check that covers 
these cases (see [[ https://reviews.llvm.org/D40813 | D40813 ]]).


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 125449.
juliehockett marked an inline comment as done.

https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,138 @@
+// 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 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 which 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 which 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 which 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 {};
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: struct D3 : V3, V4 {};
+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 which 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(); };
+struct V9 : virtual Base5 { virtual void f(); };
+struct V10 : virtual Base5 { virtual void g() = 0; };
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: struct D6 : V9, V10 {};
+struct D6 : V9, V10 {};
+
+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 {};
+
+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;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-ti

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:60
+  // To be an interface, all base classes must be interfaces as well.
+  for (const auto &I : Node->bases()) {
+const auto *Ty = I.getType()->getAs();

juliehockett wrote:
> aaron.ballman wrote:
> > What about virtual bases (`Node->vbases()`)? This would also be worth some 
> > test cases.
> Added test cases for virtual, but aren't virtual bases also included in 
> `bases()`?
No, they are separate in `CXXRecordDecl`.



Comment at: test/clang-tidy/fuchsia-multiple-inheritance.cpp:48
+};
+
+// Inherits from multiple concrete classes.

The virtual base test cases I was thinking of were:
```
struct Base { virtual void foo() = 0; };
struct V1 : virtual Base {};
struct V2 : virtual Base {};
struct D : V1, V2 {}; // Should be fine
---
struct Base { virtual void foo(); };
struct V1 : virtual Base {};
struct V2 : virtual Base {};
struct D : V1, V2 {}; // Should be fine (there's only one concrete base)?
---
struct Base {};
struct V1 : virtual Base { virtual void f(); }
struct V2 : virtual Base { virtual void g(); }
struct D : V1, V2 {}; // Not okay
---
struct Base {};
struct V1 : virtual Base { virtual void f() = 0; }
struct V2 : virtual Base { virtual void g() = 0; }
struct D : V1, V2 {}; // Okay
---
struct Base { virtual void f(); };
struct V1 : virtual Base { virtual void f(); }
struct V2 : virtual base { virtual void g() = 0; }
struct D : V1, V2 {}; // Should be okay (V1::f() overrides Base::f() which is 
only inherited once)?
```


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:60
+  // To be an interface, all base classes must be interfaces as well.
+  for (const auto &I : Node->bases()) {
+const auto *Ty = I.getType()->getAs();

aaron.ballman wrote:
> What about virtual bases (`Node->vbases()`)? This would also be worth some 
> test cases.
Added test cases for virtual, but aren't virtual bases also included in 
`bases()`?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:77
+  // Match declarations which have definitions.
+  Finder->addMatcher(cxxRecordDecl(hasDefinition()).bind("decl"), this);
+}

aaron.ballman wrote:
> It might be nice to not bother matching class definitions that have no bases 
> or virtual bases rather than matching every class definition. However, this 
> could be done in a follow-up patch (it likely requires adding an AST matcher).
Good point -- will do.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.h:34-36
+  bool getInterfaceStatus(const CXXRecordDecl *Node, bool &isInterface);
+  bool isCurrentClassInterface(const CXXRecordDecl *Node);
+  bool isInterface(const CXXRecordDecl *Node);

aaron.ballman wrote:
> I believe all these methods can be marked `const`.
getInterfaceStatus and isInterface can't be -- they update the map. The other 
ones yes though!


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 125215.
juliehockett marked 7 inline comments as done.
juliehockett added a comment.

Updating tests


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,126 @@
+// 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;
+};
+
+class Interface_with_A_Virtual_Parent : public virtual 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 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 which 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 which 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; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child4 : public Interface_with_A_Virtual_Parent, Base_B {
+class Bad_Child4 : public Interface_with_A_Virtual_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; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Virtual_Child1 : public virtual Base_A, public Base_B {};
+class Virtual_Child1 : public Base_A, public virtual Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Virtual_Child1 : public virtual Base_A, Base_B {};
+class Virtual_Child2 : public virtual Base_A, Base_B {};
+
+struct B1 {
+  int x;
+};
+
+struct B2 {
+  int x;
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: struct D : B1, B2 {};
+struct D : B1, B2 {};
+
+int main(void) {
+  Bad_Child1 a;
+  Bad_Child2 b;
+  Bad_Child3 c;
+  Bad_Child4 x;
+  Simple_Child1 d;
+  Simple_Child2 e;
+  Good_Child1 f;
+  Good_Child2 g;
+  Good_Child3 h;
+  Virtual_Child1 i;
+  Virtual_Child2 j;
+  D k;
+  return 0;
+}
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: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
===
---

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:35
+  StringRef Name = Node->getIdentifier()->getName();
+  auto Pair = InterfaceMap.find(Name);
+  if (Pair == InterfaceMap.end())

Eugene.Zelenko wrote:
> aaron.ballman wrote:
> > Don't use `auto` as the type is not spelled out explicitly in the 
> > initialization.
> But such cases are included in modernize-use-auto.
Hmm, I suppose you could maybe make a case that `StringMap::iterator` is 
"complex enough" and that the returned type is sufficiently clear to warrant 
using `auto`...


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:35
+  StringRef Name = Node->getIdentifier()->getName();
+  auto Pair = InterfaceMap.find(Name);
+  if (Pair == InterfaceMap.end())

aaron.ballman wrote:
> Don't use `auto` as the type is not spelled out explicitly in the 
> initialization.
But such cases are included in modernize-use-auto.


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:35
+  StringRef Name = Node->getIdentifier()->getName();
+  auto Pair = InterfaceMap.find(Name);
+  if (Pair == InterfaceMap.end())

Don't use `auto` as the type is not spelled out explicitly in the 
initialization.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:60
+  // To be an interface, all base classes must be interfaces as well.
+  for (const auto &I : Node->bases()) {
+const auto *Ty = I.getType()->getAs();

What about virtual bases (`Node->vbases()`)? This would also be worth some test 
cases.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:77
+  // Match declarations which have definitions.
+  Finder->addMatcher(cxxRecordDecl(hasDefinition()).bind("decl"), this);
+}

It might be nice to not bother matching class definitions that have no bases or 
virtual bases rather than matching every class definition. However, this could 
be done in a follow-up patch (it likely requires adding an AST matcher).



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:85
+unsigned NumConcrete = 0;
+for (const auto &I : D->bases()) {
+  const auto *Ty = I.getType()->getAs();

And virtual bases?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:88
+  assert(Ty && "RecordType of base class is unknown");
+  const auto *Base = cast(Ty->getDecl()->getDefinition());
+  if (!isInterface(Base)) NumConcrete++;

It might make sense to add a degenerate case here to ensure a base class 
without a definition doesn't cause a null pointer dereference. e.g.,
```
struct B;

struct D : B {}; // compile error, B is not a complete type
```
I'm not certain whether Clang's AST will contain the base or not.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.h:34-36
+  bool getInterfaceStatus(const CXXRecordDecl *Node, bool &isInterface);
+  bool isCurrentClassInterface(const CXXRecordDecl *Node);
+  bool isInterface(const CXXRecordDecl *Node);

I believe all these methods can be marked `const`.



Comment at: test/clang-tidy/fuchsia-multiple-inheritance.cpp:80
+};
+
+int main(void) {

Please add a test case consisting of only data members, e.g.,
```
struct B1 {
  int x;
};

struct B2 {
  int x;
};

struct D : B1, B2 {};
```


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 125000.
juliehockett marked 4 inline comments as done.

https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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 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 which 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 which 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; }
+};
+
+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;
+}
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: 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/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -135,6 +135,11 @@
 
   Warns if a function or method is declared or called with default arguments.
 
+- New `fuchsia-multiple-inheritance

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:35-36
+  StringRef Name = Node->getIdentifier()->getName();
+  if (InterfaceMap.count(Name)) {
+isInterface = InterfaceMap.lookup(Name);
+return true;

One lookup is enough here. Use `StringMap::find()` instead of count + lookup.


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124949.
juliehockett added a comment.

Updated warning wording to more accurately reflect guidelines


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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 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 which 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 which 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; }
+};
+
+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;
+}
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: 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/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -135,6 +135,11 @@
 
   Warns if a function or method is declared or called with default ar

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:46
+
+See the features disallowed in Fuchsia at 
https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md

juliehockett wrote:
> alexfh wrote:
> > This is not about the check, rather about the underlying style guide. The 
> > document linked here doesn't explain why certain features are disallowed. 
> > I'd suggest putting some effort in expanding the document to include 
> > reasoning for each rule (e.g. see 
> > https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance for 
> > a related rule in the Google C++ style guide).
> Good point -- we're looking into updating it. Thanks!
Hmm, the document linked here explicitly says that multiple inheritance of 
non-interface classes is allowed:

> * Allowed
>* ...
>* Multiple implementation inheritance
>* But be judicious. This is used widely for e.g. intrusive container 
> mixins.

That seems to directly contradict the warning produced by this check, 
`inheriting mulitple classes which aren't pure virtual is disallowed`. Should 
it just say "... is discouraged"?


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:46
+
+See the features disallowed in Fuchsia at 
https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md

alexfh wrote:
> This is not about the check, rather about the underlying style guide. The 
> document linked here doesn't explain why certain features are disallowed. I'd 
> suggest putting some effort in expanding the document to include reasoning 
> for each rule (e.g. see 
> https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance for a 
> related rule in the Google C++ style guide).
Good point -- we're looking into updating it. Thanks!


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124853.
juliehockett added a comment.

Rebasing for updated Release Notes -- so much nicer :)


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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;
+}
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: 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/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -135,6 +135,11 @@
 
   Warns if a function or method is declared or called with default arguments.
 

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please rebase from trunk. I just enforced some order in chaos of Release Notes 
:-)


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124772.
juliehockett marked 11 inline comments as done.
juliehockett added a comment.

Moved AST matcher to ASTMatchers.h (see D40611 
), addressing comments.


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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;
+}
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: 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/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -1

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:58
+  // Interfaces should have exclusively pure methods.
+  for (auto method : Node->methods()) {
+if (method->isUserProvided() && !method->isPure()) {

Eugene.Zelenko wrote:
> const auto?
Should be `const auto *`; also `method` does not meet our naming convention 
rules. Actually, the whole thing can be replaced by `return 
llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) { return 
M->isUserProvided() && !M->isPure(); });`



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:21
+
+AST_MATCHER(CXXRecordDecl, hasDefinition) {
+  if (!Node.hasDefinition())

This is another example of an AST matcher that should simply be made public in 
ASTMatchers.h (as a separate commit). Feel free to add me as a reviewer to that 
patch (it should be pretty trivial).



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:29-30
+// previously.
+void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node,
+bool isInterface) {
+  StringRef Name = Node->getIdentifier()->getName();

Formatting is off here -- be sure to run the patch through clang-format.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:40
+bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node,
+bool *isInterface) {
+  StringRef Name = Node->getIdentifier()->getName();

Rather than passing a pointer, why not pass a reference?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:49
+
+// TODO(smklein): Make an exception for 'Dispatcher' -- consider it an
+// interface, even though it isn't.

We don't put names into our TODO or FIXME comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:54
+  // Interfaces should have no fields.
+  if (!Node->field_empty()) {
+return false;

Can elide braces, here and elsewhere.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:102
+// concrete classes
+int NumConcrete = 0;
+for (const auto &I : D->bases()) {

Might as well make this `unsigned` rather than `int`.


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:22
+AST_MATCHER(CXXRecordDecl, hasDefinition) {
+  if (!Node.hasDefinition())
+return false;

`return Node.hasDefinition();`



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:32
+  StringRef Name = Node->getIdentifier()->getName();
+  MultipleInheritanceCheck::InterfaceMap->insert(
+std::make_pair(Name, isInterface));

What's the reason to qualify `InterfaceMap` and other members of this class?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.h:35-38
+  void addNodeToInterfaceMap(const CXXRecordDecl *Node, bool isInterface);
+  bool getInterfaceStatus(const CXXRecordDecl *Node, bool *isInterface);
+  bool isCurrentClassInterface(const CXXRecordDecl *Node);
+  bool isInterface(const CXXRecordDecl *Node);

Do all these methods need to be public?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.h:44
+  // where N is the number of classes.
+  llvm::StringMap *InterfaceMap;
+};

What's the reason to use a pointer and dynamically allocate the map? Just make 
it a value and clear it instead of deleting.



Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:46
+
+See the features disallowed in Fuchsia at 
https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md

This is not about the check, rather about the underlying style guide. The 
document linked here doesn't explain why certain features are disallowed. I'd 
suggest putting some effort in expanding the document to include reasoning for 
each rule (e.g. see 
https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance for a 
related rule in the Google C++ style guide).


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124675.
juliehockett added a comment.

Fixing typo


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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;
+}
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: 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/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -114,6 +114,11 @@
   `_ check
 
   Warns if a function or method 

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124669.
juliehockett marked 4 inline comments as done.

https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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;
+}
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: 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/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -114,6 +114,11 @@
   `_ check
 
   Warns if a function or meth

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:76
+  for (const auto &I : Node->bases()) {
+const RecordType *Ty = I.getType()->getAs();
+assert(Ty && "RecordType of base class is unknown");

Could be const auto *.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:98
+void MultipleInheritanceCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const CXXRecordDecl *D =
+Result.Nodes.getNodeAs("decl")) {

Could be const auto *.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:104
+for (const auto &I : D->bases()) {
+  const RecordType *Ty = I.getType()->getAs();
+  assert(Ty && "RecordType of base class is unknown");

Could be const auto *.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:106
+  assert(Ty && "RecordType of base class is unknown");
+  CXXRecordDecl *Base = 
cast(Ty->getDecl()->getDefinition());
+  if (!MultipleInheritanceCheck::isInterface(Base))

Could be const auto *.


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124662.
juliehockett marked 6 inline comments as done.

https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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;
+}
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: 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/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -114,6 +114,11 @@
   `_ check
 
   Warns if a function or meth

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:21
+
+  AST_MATCHER(CXXRecordDecl, hasDefinition) {
+if (!Node.hasDefinition())

Please reduce indentation level.



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:58
+  // Interfaces should have exclusively pure methods.
+  for (auto method : Node->methods()) {
+if (method->isUserProvided() && !method->isPure()) {

const auto?



Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:78
+assert(Ty && "RecordType of base class is unknown");
+CXXRecordDecl *Base = cast(Ty->getDecl()->getDefinition());
+if (!MultipleInheritanceCheck::isInterface(Base)) {

Could be (const?) auto *, since you spell type in cast. Same in other places.



Comment at: docs/ReleaseNotes.rst:60
 
+- New `fuchsia-multiple-inheritance
+  
`_
 check

Please move it to previous fuchsia check in alphabetical order.



Comment at: docs/ReleaseNotes.rst:65
+
 - New `objc-avoid-spinlock
   `_ 
check

Please move this in alphabetical order after renamed checks.



Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:8
+
+For example, Declaring a class that inherits from multiple concrete classes is
+disallowed:

Declaring -> declaring 


https://reviews.llvm.org/D40580



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.

Adds a check to the Fuchsia module to warn when a class inheritsfrom multiple 
classes that are not pure virtual.

See https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md for reference.


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

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;
+}
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: 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/ReleaseNotes.rst
==