Fix 30 bugs found during code review (#618)

Author: Centimo

.github/workflows/windows.yaml

@@ -51,9 +51,9 @@ jobs:
           cmake -S . -B build -DCMAKE_CXX_STANDARD=20 -DREFLECTCPP_BUILD_TESTS=ON -DREFLECTCPP_JSON=OFF -DREFLECTCPP_${{ matrix.format }}=ON -DCMAKE_BUILD_TYPE=Release
           cmake --build build --config Release -j4 
       - name: Run tests
-        if: matrix.format != 'benchmarks' 
+        if: matrix.format != 'benchmarks'
         run: |
-          ctest --test-dir build --output-on-failure
+          ctest --test-dir build -C Release --output-on-failure
       - name: Run benchmarks 
         if: matrix.format == 'benchmarks' 
         run: |
@@ -106,6 +106,6 @@ jobs:
           cmake --build build --config Release -j4 
       - name: Run tests
         run: |
-          ctest --test-dir build --output-on-failure
+          ctest --test-dir build -C Release --output-on-failure
  
 

MR_DESCRIPTION.md

@@ -34,7 +34,7 @@ struct Field {
   Field(const Field<_name, U>& _field) : value_(_field.get()) {}
 
   template <class U>
-  Field(Field<_name, U>&& _field) : value_(_field.get()) {}
+  Field(Field<_name, U>&& _field) : value_(std::move(_field.get())) {}
 
   template <class U>
     requires std::is_convertible_v<U, Type>

include/rfl/Field.hpp

@@ -29,15 +29,15 @@ struct Flatten {
   Flatten(const Flatten<U>& _f) : value_(_f.get()) {}
 
   template <class U>
-  Flatten(Flatten<U>&& _f) : value_(_f.get()) {}
+  Flatten(Flatten<U>&& _f) : value_(std::move(_f.get())) {}
 
   template <class U>
     requires std::is_convertible_v<U, Type>
   Flatten(const U& _value) : value_(_value) {}
 
   template <class U>
     requires std::is_convertible_v<U, Type>
-  Flatten(U&& _value) : value_(_value) {}
+  Flatten(U&& _value) : value_(std::forward<U>(_value)) {}
 
   ~Flatten() = default;
 
@@ -101,7 +101,7 @@ struct Flatten {
   /// Assigns the underlying object.
   template <class U>
   Flatten& operator=(Flatten<U>&& _f) {
-    value_ = std::forward<U>(_f);
+    value_ = std::move(_f.get());
     return *this;
   }
 

include/rfl/Flatten.hpp

@@ -1,6 +1,7 @@
 #ifndef RFL_GENERIC_HPP_
 #define RFL_GENERIC_HPP_
 
+#include <limits>
 #include <optional>
 #include <ostream>
 #include <string>
@@ -177,6 +178,11 @@ class RFL_API Generic {
         [](auto _v) -> Result<int> {
           using V = std::remove_cvref_t<decltype(_v)>;
           if constexpr (std::is_same_v<V, int64_t>) {
+            if (_v < static_cast<int64_t>(std::numeric_limits<int>::min()) ||
+                _v > static_cast<int64_t>(std::numeric_limits<int>::max())) {
+              return error(
+                  "rfl::Generic: int64_t value out of range for int.");
+            }
             return static_cast<int>(_v);
           } else {
             return error(

include/rfl/Generic.hpp

@@ -26,7 +26,7 @@ class Object {
   using mapped_type = T;
   using value_type = std::pair<std::string, T>;
   using size_type = typename DataType::size_type;
-  using difference_type = typename DataType::size_type;
+  using difference_type = typename DataType::difference_type;
   using reference = value_type&;
   using const_reference = const value_type&;
   using pointer = typename DataType::pointer;

include/rfl/Object.hpp

@@ -211,7 +211,10 @@ class Result {
     requires std::is_convertible_v<U, T>
   auto& operator=(const Result<U>& _other) {
     const auto to_t = [](const U& _u) -> T { return _u; };
-    t_or_err_ = _other.transform(to_t).t_or_err_;
+    auto temp = _other.transform(to_t);
+    destroy();
+    success_ = temp.success_;
+    move_from_other(temp);
     return *this;
   }
 
@@ -292,7 +295,7 @@ class Result {
   }
 
   /// Returns the value or a default.
-  T&& value_or(T&& _default) && noexcept {
+  T value_or(T&& _default) && noexcept {
     if (success_) {
       return std::move(*this).get_t();
     } else {
@@ -332,7 +335,7 @@ class Result {
 
   bool has_value() const noexcept { return success_; }
 
-  Error& error() && {
+  Error&& error() && {
     if (success_) throw std::runtime_error("Expected does not contain value");
     return std::move(*this).get_err();
   }

include/rfl/Result.hpp

@@ -12,7 +12,7 @@ namespace rfl {
 // https://serde.rs/enum-representations.html
 template <internal::StringLiteral _discriminator, class... Ts>
 struct TaggedUnion {
-  static constexpr internal::StringLiteral discrimininator_ = _discriminator;
+  static constexpr internal::StringLiteral discriminator_ = _discriminator;
 
   /// The type of the underlying variant.
   using VariantType = rfl::Variant<Ts...>;

include/rfl/TaggedUnion.hpp

@@ -102,17 +102,19 @@ class Tuple {
     static_assert(sizeof...(Types) == sizeof...(OtherTypes),
                   "The size of the two tuples must be the same.");
 
-    const auto compare = [&]<int _i>(std::strong_ordering* _ordering,
-                                     std::integral_constant<int, _i>) {
-      if (*_ordering != std::strong_ordering::equivalent &&
-          this->get<_i>() != _other.template get<_i>()) {
-        *_ordering = (this->get<_i>() <=> _other.template get<_i>());
-      }
+    using OrderingType = std::common_comparison_category_t<
+        decltype(std::declval<Types>() <=> std::declval<OtherTypes>())...>;
+
+    auto ordering = OrderingType::equivalent;
+
+    const auto compare = [&]<int _i>(std::integral_constant<int, _i>) {
+      ordering = static_cast<OrderingType>(
+          this->get<_i>() <=> _other.template get<_i>());
+      return ordering != 0;
     };
 
     return [&]<int... _is>(std::integer_sequence<int, _is...>) {
-      auto ordering = std::strong_ordering::equivalent;
-      (compare(&ordering, std::integral_constant<int, _is>{}), ...);
+      (compare(std::integral_constant<int, _is>{}) || ...);
       return ordering;
     }(std::make_integer_sequence<int, sizeof...(Types)>());
   }

include/rfl/Tuple.hpp

@@ -124,12 +124,24 @@ struct Reader {
       return value.v_bool;
 
     } else if constexpr (std::is_floating_point<std::remove_cvref_t<T>>()) {
-      if (btype != BSON_TYPE_DOUBLE) {
-        return error(
-            "Could not cast to numeric value. The type must be double, "
-            "int32, int64 or date_time.");
+      switch (btype) {
+        case BSON_TYPE_DOUBLE:
+          return static_cast<T>(value.v_double);
+
+        case BSON_TYPE_INT32:
+          return static_cast<T>(value.v_int32);
+
+        case BSON_TYPE_INT64:
+          return static_cast<T>(value.v_int64);
+
+        case BSON_TYPE_DATE_TIME:
+          return static_cast<T>(value.v_datetime);
+
+        default:
+          return error(
+              "Could not cast to numeric value. The type must be double, "
+              "int32, int64 or date_time.");
       }
-      return static_cast<T>(value.v_double);
 
     } else if constexpr (std::is_integral<std::remove_cvref_t<T>>()) {
       switch (btype) {