JonasToth marked 3 inline comments as done.
JonasToth added inline comments.


================
Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:9
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 
----------------
aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > Can you add a test that uses multiple inheritance? e.g.,
> > > ```
> > > class terrible_idea : public non_derived_exception, public 
> > > derived_exception {};
> > > ```
> > > Also, is private inheritance also acceptable, or does it need to be 
> > > public inheritance? I kind of get the impression it needs to be public, 
> > > because the goal appears to be that you should always be able to catch a 
> > > `std::exception` instance, and you can't do that if it's privately 
> > > inherited. That should have a test as well.
> > The rules do not state directly, that it must be inherited public, but i 
> > dont see a good reason to allow non-public inheritance.
> > Another thing is, that you can always call `e.what()` on public derived 
> > exceptions.
> > 
> > Multiple inheritance is harder, since the type is still a `std::exception`. 
> > One could catch it and use its interface, so these reasons are gone to 
> > disallow it.
> > The rules do not state directly, that it must be inherited public, but i 
> > dont see a good reason to allow non-public inheritance.
> 
> Agreed.
> 
> > Another thing is, that you can always call e.what() on public derived 
> > exceptions.
> 
> Agreed.
> 
> > Multiple inheritance is harder, since the type is still a std::exception. 
> > One could catch it and use its interface, so these reasons are gone to 
> > disallow it.
> 
> I think the multiple inheritance case should not diagnose because it meets 
> the HIC++ requirement of being derived from `std::exception`.
I have a problem with implementing the inheritance rules.

From the Matchers, there seems to be no way to test, if the inheritance is 
public. Should i work a new matcher for that, or rather move the tests, if the 
type holds all conditions into the callback function. This would mean, that 
every `throw` gets matched.


https://reviews.llvm.org/D37060



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

Reply via email to