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). -- Brane
