On Thu, Dec 27, 2018 at 12:18:11AM +0100, Branko Čibej wrote:
> On 26.12.2018 23:44, Daniel Shahaf wrote:
> > Branko Čibej wrote on Wed, 26 Dec 2018 23:37 +0100:
> >> On 26.12.2018 23:21, Daniel Shahaf wrote:
> >>> Branko Čibej wrote on Wed, 26 Dec 2018 22:41 +0100:
> >>>> On 26.12.2018 19:50, Daniel Shahaf wrote:
> >>>>> Haven't reviewed the rest of the patch, nor the mapping of
> >>>>> svn_error_t::apr_err values to this hierarchy.
> >>>> There is just one exception type that encapsulates all of svn_error_t,
> >>>> including the apr_err bit; that's 'svn::error'. I have no intention of
> >>>> going down the rabbit hole of having one exception type for each
> >>>> possible apr_err value!
> >>>>
> >>> Yeah, that'd be way too much; but I was thinking of two things:
> >>>
> >>> 1. apr_err can be an errno error code, not just an APR_OS_START_USERERR
> >>> code. I don't have the C++ exceptions hierarchy in mind, but I
> >>> suspect that when APR_STATUS_IS_EEXIST(err->apr_err), an svn::error
> >>> instance is not what people (and 'catch' blocks) will expect.
> >>
> >> Ah, good point. I hadn't actually thought about this side of things, as
> >> clients "shouldn't" have to worry about them iff our error messages make
> >> any sense. But, yes, adding such predicates would be a big help.
> >>
> >> They don't actually have to be part of svn::error, I'd make them
> >> namespace-scope functions, e.g.:
> >>
> >> bool svn::error_is_eexist(const svn::error& e) noexcept;
> >>
> >>
> >> the point being that the svn::error object serves as both an exception
> >> type and a detailed error description (and that's the reason for
> >> deriving svn::cancelled from svn::error).
> > I don't disagree with adding such predicates, but they aren't my point.
> > What I was trying to say is, doesn't the standard C++ exception
> > hierarchy have some std::* exception class for, say, IO errors? In
> > which case, C++ consumers might be surprised that an svn::error object
> > with apr_err==EEXIST isn't caught by their 'catch' clauses for std::* IO errors?
>
> Not so much. Or at least not as detailed as you'd think:
>
> https://en.cppreference.com/w/cpp/error/exception
Don't forget the system error stuff introduced in c++11:
https://en.cppreference.com/w/cpp/header/system_error
Cheers,
--
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7 2D23 DFE6 91AE 331B A3DB
Received on 2018-12-27 01:18:58 CET