svn commit: r1849782 - in /subversion/trunk/subversion/bindings/cxx: include/svnxx/exception.hpp src/exception.cpp src/init.cpp src/private/exception-private.hpp tests/test_exceptions.cpp

[br...@apache.org]

Author: brane
Date: Thu Dec 27 00:28:59 2018
New Revision: 1849782

URL: http://svn.apache.org/viewvc?rev=1849782&view=rev
Log:
More SVN++ exceptions.

 - Renamed 'svn::canceled' back to 'svn::cancelled', grammar
   police please note.
 - Renamed 'svn::cancel' to 'svn::stop_iteration' (thanks danielsh).
 - Changed inhertiance of 'svn::allocation_failed'.
 - Avoid allocating a 'wrapped-error' object when creating smart
   pointers to svn_error_t.
 - More tests etc.

[in subversion/bindings/cxx]
* include/svnxx/exception.hpp: Update docstring.
  (allocation_failed): Derive from std::bad_alloc.
  (detail::svn_error): Renamed from svn_error_t.
  (detail::error_ptr): Updated.
  (cancelled): Renamed from 'canceled'. All uses udpated.
  (stop_iteration): Renamed from 'cancel'. All uses updated.

* private/exception-private.hpp
  (detail::checked_call): Update docstring.
  (detail::iteration_stopped): New.

* src/exception.cpp
  (detail::svn_error): Define.
  (checked_call): Create detail::error_ptr with custom deleters.
  (best_message, error::code, error::name, error::compile_message):
   Update for different structure of detail::error_ptr.
  (stop_iteration::what): Implement.

* src/init.cpp
  (allocation_failed_builder): New, used for constructing
   svn::allocation_failed which now has no public constructors.
  (handle_failed_allocation, create_root_pool): Use it.

* tests/test_exceptions.cpp
  (thrown_error, propagate_error,
   thtrown_cancelled, propagate_cancelled,
   iteration_stopped_cancels, iteration_stopped): New test cases.
  (catch_error, catch_cancelled): Updated.

Modified:
    subversion/trunk/subversion/bindings/cxx/include/svnxx/exception.hpp
    subversion/trunk/subversion/bindings/cxx/src/exception.cpp
    subversion/trunk/subversion/bindings/cxx/src/init.cpp
    subversion/trunk/subversion/bindings/cxx/src/private/exception-private.hpp
    subversion/trunk/subversion/bindings/cxx/tests/test_exceptions.cpp

Modified: subversion/trunk/subversion/bindings/cxx/include/svnxx/exception.hpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/include/svnxx/exception.hpp?rev=1849782&r1=1849781&r2=1849782&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/include/svnxx/exception.hpp (original)
+++ subversion/trunk/subversion/bindings/cxx/include/svnxx/exception.hpp Thu Dec 27 00:28:59 2018
@@ -25,10 +25,10 @@
 #define SVNXX_EXCEPTION_HPP
 
 #include <exception>
-#include <stdexcept>
 #include <memory>
+#include <new>
+#include <stdexcept>
 #include <string>
-#include <utility>
 #include <vector>
 
 /**
@@ -58,14 +58,17 @@
  * All SVN++ exceptions are ultimately derived from @c std:::exception.
  *
  * * <em>std::exception</em>
- *   + <em>std::runtime_error</em>
+ *   + <em>std::bad_alloc</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
+ *   + <em>std::runtime_error</em>
+ *     - apache::subversion::svnxx::error\n
+ *       Thrown when an operation failed (see @ref svn_error_t)
+ *       - apache::subversion::svnxx::cancelled\n
+ *         Thrown when an operation was cancelled, including by the
+ *         user code throwing a @c stop_iteration exception (see
+ *         below)
+ *   + apache::subversion::svnxx::stop_iteration\n
  *     Thrown by user callbacks to terminate iteration
  *
  * @{
@@ -78,23 +81,28 @@ namespace svnxx {
 /**
  * @brief Exception type that will be thrown when memory allocation fails.
  */
