On 26.12.2018 19:50, Daniel Shahaf wrote:
> br...@apache.org wrote on Wed, 26 Dec 2018 04:28 +0000:
>> Reimplement exceptions in SVN++.
>>
>> Instead of extracting error messages from the svn_error_t when the exception
>> is created, keep the svn_error_t embedded in the exception and use its
>> contents
>> only when needed.
>> +++ subversion/trunk/subversion/bindings/cxx/include/svnxx/exception.hpp Wed
>> Dec 26 04:28:25 2018
>> @@ -31,6 +31,46 @@
>> +/**
>> + * @defgroup svnxx_exceptions Exceptions
>> + *
>> + * Exceptions in SVN++
>> + * ===================
>> + *
>> + * SVN++ uses exceptions for the following purposes:
>> + * @li Reporting memory allocation failure; where Subversion's
>> + * default hehaviour is to abort when an allocation from an APR
>> + * pool fails, SVN++ throws an exception instead.
>> + * @li Reporting errors; Subversion's error messages are wrapped in
>> + * exceptions.
>> + * @li Reporting cancelled operations; an operation that was
>> + * cancelled from user code will report this by throwing a
>> + * specific exception type.
>> + * @li Terminating iteration; user-level callbacks may throw a
>> + * specific exception type to cancel an ongoing operation that
>> + * is generating the callback messages. Other exceptions from
>> + * user-level callbacks will be propagated back to the calling
>> + * application.
> This part is an excellent overview; if it's not already in the C doxygen
> docs or in HACKING, it should be (mutatis mutandis).
>
>> + * Exception Hierarchy
>> + * -------------------
>> + *
>> + * All SVN++ exceptions are ultimately derived from @c std:::exception.
>> + *
>> + * * <em>std::exception</em>
>> + * + <em>std::runtime_error</em>
>> + * - apache::subversion::svnxx::allocation_failed\n
>> + * Thrown when memory cannot be allocated from an APR pool
>> + * + apache::subversion::svnxx::error\n
>> + * Thrown when an operation failed (see @ref svn_error_t)
>> + * - apache::subversion::svnxx::canceled\n
>> + * Thrown when an operation was canceled
>> + * + apache::subversion::svnxx::cancel\n
>> + * Thrown by user callbacks to terminate iteration
> I think the last two names here are too similar; it'll be way too easy
> to mis-tab-complete them for each other. Suggest to rename the second
> one to stop_iteration to match the name of SVN_ERR_STOP_ITERATION.
Actually users can't create objects of type 'svn::canceled' — or at
least not without a lot of effort, as there are no public constructors.
I would hope a smart IDE would notice, or else at least a smart user
will. :)
But yes, I hear you. stop_iteration is a good suggestion.
> 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!
TL;DR:
class error : public std::exception,
protected std::shared_ptr<svn_error_t>
{
public:
// ...
const char* what(); // "best" error message (override)
int code(); // converted apr_status_t
const char* name(); // symbolic error name, when available
// ...
};
