subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko Čibej <>
Subject 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
Date Thu, 27 Dec 2018 00:36:25 GMT
On 27.12.2018 01:18, James McCoy wrote:
> 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
>>>>>> 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:
> Don't forget the system error stuff introduced in c++11:

Good point, we should be able to map that.

-- Brane

View raw message