-class allocation_failed : public std::runtime_error
+class allocation_failed : public std::bad_alloc
 {
 public:
-  explicit allocation_failed(const std::string& what_arg)
-    : std::runtime_error(what_arg)
-    {}
+  virtual ~allocation_failed() noexcept {}
 
-  explicit allocation_failed(const char* what_arg)
-    : std::runtime_error(what_arg)
+  virtual const char* what() const noexcept override
+    {
+      return reason;
+    }
+
+protected:
+  explicit allocation_failed(const char* what_arg) noexcept
+    : reason(what_arg)
     {}
 
-  virtual ~allocation_failed() {}
+private:
+  const char* reason;
 };
 
 namespace detail {
-struct wrapped_error;
-using error_ptr = std::shared_ptr<wrapped_error>;
+struct svn_error;
+using error_ptr = std::shared_ptr<svn_error>;
 } // namespace detail
 
 /**
@@ -110,10 +118,10 @@ public:
    * Returns the message associated with the top-level error that
    * caused the exception.
    *
-   * @note the returned string is valid onlu as long as the @c error
+   * @note the returned string is valid only as long as this @c error
    *       object is in scope.
    */
-  virtual const char* what() const noexcept;
+  virtual const char* what() const noexcept override;
 
   /**
    * Returns the error code associated with the top-level error that
@@ -198,7 +206,7 @@ public:
     }
 
 protected:
-  error(detail::error_ptr svn_error);
+  error(detail::error_ptr err);
   const char* const m_message;
   std::vector<message> compile_messages(bool show_traces) const;
 };
@@ -207,14 +215,14 @@ protected:
  * Thrown instead of Error when the error chain contains a
  * @c SVN_ERR_CANCELLED error code.
  */
-class canceled : public error
+class cancelled : public error
 {
 public:
-  virtual ~canceled() {}
+  virtual ~cancelled() {}
 
 protected:
-  canceled(detail::error_ptr svn_error)
-    : error(svn_error)
+  cancelled(detail::error_ptr err)
+    : error(err)
     {}
 };
 
@@ -222,12 +230,12 @@ protected:
  * User code should throw this exception from callbacks to
  * cancel an operation.
  */
-class cancel : public std::exception
+class stop_iteration : public std::exception
 {
 public:
-  cancel() {}
-  virtual ~cancel() {}
-  virtual const char* what() const noexcept;
+  stop_iteration() {}
+  virtual ~stop_iteration() {}
+  virtual const char* what() const noexcept override;
 };
 
 } // namespace svnxx

