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

Reply via email to