Re: svn commit: r1849737 - in /subversion/trunk/subversion/bindings/cxx: include/svnxx.hpp include/svnxx/exception.hpp src/aprwrap/pool.hpp src/exception.cpp src/private/exception-private.hpp tests/test_exceptions.cpp

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
Received on 2018-12-26 23:37:30 CET