Modified: subversion/trunk/subversion/bindings/cxx/src/exception.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/exception.cpp?rev=1849782&r1=1849781&r2=1849782&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/exception.cpp (original)
+++ subversion/trunk/subversion/bindings/cxx/src/exception.cpp Thu Dec 27 00:28:59 2018
@@ -37,16 +37,49 @@ namespace apache {
 namespace subversion {
 namespace svnxx {
 
+//
+// checked_call
+//
+
 namespace detail {
-struct wrapped_error final
+
+struct svn_error final : svn_error_t {};
+
+void checked_call(svn_error_t* const err)
 {
-  svn_error_t* err;
-  wrapped_error(svn_error_t* const& e) : err(e) {}
-  ~wrapped_error()
+  if (!err)
+    return;
+
+  struct error_builder final : public error
+  {
+    explicit error_builder(error_ptr err_)
+      : error(err_)
+      {}
+  };
+
+  struct cancelled_builder final : public cancelled
+  {
+    explicit cancelled_builder(error_ptr err_)
+      : cancelled(err_)
+      {}
+  };
+
+  static const auto error_deleter =
+    [](svn_error* ptr) noexcept
+      {
+        svn_error_clear(ptr);
+      };
+
+  auto err_ptr = error_ptr(static_cast<svn_error*>(err), error_deleter);
+  for (auto next = err; next; next = next->child)
     {
-      svn_error_clear(err);
+      if (next->apr_err == SVN_ERR_CANCELLED
+          || next->apr_err == SVN_ERR_ITER_BREAK)
+        throw cancelled_builder(err_ptr);
     }
-};
+  throw error_builder(err_ptr);
+}
+
 } // namespace detail
 
 //
@@ -54,20 +87,20 @@ struct wrapped_error final
 //
 
 namespace {
-inline const char* best_message(const detail::error_ptr& wrapper)
+inline const char* best_message(const detail::error_ptr& err)
 {
-  if (!wrapper || !wrapper->err)
+  if (!err)
     return "";
 
   const apr_size_t bufsize = 512;
-  char* buf = static_cast<char*>(apr_palloc(wrapper->err->pool, bufsize));
-  return svn_err_best_message(wrapper->err, buf, bufsize);
+  char* buf = static_cast<char*>(apr_palloc(err->pool, bufsize));
+  return svn_err_best_message(err.get(), buf, bufsize);
 }
 } // anonymous namspace
 
-error::error(detail::error_ptr svn_error)
-  : detail::error_ptr(svn_error),
-    m_message(best_message(svn_error))
+error::error(detail::error_ptr err)
+  : detail::error_ptr(err),
+    m_message(best_message(err))
 {}
 
 const char* error::what() const noexcept
@@ -77,18 +110,17 @@ const char* error::what() const noexcept
 
 int error::code() const noexcept
 {
-  const auto wrapper = get();
-  if (!wrapper || !wrapper->err)
+  const auto err = detail::error_ptr::get();
+  if (!err)
     return 0;
 
-  return static_cast<int>(wrapper->err->apr_err);
+  return static_cast<int>(err->apr_err);
 }
 
 const char* error::name() const noexcept
 {
-  const auto wrapper = get();
-  return svn_error_symbolic_name(!wrapper || !wrapper->err ? 0
-                                 : wrapper->err->apr_err);
+  const auto err = detail::error_ptr::get();
+  return svn_error_symbolic_name(!err ? 0 : err->apr_err);
 }
 
 namespace {
@@ -167,7 +199,7 @@ std::vector<error::message> error::compi
 {
   // Determine the maximum size of the returned list
   std::vector<message>::size_type max_length = 0;
-  for (svn_error_t* err = get()->err; err; err = err->child)
+  for (svn_error_t* err = detail::error_ptr::get(); err; err = err->child)
     {
       if (show_traces && err->file)
         ++max_length;                   // We will display an error location
@@ -184,7 +216,7 @@ std::vector<error::message> error::compi
   empties.reserve(max_length);
 
   apr::pool iterbase;
-  for (svn_error_t* err = get()->err; err; err = err->child)
+  for (svn_error_t* err = detail::error_ptr::get(); err; err = err->child)
     {
       apr::pool::iteration iterpool(iterbase);
 
@@ -207,47 +239,14 @@ std::string error::message::generic_text
 }
 
 //
-// Class cancel
+// Class stop_iteration
 //
 
-const char* cancel::what() const noexcept
+const char* stop_iteration::what() const noexcept
 {
-  return "svn::cancel";
+  return "svn::stop_iteration";
 }
 
-//
-// checked_call
-//
-
-namespace detail {
-void checked_call(svn_error_t* const err)
-{
-  struct error_builder final : public error
-  {
-    explicit error_builder(error_ptr svn_error)
-      : error(svn_error)
-      {}
-  };
-
-  struct canceled_builder final : public canceled
-  {
-    explicit canceled_builder(error_ptr svn_error)
-      : canceled(svn_error)
-      {}
-  };
-
-  if (!err)
-    return;
-
-  auto wrapper = std::make_shared<wrapped_error>(err);
-  for (svn_error_t* next = err; next; next = next->child)
-    {
-      if (next->apr_err == SVN_ERR_CANCELLED)
-        throw canceled_builder(wrapper);
-    }
-  throw error_builder(wrapper);
-}
-} // namespace detail
 } // namespace svnxx
 } // namespace subversion
 } // namespace apache

Modified: subversion/trunk/subversion/bindings/cxx/src/init.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/init.cpp?rev=1849782&r1=1849781&r2=1849782&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/init.cpp (original)
+++ subversion/trunk/subversion/bindings/cxx/src/init.cpp Thu Dec 27 00:28:59 2018
@@ -40,9 +40,16 @@ init::init()
 namespace detail {
 
 namespace {
+struct allocation_failed_builder final : public allocation_failed
+{
+  explicit allocation_failed_builder(const char* what_arg) noexcept
+    : allocation_failed(what_arg)
+    {}
+};
+
 int handle_failed_allocation(int)
 {
-  throw allocation_failed("");
+  throw allocation_failed_builder("svn::allocation_failed");
 }
 
 apr_pool_t* create_root_pool()
@@ -51,14 +58,14 @@ apr_pool_t* create_root_pool()
   apr_allocator_t *allocator = nullptr;
   auto status = apr_allocator_create(&allocator);
   if (status || !allocator)
-    throw allocation_failed("svn++ creating pool allocator");
+    throw allocation_failed_builder("svn++ creating pool allocator");
 
   // Create the root pool.
   apr_pool_t* root_pool = nullptr;
   status = apr_pool_create_ex(&root_pool, nullptr,
                               handle_failed_allocation, allocator);
   if (status || !root_pool)
-    throw allocation_failed("svn++ creating root pool");
+    throw allocation_failed_builder("svn++ creating root pool");
 
 #if APR_POOL_DEBUG
   apr_pool_tag(root_pool, "svn++ root pool");
@@ -69,7 +76,7 @@ apr_pool_t* create_root_pool()
   apr_thread_mutex_t *mutex = nullptr;
   status = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, root_pool);
   if (status || !mutex)
-    throw allocation_failed("svn++ creating allocator mutex");
+    throw allocation_failed_builder("svn++ creating allocator mutex");
   apr_allocator_mutex_set(allocator, mutex);
 #endif
 

Modified: subversion/trunk/subversion/bindings/cxx/src/private/exception-private.hpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/src/private/exception-private.hpp?rev=1849782&r1=1849781&r2=1849782&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/src/private/exception-private.hpp (original)
+++ subversion/trunk/subversion/bindings/cxx/src/private/exception-private.hpp Thu Dec 27 00:28:59 2018
@@ -37,10 +37,18 @@ namespace detail {
 
 /**
  * Given a @a err, if it is not @c nullptr, convert it to a and throw an
- * svn::error or svn::canceled exception; otherwise do nothing.
+ * svn::error or svn::cancelled exception; otherwise do nothing.
  */
 void checked_call(svn_error_t* const err);
 
+/**
+ * Call this when a callback throws svn::stop_iteration to return
+ * an appropriate error.
+ */
+inline svn_error_t* iteration_etopped()
+{
+  return svn_error_create(SVN_ERR_ITER_BREAK, nullptr, nullptr);
+}
 } // namespace detail
 } // namespace svnxx
 } // namespace subversion

Modified: subversion/trunk/subversion/bindings/cxx/tests/test_exceptions.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxx/tests/test_exceptions.cpp?rev=1849782&r1=1849781&r2=1849782&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxx/tests/test_exceptions.cpp (original)
+++ subversion/trunk/subversion/bindings/cxx/tests/test_exceptions.cpp Thu Dec 27 00:28:59 2018
@@ -50,6 +50,13 @@ svn_error_t* make_error_test_error()
 }
 } // anonymous namespace
 
