Prefer T(...) over T{...} when they behave differently

...in gen::construct and gen::makeUnique. Also, add support for aggregate initialization in gen::makeShared and tests for all of them.
emil-e · Dec 23, 2018 · 0e25dfa · 0e25dfa
1 parent 3eb9b4f
commit 0e25dfa
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 14 deletions.
diff --git a/include/rapidcheck/detail/ApplyTuple.h b/include/rapidcheck/detail/ApplyTuple.h
@@ -13,7 +13,7 @@ struct ApplyTupleImpl;
 
 template <typename... Ts, std::size_t... Indexes, typename Callable>
 struct ApplyTupleImpl<std::tuple<Ts...>, Callable, IndexSequence<Indexes...>> {
-  using ReturnType = typename std::result_of<Callable(Ts &&...)>::type;
+  using ReturnType = typename std::result_of<Callable&&(Ts &&...)>::type;
 
   static ReturnType apply(std::tuple<Ts...> &&tuple, Callable &&callable) {
     return callable(std::move(std::get<Indexes>(tuple))...);
@@ -24,7 +24,7 @@ template <typename... Ts, std::size_t... Indexes, typename Callable>
 struct ApplyTupleImpl<std::tuple<Ts...> &,
                       Callable,
                       IndexSequence<Indexes...>> {
-  using ReturnType = typename std::result_of<Callable(Ts &...)>::type;
+  using ReturnType = typename std::result_of<Callable&&(Ts &...)>::type;
 
   static ReturnType apply(std::tuple<Ts...> &tuple, Callable &&callable) {
     return callable(std::get<Indexes>(tuple)...);
@@ -35,7 +35,7 @@ template <typename... Ts, std::size_t... Indexes, typename Callable>
 struct ApplyTupleImpl<const std::tuple<Ts...> &,
                       Callable,
                       IndexSequence<Indexes...>> {
-  using ReturnType = typename std::result_of<Callable(const Ts &...)>::type;
+  using ReturnType = typename std::result_of<Callable&&(const Ts &...)>::type;
 
   static ReturnType apply(const std::tuple<Ts...> &tuple, Callable &&callable) {
     return callable(std::get<Indexes>(tuple)...);

diff --git a/include/rapidcheck/gen/Build.hpp b/include/rapidcheck/gen/Build.hpp
@@ -119,15 +119,51 @@ class BuildMapper<T, rc::detail::IndexSequence<Indexes...>, Lenses...> {
   std::tuple<Lenses...> m_lenses;
 };
 
+template<typename T, typename... Args>
+typename std::enable_if<std::is_constructible<T, Args...>::value, T>::type
+construct(Args &&... args) {
+  return T(std::forward<Args>(args)...);
+}
+
+template<typename T, typename... Args>
+typename std::enable_if<!std::is_constructible<T, Args...>::value, T>::type
+construct(Args &&... args) {
+  return T{std::forward<Args>(args)...};
+}
+
+template<typename T, typename... Args>
+typename std::enable_if<std::is_constructible<T, Args...>::value, std::unique_ptr<T>>::type
+makeUnique(Args &&... args) {
+  return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
+}
+
+template<typename T, typename... Args>
+typename std::enable_if<!std::is_constructible<T, Args...>::value, std::unique_ptr<T>>::type
+makeUnique(Args &&... args) {
+  return std::unique_ptr<T>(new T{std::forward<Args>(args)...});
+}
+
+template<typename T, typename... Args>
+typename std::enable_if<std::is_constructible<T, Args...>::value, std::shared_ptr<T>>::type
+makeShared(Args &&... args) {
+  return std::shared_ptr<T>(new T(std::forward<Args>(args)...));
+}
+
+template<typename T, typename... Args>
+typename std::enable_if<!std::is_constructible<T, Args...>::value, std::shared_ptr<T>>::type
+makeShared(Args &&... args) {
+  return std::shared_ptr<T>(new T{std::forward<Args>(args)...});
+}
 } // namespace detail
 
 template <typename T, typename... Args>
 Gen<T> construct(Gen<Args>... gens) {
   return gen::map(gen::tuple(std::move(gens)...),
                   [](std::tuple<Args...> &&argsTuple) {
                     return rc::detail::applyTuple(
-                        std::move(argsTuple),
-                        [](Args &&... args) { return T{std::move(args)...}; });
+                      std::forward<std::tuple<Args...>>(argsTuple),
+                      detail::construct<T, Args...>
+                    );
                   });
 }
 
@@ -141,22 +177,20 @@ Gen<std::unique_ptr<T>> makeUnique(Gen<Args>... gens) {
   return gen::map(gen::tuple(std::move(gens)...),
                   [](std::tuple<Args...> &&argsTuple) {
                     return rc::detail::applyTuple(
-                        std::move(argsTuple),
-                        [](Args &&... args) {
-                          return std::unique_ptr<T>(new T{std::move(args)...});
-                        });
+                      std::forward<std::tuple<Args...>>(argsTuple),
+                      detail::makeUnique<T, Args...>
+                    );
                   });
 }
 
 template <typename T, typename... Args>
 Gen<std::shared_ptr<T>> makeShared(Gen<Args>... gens) {
   return gen::map(gen::tuple(std::move(gens)...),
                   [](std::tuple<Args...> &&argsTuple) {
-                    return rc::detail::applyTuple(std::move(argsTuple),
-                                                  [](Args &&... args) {
-                                                    return std::make_shared<T>(
-                                                        std::move(args)...);
-                                                  });
+                    return rc::detail::applyTuple(
+                      std::forward<std::tuple<Args...>>(argsTuple),
+                      detail::makeShared<T, Args...>
+                    );
                   });
 }
 

diff --git a/test/gen/BuildTests.cpp b/test/gen/BuildTests.cpp
@@ -1,6 +1,7 @@
 #include <catch2/catch.hpp>
 #include <rapidcheck/catch.h>
 
+#include "util/Aggregate.h"
 #include "util/GenUtils.h"
 #include "util/Predictable.h"
 #include "util/Logger.h"
@@ -90,6 +91,16 @@ TEST_CASE("gen::construct") {
     RC_ASSERT(isArbitraryPredictable(std::get<0>(value)));
     RC_ASSERT(isArbitraryPredictable(std::get<1>(value)));
   }
+
+  SECTION("works with aggregate initializers") {
+    gen::construct<Aggregate>(gen::arbitrary<int>(), gen::arbitrary<int>());
+  }
+
+  SECTION("prefers T(x, y) over T{x, y} when they behave differently") {
+    auto a = gen::construct<std::vector<int>>(gen::just(3), gen::just(3))(Random(), 0).value();
+    std::vector<int> b(3, 3);
+    RC_ASSERT(a == b);
+  }
 }
 
 TEST_CASE("gen::makeUnique") {
@@ -122,6 +133,16 @@ TEST_CASE("gen::makeUnique") {
     RC_ASSERT(isArbitraryPredictable(std::get<0>(value)));
     RC_ASSERT(isArbitraryPredictable(std::get<1>(value)));
   }
+
+  SECTION("works with aggregate initializers") {
+    gen::makeUnique<Aggregate>(gen::arbitrary<int>(), gen::arbitrary<int>());
+  }
+
+  SECTION("prefers T(x, y) over T{x, y} when they behave differently") {
+    auto a = gen::makeUnique<std::vector<int>>(gen::just(3), gen::just(3))(Random(), 0).value();
+    auto b = std::unique_ptr<std::vector<int>>(new std::vector<int>(3, 3));
+    RC_ASSERT(*a == *b);
+  }
 }
 
 TEST_CASE("gen::makeShared") {
@@ -154,6 +175,16 @@ TEST_CASE("gen::makeShared") {
     RC_ASSERT(isArbitraryPredictable(std::get<0>(value)));
     RC_ASSERT(isArbitraryPredictable(std::get<1>(value)));
   }
+
+  SECTION("works with aggregate initializers") {
+    gen::makeShared<Aggregate>(gen::arbitrary<int>(), gen::arbitrary<int>());
+  }
+
+  SECTION("prefers T(x, y) over T{x, y} when they behave differently") {
+    auto a = gen::makeShared<std::vector<int>>(gen::just(3), gen::just(3))(Random(), 0).value();
+    auto b = std::shared_ptr<std::vector<int>>(new std::vector<int>(3, 3));
+    RC_ASSERT(*a == *b);
+  }
 }
 
 namespace {

diff --git a/test/util/Aggregate.h b/test/util/Aggregate.h
@@ -0,0 +1,8 @@
+#pragma once
+
+namespace rc {
+  struct Aggregate {
+    int x;
+    int y;
+  };
+} // namespace rc