getml / reflect-cpp

A C++20 library for fast serialization, deserialization and validation using reflection. Supports JSON, BSON, CBOR, flexbuffers, msgpack, TOML, XML, YAML / msgpack.org[C++20]
https://getml.github.io/reflect-cpp/
MIT License
1.05k stars 90 forks source link

rfl::Variant visit not working with "Move-Only" types with `const` fields #228

Open Urfoex opened 2 days ago

Urfoex commented 2 days ago

Related to https://github.com/getml/reflect-cpp/issues/184

The const field creates errors.

#include "rfl/TaggedUnion.hpp"
#include <rfl/Variant.hpp>
#include <rfl/visit.hpp>

struct MoveOnly {
    MoveOnly() = default;
    MoveOnly(MoveOnly &&) = default;
    auto operator=(MoveOnly &&) -> MoveOnly & = default;
    MoveOnly(MoveOnly const &) = delete;
    auto operator=(MoveOnly const &) -> MoveOnly & = delete;
    ~MoveOnly() = default;

    int const index = {};
};

using TaggedUnionMoveOnly = rfl::TaggedUnion<"MoveOnly", MoveOnly>;
using VariantMoveOnly = rfl::Variant<MoveOnly>;

auto main() -> int {
    rfl::visit([](auto &&) { return MoveOnly{}; }, VariantMoveOnly{});
    rfl::visit([](auto &&) { return MoveOnly{}; },
               TaggedUnionMoveOnly{MoveOnly{}});
    return 0;
}
include/rfl/Variant.hpp:359:15: error: use of deleted function ‘std::optional<MoveOnly>& std::optional<MoveOnly>::operator=(std::optional<MoveOnly>&&)’
[1/2] Building CXX object CMakeFiles/move_only.dir/main.cpp.o
FAILED: CMakeFiles/move_only.dir/main.cpp.o 
/usr/bin/c++  -Ireflectcpp-move-only/build/_deps/reflectcpp-src/include -Ireflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/thirdparty -Ireflectcpp-move-only/build/_deps/reflectcpp-build/include -std=gnu++23 -Wall -Wextra -pedantic -O0 -g3 -ggdb -fdiagnostics-color=always -MD -MT CMakeFiles/move_only.dir/main.cpp.o -MF CMakeFiles/move_only.dir/main.cpp.o.d -fmodules-ts -fmodule-mapper=CMakeFiles/move_only.dir/main.cpp.o.modmap -MD -fdeps-format=p1689r5 -x c++ -o CMakeFiles/move_only.dir/main.cpp.o -c reflectcpp-move-only/main.cpp
In file included from reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/define_literal.hpp:4,
                 from reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/TaggedUnion.hpp:5,
                 from reflectcpp-move-only/main.cpp:1:
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Literal.hpp: In instantiation of ‘class rfl::Literal<>’:
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/internal/../internal/get_field_names.hpp:129:55:   required from here
  129 | inline auto concat_literals() { return rfl::Literal<>(); }
      |                                                       ^
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Literal.hpp:32:45: warning: comparison is always true due to limited range of data type [-Wtype-limits]
   32 |       std::conditional_t<sizeof...(fields_) <=
      |                          ~~~~~~~~~~~~~~~~~~~^~
   33 |                              std::numeric_limits<std::uint8_t>::max(),
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Literal.hpp:32:45: warning: comparison is always true due to limited range of data type [-Wtype-limits]
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Literal.hpp:32:45: warning: comparison is always true due to limited range of data type [-Wtype-limits]
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Literal.hpp:32:45: warning: comparison is always true due to limited range of data type [-Wtype-limits]
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Literal.hpp:32:45: warning: comparison is always true due to limited range of data type [-Wtype-limits]
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Literal.hpp:32:45: warning: comparison is always true due to limited range of data type [-Wtype-limits]
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Literal.hpp:32:45: warning: comparison is always true due to limited range of data type [-Wtype-limits]