+BOOST_AUTO_TEST_CASE(thrown_error)
+{
+  BOOST_CHECK_THROW(
+      svn::detail::checked_call(make_error_test_error()),
+      svn::error);
+}
+
 BOOST_AUTO_TEST_CASE(catch_error)
 {
   BOOST_CHECK_THROW(
@@ -62,6 +69,10 @@ BOOST_AUTO_TEST_CASE(catch_error)
     }
   catch (const svn::error& err)
     {
+      BOOST_TEST(err.code() == SVN_ERR_UNSUPPORTED_FEATURE);
+      BOOST_TEST(err.name() == "SVN_ERR_UNSUPPORTED_FEATURE");
+      BOOST_TEST(err.what() == "Trying to use an unsupported feature");
+
       auto ml = err.messages();
       BOOST_TEST(ml.size() == 3);
       BOOST_TEST(ml[0].code() == SVN_ERR_UNSUPPORTED_FEATURE);
@@ -93,6 +104,21 @@ BOOST_AUTO_TEST_CASE(catch_error)
     }
 }
 
+BOOST_AUTO_TEST_CASE(propagate_error)
+{
+  std::exception_ptr xptr;
+  try
+    {
+      svn::detail::checked_call(make_error_test_error());
+    }
+  catch (...)
+    {
+      xptr =  std::current_exception();
+    }
+  BOOST_REQUIRE(xptr);
+  BOOST_CHECK_THROW(std::rethrow_exception(xptr), svn::error);
+}
+
 
 namespace {
 svn_error_t* make_cancel_test_error()
@@ -108,18 +134,25 @@ svn_error_t* make_cancel_test_error()
 }
 } // anonymous namespace
 
-BOOST_AUTO_TEST_CASE(catch_canceled)
+BOOST_AUTO_TEST_CASE(thtrown_cancelled)
 {
   BOOST_CHECK_THROW(
       svn::detail::checked_call(make_cancel_test_error()),
-      svn::canceled);
+      svn::cancelled);
+}
 
+BOOST_AUTO_TEST_CASE(catch_cancelled)
+{
   try
     {
       svn::detail::checked_call(make_cancel_test_error());
     }
-  catch (const svn::canceled& err)
+  catch (const svn::cancelled& err)
     {
+      BOOST_TEST(err.code() == SVN_ERR_BASE);
+      BOOST_TEST(err.name() == "SVN_ERR_BASE");
+      BOOST_TEST(err.what() == "wrapper message");
+
       auto ml = err.messages();
       BOOST_TEST(ml.size() == 3);
       BOOST_TEST(ml[0].code() == SVN_ERR_BASE);
@@ -151,4 +184,46 @@ BOOST_AUTO_TEST_CASE(catch_canceled)
     }
 }
 
+BOOST_AUTO_TEST_CASE(propagate_cancelled)
+{
+  std::exception_ptr xptr;
+  try
+    {
+      svn::detail::checked_call(make_cancel_test_error());
+    }
+  catch (...)
+    {
+      xptr =  std::current_exception();
+    }
+  BOOST_REQUIRE(xptr);
+  BOOST_CHECK_THROW(std::rethrow_exception(xptr), svn::cancelled);
+}
+
+BOOST_AUTO_TEST_CASE(iteration_stopped_cancels)
+{
+  BOOST_CHECK_THROW(
+      svn::detail::checked_call(svn::detail::iteration_etopped()),
+      svn::cancelled);
+}
+
+BOOST_AUTO_TEST_CASE(iteration_stopped)
+{
+  try
+    {
+      svn::detail::checked_call(svn::detail::iteration_etopped());
+    }
+  catch (const svn::cancelled& err)
+    {
+      BOOST_TEST(err.code() == SVN_ERR_ITER_BREAK);
+      BOOST_TEST(err.name() == "SVN_ERR_ITER_BREAK");
+      BOOST_TEST(err.what() == "Iteration terminated before completion");
+
+      auto ml = err.messages();
+      BOOST_TEST(ml.size() == 1);
+      BOOST_TEST(ml[0].code() == SVN_ERR_ITER_BREAK);
+      BOOST_TEST(ml[0].name() == "SVN_ERR_ITER_BREAK");
+      BOOST_TEST(ml[0].text() == "Iteration terminated before completion");
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END();