stephenberry / glaze

Extremely fast, in memory, JSON and interface library for modern C++
MIT License
1.17k stars 118 forks source link

Reference or incorporate simple_enum support #874

Open arturbac opened 7 months ago

arturbac commented 7 months ago

I asked You on other discussion for help You answered only to first question suggesting meta So i took of the constraint if constexpr (write_json_invocable<Opts, T, Ctx, B, IX>) to see what will fail. And it fails because of ambiguity even when having meta for partial specializations So for write and read(similar) I think You are missing glaze is missing this below What is interesting when I change to bounded enum so more constrained enum a subset of normal enum it works out of the box .. But for normal enum You need that fixes below : (I dont need them but for glaze I think it would be nice to respect own meta<> customisations by users

      template <class T>
         requires(std::is_enum_v<std::decay_t<T>> && !glaze_enum_t<T> 
         && !meta<std::decay_t<T>>::custom_write // <----- glaze is missing this
         )
      struct to_json<T>
      {
         template <auto Opts, class... Args>
         GLZ_ALWAYS_INLINE static void op(auto&& value, is_context auto&& ctx, Args&&... args) noexcept
         {
            write<json>::op<Opts>(static_cast<std::underlying_type_t<std::decay_t<T>>>(value), ctx,
                                  std::forward<Args>(args)...);
         }
      };

Do You agree ?

I took off constraint to check here

     template <>
      struct write<json>
      {
         template <auto Opts, class T, is_context Ctx, class B, class IX>
         GLZ_ALWAYS_INLINE static void op(T&& value, Ctx&& ctx, B&& b, IX&& ix) noexcept
         {
            // if constexpr (write_json_invocable<Opts, T, Ctx, B, IX>) {
               to_json<std::remove_cvref_t<T>>::template op<Opts>(std::forward<T>(value), std::forward<Ctx>(ctx),
                                                                  std::forward<B>(b), std::forward<IX>(ix));
            // }
            // else {
               // static_assert(false_v<std::remove_cvref_t<T>>, "Glaze metadata is probably needed for your type");
            // }
         }
      };

I am going to add galze support for simple enum so I will not need to use glz::enuemrate as I will use simple_enum::enum_view for automatic enumerations for all bounded_enums at once And simple_enum with galze can be symbiotically working and galze support would be then included in simple enum

glaze_json_enum_name.hpp

glaze_enum_name_ut.cc

It is up to You if You want to fix this, You can close issue from my point of view, just for Your information I am exploiting this as I need this automatic enumeration for json rpc with enumerations without bolder plate defining enum by enum by all enumerations glz::enuemration()

stephenberry commented 1 month ago

@anders-wind, thanks for reporting this. This is a problem with ADL lookup. I was experimenting making this more broadly useful, but this shows the problem with this approach. I'll remove the nameof approach and build out a more flexible glz::meta approach where lists of values and keys can be provided.

anders-wind commented 1 month ago

Thanks for the swift response! We'll look forward to the new approach 👍

stephenberry commented 1 month ago

@anders-wind, as of v3.4.0 the glz::nameof function has been removed along with the enum_macro.hpp header. The new approach for enums, which allows a keys field to be added to glz::meta is now supported. I expect this approach to mature more in the future. But, this should solve your collision issue with the nameof library.

anders-wind commented 1 month ago

Amazing! Ill try it out when it gets added to vcpkg main 🙌

arturbac commented 1 month ago

How to upgrade this code ?

namespace glz::detail
  {

template<simple_enum::bounded_enum enumeration_type>
struct from_json<enumeration_type>
  {
  template<auto Opts>
    requires simple_enum::bounded_enum<enumeration_type>
  static void op(enumeration_type & arg, is_context auto && ctx, auto &&... args)
    {
    std::string_view value;
    read<json>::op<Opts>(value, ctx, args...);

    if(bool(ctx.error)) [[unlikely]]
      return;

    cxx23::expected<enumeration_type, simple_enum::enum_cast_error> res{simple_enum::enum_cast<enumeration_type>(value)
    };
    if(!res.has_value()) [[unlikely]]
      {
      ctx.error = error_code::syntax_error;
      return;
      }
    arg = res.value();
    }
  };

template<simple_enum::bounded_enum enumeration_type>
struct to_json<enumeration_type>
  {
  template<auto Opts>
    requires simple_enum::bounded_enum<enumeration_type>
  static void op(enumeration_type const & arg, auto &&... args) noexcept
    {
    std::string_view value{simple_enum::enum_name(arg)};
    write<json>::op<Opts>(value, args...);
    }
  };
  }  // namespace glz::detail

I am having errors like

In file included from simple_enum/glaze_json_enum_name.hpp:146:8: error: explicit specialization of undeclared template struct 'from_json'
stephenberry commented 1 month ago

from_json changes to from<JSON and to_json changes to to<JSON

arturbac commented 1 month ago

Can I detect this change with preprocessor conditional ?

stephenberry commented 1 month ago

My bad, I didn't have a plan to add feature test macros for every breaking change, but I see the tremendous benefit. I will start adding them. I will release a new version this week that will have one for this change, so you can hold off on implementing support for v3.5

arturbac commented 1 month ago

Yes, thank you. With feature macro I can make some code conditional and resolve any breaking change and maintain backward compatibility. I am not going to support c++ modules till they will be matured in compilers and cmake so for next few years at least, so feature macro definition is perfect for simple_enum.

stephenberry commented 1 month ago

Sounds good. I think my approach is going to add a new feature macro for each new breaking change. If a macro is superseded, then I will remove that macro so the feature doesn't trigger and replace it with a new macro for the new feature.

stephenberry commented 4 weeks ago

@arturbac, v3.6.0 adds feature test macros for changes in v3.5.0 and v3.6.0

You will be interested in:

// v3.5.0 change glz::detail::to_json and glz::detail::from_json specializations
// to glz::detail::to<JSON and glz::detail::from<JSON
// The template specialization take in a uint32_t Format as the first template parameter
#define glaze_v3_5_0_to_from

In glaze/core/feature_test.hpp