In file included from reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/TaggedUnion.hpp:4:
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp: In instantiation of ‘rfl::Variant<MoveOnly>::do_visit_with_result<main()::<lambda(auto:379&&)>, MoveOnly, 0>(const main()::<lambda(auto:379&&)>&, std::optional<MoveOnly>*, std::integer_sequence<unsigned char, 0>)::<lambda(const main()::<lambda(auto:379&&)>&, std::optional<MoveOnly>*, rfl::Variant<MoveOnly>::Index<_i>)> [with std::conditional<true, unsigned char, short unsigned int>::type _i = 0; rfl::Variant<MoveOnly>::Index<_i> = std::integral_constant<unsigned char, 0>]’:
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:362:15:   required from ‘void rfl::Variant<AlternativeTypes>::do_visit_with_result(const F&, std::optional<ResultType>*, std::integer_sequence<typename std::conditional<(sizeof... (AlternativeTypes) <= std::numeric_limits<unsigned char>::max()), unsigned char, short unsigned int>::type, _is ...>) [with F = main()::<lambda(auto:379&&)>; ResultType = MoveOnly; typename std::conditional<(sizeof... (AlternativeTypes) <= std::numeric_limits<unsigned char>::max()), unsigned char, short unsigned int>::type ..._is = {0}; AlternativeTypes = {MoveOnly}; typename std::conditional<(sizeof... (AlternativeTypes) <= std::numeric_limits<unsigned char>::max()), unsigned char, short unsigned int>::type = unsigned char]’
  362 |     (visit_one(_f, _res, Index<_is>{}), ...);
      |      ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:208:27:   required from ‘rfl::Variant<AlternativeTypes>::result_t<F> rfl::Variant<AlternativeTypes>::visit(const F&) [with F = main()::<lambda(auto:379&&)>; AlternativeTypes = {MoveOnly}; result_t<F> = MoveOnly]’
  208 |       do_visit_with_result(_f, &res,
      |       ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
  209 |                            std::make_integer_sequence<IndexType, size_>());
      |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/visit.hpp:52:18:   required from ‘rfl::internal::variant::result_t<F, AlternativeTypes ...> rfl::visit(const F&, Variant<Types ...>&&) [with F = main()::<lambda(auto:379&&)>; AlternativeTypes = {MoveOnly}; internal::variant::result_t<F, AlternativeTypes ...> = MoveOnly]’
   52 |   return _v.visit(_f);
      |          ~~~~~~~~^~~~
reflectcpp-move-only/main.cpp:20:15:   required from here
   20 |     rfl::visit([](auto &&) { return MoveOnly{}; }, VariantMoveOnly{});
      |     ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:359:15: error: use of deleted function ‘std::optional<MoveOnly>& std::optional<MoveOnly>::operator=(std::optional<MoveOnly>&&)’
  359 |         *_res = _f(get_alternative<_i>());
      |         ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:8:
/usr/include/c++/14/optional:703:11: note: ‘std::optional<MoveOnly>& std::optional<MoveOnly>::operator=(std::optional<MoveOnly>&&)’ is implicitly deleted because the default definition would be ill-formed:
  703 |     class optional
      |           ^~~~~~~~
/usr/include/c++/14/optional:703:11: error: use of deleted function ‘std::_Enable_copy_move<false, false, true, false, _Tag>& std::_Enable_copy_move<false, false, true, false, _Tag>::operator=(std::_Enable_copy_move<false, false, true, false, _Tag>&&) [with _Tag = std::optional<MoveOnly>]’
In file included from /usr/include/c++/14/optional:45:
/usr/include/c++/14/bits/enable_special_members.h:261:5: note: declared here
  261 |     operator=(_Enable_copy_move&&) noexcept                         = delete;
      |     ^~~~~~~~
/usr/include/c++/14/optional:703:11: note: use ‘-fdiagnostics-all-candidates’ to display considered candidates
  703 |     class optional
      |           ^~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:359:15: note: use ‘-fdiagnostics-all-candidates’ to display considered candidates
  359 |         *_res = _f(get_alternative<_i>());
      |         ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp: In instantiation of ‘rfl::Variant<MoveOnly>::do_visit_with_result<main()::<lambda(auto:380&&)>, MoveOnly, 0>(const main()::<lambda(auto:380&&)>&, std::optional<MoveOnly>*, std::integer_sequence<unsigned char, 0>)::<lambda(const main()::<lambda(auto:380&&)>&, std::optional<MoveOnly>*, rfl::Variant<MoveOnly>::Index<_i>)> [with std::conditional<true, unsigned char, short unsigned int>::type _i = 0; rfl::Variant<MoveOnly>::Index<_i> = std::integral_constant<unsigned char, 0>]’:
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:362:15:   required from ‘void rfl::Variant<AlternativeTypes>::do_visit_with_result(const F&, std::optional<ResultType>*, std::integer_sequence<typename std::conditional<(sizeof... (AlternativeTypes) <= std::numeric_limits<unsigned char>::max()), unsigned char, short unsigned int>::type, _is ...>) [with F = main()::<lambda(auto:380&&)>; ResultType = MoveOnly; typename std::conditional<(sizeof... (AlternativeTypes) <= std::numeric_limits<unsigned char>::max()), unsigned char, short unsigned int>::type ..._is = {0}; AlternativeTypes = {MoveOnly}; typename std::conditional<(sizeof... (AlternativeTypes) <= std::numeric_limits<unsigned char>::max()), unsigned char, short unsigned int>::type = unsigned char]’
  362 |     (visit_one(_f, _res, Index<_is>{}), ...);
      |      ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:208:27:   required from ‘rfl::Variant<AlternativeTypes>::result_t<F> rfl::Variant<AlternativeTypes>::visit(const F&) [with F = main()::<lambda(auto:380&&)>; AlternativeTypes = {MoveOnly}; result_t<F> = MoveOnly]’
  208 |       do_visit_with_result(_f, &res,
      |       ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
  209 |                            std::make_integer_sequence<IndexType, size_>());
      |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/visit.hpp:96:39:   required from ‘rfl::internal::variant::result_t<F, AlternativeTypes ...> rfl::visit(const F&, TaggedUnion<_discriminator, AlternativeTypes ...>&&) [with F = main()::<lambda(auto:380&&)>; StringLiteral<...auto...> _discriminator = internal::StringLiteral<9>{std::array<char, 9>{"MoveOnly"}}; AlternativeTypes = {MoveOnly}; internal::variant::result_t<F, AlternativeTypes ...> = MoveOnly]’
   96 |   return _tagged_union.variant().visit(_f);
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
reflectcpp-move-only/main.cpp:21:15:   required from here
   21 |     rfl::visit([](auto &&) { return MoveOnly{}; },
      |     ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   22 |                TaggedUnionMoveOnly{MoveOnly{}});
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:359:15: error: use of deleted function ‘std::optional<MoveOnly>& std::optional<MoveOnly>::operator=(std::optional<MoveOnly>&&)’
  359 |         *_res = _f(get_alternative<_i>());
      |         ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
reflectcpp-move-only/build/_deps/reflectcpp-src/include/rfl/Variant.hpp:359:15: note: use ‘-fdiagnostics-all-candidates’ to display considered candidates
ninja: build stopped: subcommand failed.

Info: Tried adjusting https://github.com/getml/reflect-cpp/blob/main/tests/json/test_rfl_variant_visit_move_only.cpp to have const fields, but std::unique_ptr const fails. But also for std::variant, so nothing lost there.

Urfoex commented 2 days ago

✨✨ Here's an AI-assisted sketch of how you might approach this issue saved by @Urfoex using Copilot Workspace v0.25