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

Usability/Feature questions: WebAssembly, C++20 concept type-traits, shorten macro for field-only definitions? #49

Closed
RalphSteinhagen opened this issue Apr 13, 2021 · 3 comments
Assignees
Labels
question Further information is requested

Comments

@RalphSteinhagen
Copy link
Contributor

@veselink1 first: Thanks for making refl-cpp a nice open-source header-only library! 👍
Sorry for the long post, which covers more than one -- albeit overlapping -- aspects.

We are evaluating porting our existing serialiser implementation (YaS - Yet-another-Serialiser) to a more modern C++20 standard using refl-cpp. Having compared several other alternatives, I find your library very neat, efficient, and in the spirit of the upcoming C++(23?) static reflection functionality. Even better: your library works as-is and with minimal requirement already now. Kudos for that! 👍 ⭐ ⭐ ⭐ 👍

While exploring the refl-cpp syntax, I gathered some usability and feature-type questions rather than issues. I was wondering whether you (or someone else reading this) would be willing to comment and/or advise on these.

I made a small example implementing a default streaming operator operator<< overload for reflection-annotated classes below to illustrate the MVP use-case which is very similar to our (or for that matter most other) serialisation use-cases.

My questions:

  1. Does refl-cpp support being transpiled to WebAssembly (WASM)?
    I tried to compile the simple serialiser example (which works nicely), but I am getting errors when transpiling to WASM. This probably linked to missing headers in the compiler explorer environment but I was wondering whether somebody else has some successes with this before diving deeper into this myself. Any help/experience related to this would be much appreciated!
  2. Is there already another way to detect/distinguish user-defined from default std-annotated types (ie. those controlled via REFL_NO_STD_SUPPORT), for example, via type-traits, concepts, or boolean function? If not -- while I found some workarounds (see below) -- would it be conceivable/useful to incorporate some of the aspects into refl-cpp or should this remain better in the user-code? My worry is that the list of possible std:: might get eventually very long and the workaround below is not very maintainable and probably better/less effort being incorporated at the location where these std:: annotations are defined. Just as a thought.
  3. We would be annotating nearly exclusively member fields and not functions - would it be possible to shorten the annotation macro to something like REFL_CUSTOM(type, fieldName1, fieldName2, ..., fieldNameN)? I tried my luck but am not good enough with macros (which seem to be unavoidable and absolutely justified in this case though). Or is this something we should implement ourselves? I found a nice solution using boost -- although that wouldn't probably a suitable dependency to be incorporated into refl-cpp.
  4. If you like and find the example below also useful for other users, shall we make a PR for this?

Any help/experience related to this would be much appreciated!

The example code -- also runnable on compiler-explorer:

//#define REFL_NO_STD_SUPPORT // Q2-alt: enable this to skip parsing std templates (notably std::string)
#include <https://raw.githubusercontent.com/veselink1/refl-cpp/master/include/refl.hpp>
#include <iostream>

namespace unique {
    template <typename, template <typename...> typename>
    struct is_instance_impl : public std::false_type {};

    template <template <typename...> typename U, typename...Ts>
    struct is_instance_impl<U<Ts...>, U> : public std::true_type {};
    template <typename T, template <typename ...> typename U>

    using is_instance = unique::is_instance_impl<std::decay_t<T>, U>;
}

template < class T >
constexpr bool isStdTemplate() {    
    return std::is_same<T, std::string>{}
    || unique::is_instance<T, std::basic_string>::value
    || unique::is_instance<T, std::basic_string_view>::value
    || unique::is_instance<T, std::tuple>::value
    || unique::is_instance<T, std::unique_ptr>::value
    || unique::is_instance<T, std::shared_ptr>::value
    || unique::is_instance<T, std::pair>::value
    || unique::is_instance<T, std::complex>::value;
}

template<class T>
concept ReflectableClass = refl::is_reflectable<T>() && std::is_class<T>::value /*&& !isStdTemplate<T>()*/; // Q2-alt: add perhaps trait to identify std templates

template<class T>
constexpr bool isReflectableClass() {
    return refl::is_reflectable<T>() && std::is_class<T>() /*&& !isStdTemplate<T>()*/;
}

// automatically generate/enable the '<<' stream operator for REFL_... annotated classes
template <ReflectableClass T>
constexpr std::ostream& operator<<(std::ostream& os, const T& value) {
    os << refl::reflect(value).name << "(";
    if constexpr (std::is_class<std::remove_reference_t<decltype(value)>>::value) {
        for_each(refl::reflect(value).members, [&](const auto member, const auto index) constexpr {
            if (index > 0) { // how to make this constexpr??
                os << ", ";
            }
            os << get_display_name(member) << '=' << member(value);
        });
        os << ')';
    }
    return os;
}

// Q3 - custom macros to shorten simple declarations with only member fields - Q: incorporate to refl-cpp? user-code?
// there must be something smarter?!?
// perhaps something like: https://stackoverflow.com/questions/29224493/wrap-each-element-in-variadic-macro-with-an-expression#answer-29225568
#define REFL_CUSTOM(typeName, ...) REFL_AUTO(type(typeName), __VA_ARGS__)
#define REFL_CUSTOM1(typeName, f1) REFL_AUTO(type(typeName), field(f1))
#define REFL_CUSTOM2(typeName, f1, f2) REFL_AUTO(type(typeName), field(f1), field(f2))
#define REFL_CUSTOM3(typeName, f1, f2, f3) REFL_AUTO(type(typeName), field(f1), field(f2), field(f3))
#define REFL_CUSTOM4(typeName, f1, f2, f3, f4) REFL_AUTO(type(typeName), field(f1), field(f2), field(f3), field(f4))
// [..]

// ############## test classes (user code) ###################################

struct OtherStruct {    
    const char* name = "test";
    int a; // default init value (int -> '0')
    int b{-1};
    std::string otherString = "Hello";
};

// following line is important for serialisation until 'reflexpr' is available (target: ~C++23)
//REFL_AUTO(type(OtherStruct), field(name), field(a), field(b), field(otherString))
//REFL_CUSTOM(OtherStruct, field(name), field(a), field(b), field(otherString))
REFL_CUSTOM4(OtherStruct, name, a, b, otherString) // much shorter than the 'official version below


struct UnknownClass {
    int unknownValue;
};

struct DomainObject {    
    float x;
    float y;
    OtherStruct innerClass;
};

// alt declaration option (more verbose)
REFL_TYPE(DomainObject)    
    REFL_FIELD(x)
    REFL_FIELD(y)
    REFL_FIELD(innerClass)
REFL_END

#include <list> // just needed for test-case
int main() {
    const auto obj = DomainObject{41, 42};
    std::cout << "toStream: " << obj << "\n";
    std::cout << "toStream: " << DomainObject{43, 44} << "\n\n";

    std::cout << std::boolalpha;
    std::cout << "isReflectableClass(DomainObject) = " << isReflectableClass<DomainObject>() << '\n';  // true, OK
    std::cout << "isReflectableClass(OtherStruct) = " << isReflectableClass<OtherStruct>() << '\n';    // true, OK
    std::cout << "isReflectableClass(int) = " << isReflectableClass<int>() << '\n';                    // false, OK -- skip non-classes
    std::cout << "isReflectableClass(UnknownClass) = " << isReflectableClass<UnknownClass>() << '\n';  // false, OK -- skip non-REFL_AUTO classes
    std::cout << "isReflectableClass(std::string) = " << isReflectableClass<std::string>() << "\n\n";  // true, not OK --Q-B) should skip, why is std::string being reflected?

    std::cout << "isStdTemplate(std::string) = " << isStdTemplate<std::string>() << '\n';        // true, OK - not implemented
    std::cout << "isStdTemplate(std::tuple) = " << isStdTemplate<std::tuple<int,int>>() << '\n'; // true, OK - not implemented
    std::cout << "isStdTemplate(std::list) = " << isStdTemplate<std::list<int>>() << '\n';       // false, OK - not implemented
}
@veselink1
Copy link
Owner

@RalphSteinhagen Thank you for enjoying using it!
I will try to answer as fully as possible, apologies if I miss some detail.

  1. I have never attempted to compile refl-cpp to WASM.
    Attempting to include any one of the following fails in Godbolt. (The includes are from refl-cpp.) https://godbolt.org/z/K597qx989
#include <cstring>
#include <array>
#include <utility> // std::move, std::forward
#include <optional>
#include <tuple>
#include <type_traits>
#include <ostream>
#include <sstream>
#include <iomanip> // std::quoted

I'm guessing there is some explanation for that, but unsure what it is.

  1. I understand why this might be concerning. I have no plans to include metadata for std::*. In fact, at this time, I have no plans to continue adding any other types in there, as this will be a breaking change and might lead to clashes with user-defined metadata. The general idea behind the provided ones was to allow for the simplest and most common of data structures to be inspected and printed using refl::runtime::debug. This includes exceptions (which I am regretting now), strings, tuples, pairs, smart pointers, complex numbers, as these have fairly standard representations. Coupled with the support for std:: containers in runtime::debug, this should allow users to provide debug messages about composite types (containing lists, maps, etc). This is the primary reason why these exist. Anything else should be provided externally. Of course, this does not mean that no additional types will be added in the future!

To answer the question, I am not sure what the utility of checking whether a type is from std:: or not is, but have you considered just checking the prefix of the type name? https://godbolt.org/z/EYd51Tb6E

template <typename T>
constexpr bool isStdType() {
    return get_name(refl::reflect<T>()).template substr<0, 5>() == "std::";
}

Needless to say, this only works for types that have metadata generated for them.

Also:
refl::trait::is_instance_of_v<std::basic_string, T> === unique::is_instance<T, std::basic_string>::value

  1. I believe this accomplishes what you're asking for: https://godbolt.org/z/f84carnfs
#define REFL_CUSTOM(TypeName, ...) \
    REFL_TYPE(TypeName) \
    REFL_DETAIL_FOR_EACH(REFL_DETAIL_EX_1_field, __VA_ARGS__) \
    REFL_END

Unfortunately, DETAIL_ macros are neither public nor stable. You could replicate the code behind these two (possibly via trial and error) in YaS.

About if (index > 0) { // how to make this constexpr??: Argument expressions cannot participate in core constant expressions, which is what the condition for if constexpr is. The compiler will optimise that away anyway, in constexpr lambdas.

The one you have used is not constexpr, despite the constexpr specifier, because of os << .
If you really want to, you can get a the index at compile time, with a little bit more difficulty. https://godbolt.org/z/K6fPhzY6h

constexpr auto members = refl::reflect<std::remove_reference_t<decltype(value)>>().members;
constexpr auto index = refl::trait::index_of_v<
    refl::trait::remove_qualifiers_t<decltype(member)>, // member is assigned to const auto (remove the const)
    refl::trait::remove_qualifiers_t<decltype(members)>>; // members is implicity const, because of constexpr
  1. The example code does not really present a solution to a problem, but is more in the realm of exploring what is possible. I don't think it will be directly useful to users, but thanks for the suggestion.

Hope you can get YaS working on top of refl-cpp!

@veselink1 veselink1 self-assigned this Apr 14, 2021
@veselink1 veselink1 added the question Further information is requested label Apr 14, 2021
@RalphSteinhagen
Copy link
Contributor Author

@veselink1 I am struck with awe! Thanks for your quick, generous and very thorough reply and suggestions*! 👍

  1. Regarding WASM: I do not expect any troubles since the header compiles fine with clang (one of the WASM work horses) and I did not see any obvious other reasons why not. This was more to explore, my guess is that the bug I saw is rather related to the specific compiler-explorer setup and that I have to do my own homework and do an actual proof of concept for myself. ;-)
    I will let you know if I find anything to the contrary but you could consider this sub-issue/question as closed for now.

  2. This includes exceptions (which I am regretting now), [..]

    I fully understand your reasoning behind this. We have both use-cases -- those where this additional info of the std type is
    quite helpful and those where we do not want to dive into the std containers because we need to handle them differently to our annotated types.

    have you considered just checking the prefix of the type name?

    Thanks for the hint and for pointing/reminding me of this simpler solution! I am usually hesitant regarding checking for literal class names (because of potentially breaking when refactoring) but for the 'std::*' context this should be pretty safe. 👍

  3. I believe this accomplishes what you're asking for [..]

    Yes, exactly! Thanks for helping out. As mentioned, I am not familiar enough with modern C++ macros but this does the trick. I also understand and respect your disclaimer ... no worries, we will fix it on our side in case it breaks. But for now, this is sufficient. 👍

  4. The example code does not really present a solution to a problem [..]

    OK. That's why I asked. I saw that you already implemented some other debug mechanism and found '<<' for our debugging style useful. Although, overloading the '<<' operator is also quite basic and can be easily rediscovered/implemented by others without having an explicit example. 'Document as much and only as much as absolutely needed' ... I think your docs/example style meets this criterion which is probably sufficient for most users.

Please feel free to close this issue since the above -- at least for me -- answered all the relevant aspects we initially need to move forward with our YaS. 👍

*P.S. Let me know if there is something I can help you with since you kindly helped me ...

@veselink1
Copy link
Owner

Glad I could help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants