Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Glaze does not detect variant objects within variants #1096

Open
shiretu opened this issue Jun 13, 2024 · 2 comments
Open

Glaze does not detect variant objects within variants #1096

shiretu opened this issue Jun 13, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@shiretu
Copy link

shiretu commented Jun 13, 2024

Hi,

Consider the following code snippet from the library, at which I have added some debugging:

using object_types = typename variant_types<T>::object_types;
std::cerr << "T: " << get_type_string<T>() << "\n";
std::cerr << "object_types: " << get_type_string<object_types>() << "\n";
std::cerr << "glz::tuple_size_v<object_types>: " << glz::tuple_size_v<object_types> << "\n";

the output looks like this:

T: std::variant<connected, std::variant<subscribed_v4_orderbook, subscribed_v4_subaccounts, subscribed_v4_trades>, std::variant<channel_data_v4_orderbook, channel_data_v4_trades>>
object_types: glz::tuplet::tuple<connected>
glz::tuple_size_v<object_types>: 1

That is not correct. Our input T type is a std::variant consisting of 3 types. last 2 types from this variant are themselves variants. The wanted output should have been:

object_types: glz::tuplet::tuple<connected, std::variant<subscribed_v4_orderbook, subscribed_v4_subaccounts, subscribed_v4_trades>, std::variant<channel_data_v4_orderbook, channel_data_v4_trades>>
glz::tuple_size_v<object_types>: 3
@shiretu
Copy link
Author

shiretu commented Jun 13, 2024

Created this PR #1098 which also includes fixes for issue #1092

@stephenberry stephenberry added the bug Something isn't working label Jun 13, 2024
@stephenberry stephenberry added the enhancement New feature or request label Aug 8, 2024
@stephenberry
Copy link
Owner

This issue is due to the fact that a std::variant isn't necessarily an object. In this example the variant types all contain object types, so they should be considered objects. However, this recursive compile time type analysis is not simple, especially when trying to filter types.

I think the filtering code needs to be improved and optimized before this feature is added. I've pasted the current filtering code below. I'm removing the bug label because this is technically a limitation and not broken code.

I did improve the std::variant handling logic recently and added support for std::shared_ptr and std::unique_ptr in variants, but this effort is not a primary focus right now.

template <class>
struct variant_types;

template <class... Ts>
struct variant_types<std::variant<Ts...>>
{
   // TODO: this way of filtering types is compile time intensive.
   using bool_types = decltype(tuplet::tuple_cat(
      std::conditional_t<bool_t<remove_meta_wrapper_t<Ts>>, tuplet::tuple<Ts>, tuplet::tuple<>>{}...));
   using number_types = decltype(tuplet::tuple_cat(
      std::conditional_t<num_t<remove_meta_wrapper_t<Ts>>, tuplet::tuple<Ts>, tuplet::tuple<>>{}...));
   using string_types = decltype(tuplet::tuple_cat( // glaze_enum_t remove_meta_wrapper_t supports constexpr
                                                    // types while the other supports non const
      std::conditional_t < str_t<remove_meta_wrapper_t<Ts>> || glaze_enum_t<remove_meta_wrapper_t<Ts>> ||
         glaze_enum_t<Ts>,
      tuplet::tuple<Ts>, tuplet::tuple < >> {}...));
   using object_types =
      decltype(tuplet::tuple_cat(std::conditional_t<json_object<Ts>, tuplet::tuple<Ts>, tuplet::tuple<>>{}...));
   using array_types =
      decltype(tuplet::tuple_cat(std::conditional_t < array_t<remove_meta_wrapper_t<Ts>> || glaze_array_t<Ts>,
                                 tuplet::tuple<Ts>, tuplet::tuple < >> {}...));
   using nullable_types =
      decltype(tuplet::tuple_cat(std::conditional_t<null_t<Ts>, tuplet::tuple<Ts>, tuplet::tuple<>>{}...));
   using nullable_objects = decltype(tuplet::tuple_cat(
      std::conditional_t<is_memory_object<Ts>, tuplet::tuple<Ts>, tuplet::tuple<>>{}...));
};

@stephenberry stephenberry removed the bug Something isn't working label Aug 8, 2024
@stephenberry stephenberry changed the title Tuple routines not working as expected Glaze does not detect variant objects within variants Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants