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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 5 07:45:24 PST 2017


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





More information about the cfe-commits mailing list