geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael William Dodge <mdo...@pivotal.io>
Subject Re: [DISCUSS] geode-native c++ exceptions
Date Thu, 14 Sep 2017 21:24:38 GMT
+1 for avoiding multiple inheritance

> On 14 Sep, 2017, at 14:23, Ernest Burghardt <eburghardt@pivotal.io> wrote:
> 
> Sounds like the proposal currently stands to avoid the DiamondOfDeath or
> TriangleOfLove that multiple inheritance brings us....
> and just have the base Exception class inherit std::exception and extend
> Exception class as appropriate within the library.
> 
> +1  and thanks for the code examples, very illistrative!
> 
> +1 to Boost stack trace as well...
> 
> 
> On Thu, Sep 14, 2017 at 10:10 AM, Jacob Barrett <jbarrett@pivotal.io> wrote:
> 
>> The problem stems from the fact that none of the std exceptions virtually
>> inherit from std::exception so you end up in the inheritance triangle of
>> love. I say we avoid the multiple inheritance issues with exceptions by
>> avoiding multiple inheritance altogether in exceptions. See this example.
>> http://coliru.stacked-crooked.com/a/2e32e7777b021e85
>> 
>> Basically all of our exceptions extend our Exception class which extends
>> std::exception. None extend any of the other std exceptions, like
>> bad_alloc, etc. The downside is you can't catch any std exception other
>> than std::exception but the upside is that this actually works. There were
>> also very few exceptions that actually could extend any std exceptions
>> beyond std::exception.
>> 
>> -Jake
>> 
>> 
>> On Wed, Sep 13, 2017 at 10:52 PM Jacob Barrett <jbarrett@pivotal.io>
>> wrote:
>> 
>>> Let me see if I can considerate the consensus here:
>>> 
>>> As reasonably possible throw library specific exceptions from all public
>>> methods.
>>> 
>>> As reasonably possible those exceptions should extend C++11 standard
>>> library exceptions.
>>> 
>>> All exceptions should extend less specific but logically relevant base
>>> exceptions.
>>> 
>>> A few quick examples:
>>> namespace apache {
>>> namespace geode {
>>> namespace client {
>>> 
>>> class Exception : public std::exception {...};
>>> 
>>> class IllegalArgumentException : public Exception, public
>>> std::invalid_argument {...};
>>> 
>>> class TransactionException : public Exception {...};
>>> 
>>> class RollbackException : public TransactionException {...};
>>> 
>>> }
>>> }
>>> }
>>> 
>>> Additionally, investigate using Boost Stack Track library for providing
>>> stack context in exceptions, otherwise dump the current stack tracing
>>> feature that is incomplete and very platform specific.
>>> 
>>> Does anyone have a different understanding of the consensus?
>>> 
>>> 
>>> I found some problems with this model. The IllegalArgumentException would
>>> not be caught in a catch (const std::exception& e) statement do to
>>> multiple inheritance. Another nasty is std::exception doesn't have a
>>> constructor to populate the what() value. Some examples of this problem
>>> can be seen here:
>>> http://coliru.stacked-crooked.com/a/5b6e34c7ea020fc8
>>> 
>>> 
>>> -Jake
>>> 
>>> 
>>> 
>>> 
>>> On Wed, Sep 13, 2017 at 4:37 PM Mark Hanson <mhanson@pivotal.io> wrote:
>>> 
>>>> I think that it would be best to abide by using the std::exception as
>> the
>>>> base. I think it brings with it all the language support and
>> flexibility.
>>>> There are a few penalties obviously to using std::exception as a base,
>> but
>>>> I think they are sufficiently offset by using a standard. As far as the
>>>> number of exceptions, I believe in the philosophy of using as few as
>>>> possible and using inheritance to drive specificity. The benefit is that
>>>> the code can be as simple or as complex as it can be without unnecessary
>>>> overhead e.g error => network error => tcp error. So, I may just care
>> there
>>>> is a network error and the being able to tell if something derives from
>>>> network error is perfect.
>>>> 
>>>> Thanks,
>>>> Mark
>>>> 
>>>> 
>>>>> On Sep 8, 2017, at 1:35 PM, Ernest Burghardt <eburghardt@pivotal.io>
>>>> wrote:
>>>>> 
>>>>> if we continue the merging of Jake <> Sarge comments we might find
>> that
>>>>> std::exception(s) is sufficient if the many name exceptions that
>>>> pertain to
>>>>> the Library are all handled in a similar manner and only have unique
>>>> names
>>>>> for human readability, but not a functional consideration...
>>>>> 
>>>>> EB
>>>>> 
>>>>> On Fri, Sep 8, 2017 at 8:52 AM, Michael William Dodge <
>>>> mdodge@pivotal.io>
>>>>> wrote:
>>>>> 
>>>>>> I subscribe to Josh Gray's philosophy of only having another
>> exception
>>>>>> class if there is something different to be done when it's caught.
>> For
>>>>>> example, if the caller would do the exact same thing for
>>>>>> NoPermissionsException and DiskFullException, just use an IOException
>>>> and
>>>>>> be done with it. I also subscribe to the philosophy that a library
>>>> have its
>>>>>> own exception hierarchy (possibly with a single class), which I think
>>>>>> meshes with Jake's "exceptions exiting a library to be reasonably
>>>> limited
>>>>>> to exceptions generated by and relating to that library".
>>>>>> 
>>>>>> Sarge
>>>>>> 
>>>>>>> On 8 Sep, 2017, at 07:19, Jacob Barrett <jbarrett@pivotal.io>
>> wrote:
>>>>>>> 
>>>>>>> Sorry for jumping on this thread so late. This is an important
issue
>>>> we
>>>>>>> need to address.
>>>>>>> 
>>>>>>> On Thu, Aug 17, 2017 at 11:57 AM David Kimura <dkimura@pivotal.io>
>>>>>> wrote:
>>>>>>> 
>>>>>>>> Using exceptions seems contrary to the Google C++ Style Guide
we
>>>>>> adopted,
>>>>>>>> which states: *"do not use C++ exceptions"* [3
>>>>>>>> <https://google.github.io/styleguide/cppguide.html#Exceptions>].
>>>> Here
>>>>>> is
>>>>>>>> a
>>>>>>>> link [4 <https://www.youtube.com/watch?v=NOCElcMcFik#t=30m29s>]
>> to a
>>>>>>>> cppcon
>>>>>>>> talk defending their decision.  Does it make sense to enforce
this
>>>> rule
>>>>>> on
>>>>>>>> our code base?
>>>>>>>> 
>>>>>>> 
>>>>>>> I don't agree with this approach, as I always tend towards
>>>>>>> progress/modernization, but it was their choice to remain consistent
>>>>>> across
>>>>>>> their code base. I would say if we made the same decision solely
on
>>>>>>> consistency then we would have our own exceptions derived from
a
>> base
>>>>>>> exception class. This is consistent with our Java and .NET as
well
>> as
>>>>>>> current C++ clients.
>>>>>>> 
>>>>>>> 
>>>>>>>> If we decide to knowingly ignore this rule, would we want
to
>>>> continue to
>>>>>>>> propagate custom exceptions or switch to standard exceptions?
 At
>> the
>>>>>> very
>>>>>>>> least, it seems to me that our custom exceptions should derive
from
>>>>>> their
>>>>>>>> most closely matching standard exception counterparts.  Thoughts?
>>>>>>>> 
>>>>>>> 
>>>>>>> I agree with your recommendation of using our exceptions but
extend
>>>> the
>>>>>>> most appropriate C++11 exceptions where applicable. I think it
is
>> good
>>>>>> form
>>>>>>> for exceptions exiting a library to be reasonably limited to
>>>> exceptions
>>>>>>> generated by and relating to that library.
>>>>>>> 
>>>>>>> Our current Exception class also brings with it stack tracing,
>>>> although
>>>>>>> poorly supported on some platforms and disabled by default. Boost
>> just
>>>>>>> recently graduated a stack track library that is supported on
many
>>>>>>> compilers and platforms. It may be worth integrating into our
>>>> exceptions
>>>>>> as
>>>>>>> a replacement for the current stack tracing code.
>>>>>>> 
>>>>>>> -Jake
>>>>>> 
>>>>>> 
>>>> 
>>>> 
>> 


Mime
View raw message