geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacob Barrett <jbarr...@pivotal.io>
Subject Re: [DISCUSS] geode-native c++ exceptions
Date Thu, 14 Sep 2017 16:10:23 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message