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?
