[POC] Annotated meta-data

[Innokentiy-Alaytsev]

The purpose of annotated meta-data is to provide better runtime-introspection abilities with less code. Annotations are just properties created with reusable function or functional object.

https://github.com/Innokentiy-Alaytsev/entt/tree/runtime-annotations

I've implemented one of the functions I think is useful. I've renamed the concept from 'decoration' to 'annotation' - I think the new word better represents the purpose. I've only implemented the function for registering meta-data without setter and getter. If you say everything is fine, I'll add implementations for annotated meta-data with setter and getter, annotated meta-type (reflect_annotated), maybe annotated meta-function (func_annotated). Also, I'll try to find a way to minimise the amount of the required code - it's very straight forward, but it may be a support burden.

[skypjack]

As a side note, I want to mention here another nice-to-have feature for the properties.

I'm often adding properties as:

.data<...>("*"_hs, std::make_pair(property::type, type::foobar));

That is, the pair-like approach is useful but it can be also annoying and pointless sometimes. Especially when you want to set a single property like:

.data<...>("*"_hs, property::foobar);

Then probe it for existence only:

if(my_type.prop(property::foobar)) { ... }

In this case, the value can easily be represented by an invalid meta_any object to maintain the same interface for meta_prop. However, if the property exists, the meta_prop converts to true and the guard is satisfied. The other way around would be:

if(auto p = my_type.prop(property::type); p && p.value<type>() == type::foobar) { ... }

In other terms, it would be useful a sort of key-only property model.

[Innokentiy-Alaytsev]

I've reworked the meta_factory::data function (without a getter/setter) to work with a mixed list of properties and annotations. Just like before, the annotation is a function returning a property. They are just no longer packed in std::tuple.

To distinguish between properties and annotations new functions are implemented. They are not great, because is_property function fails miserably in case of anything that is not property.

I've reworked static prop variable initialisation inside data() - now it is initialised right away, without first initialising it with a default constructed value. The main reason is that it's simpler to get the type from make_property() function result than deduce it. The make_property() function does approximately the same, as the old code with an additional check for properties and annotations and application of annotations.

Most of the other functions may be reworked in the same way that the data() was. The only difference for some of them is the absence of a name to pass to annotations. I don't think that it is a problem, because annotations on some things make little sense (e.g. constructors and destructors).

Another point: should I generalise the functions to accept not only entt::hashed_string, but any type, as long as it provides value() function, returning ENTT_ENTITY_ID? My first try was a failure because I've made some mistakes, but I think it should be possible.

[Innokentiy-Alaytsev]

FYI, I put my functions in front of the old ones to keep (quasi-) lexicographical ordering and for no other reasons. In my world lexicographical is the default ordering.

[skypjack]

Thank you very much for your effort! I'm integrating a new sorting algorithm at the moment, then I'll look into this so as to merge it somehow. Don't worry for the hashed string, I can easily rework it if all the other parts fit well. :+1:

[Innokentiy-Alaytsev]

I'll try to add support for annotations returning multiple properties at once. It should allow creating a kind of concepts, e.g. SerializedData which has properties name, min, max and json_type.

P.S. The code formatting is a bit ruined by me right now. I'll fix it.

[Innokentiy-Alaytsev]

I've implemented a bunch of property traits for checking whether a type if property, annotation (a callable returning a property) or annotation builder (a callable returning a tuple of properties). I've checked them with godbolt so far and it seems to work. Tomorrow I'll add some tests. After this, I'll rework my current implementation to use property traits and shall try to introduce support for annotation builders alongside properties and annotations as arguments for entt::meta_factory::data.

The property traits are created to work with entt::hased_string, but I think it should be a bit easier to rework them to work with arbitrary argument list for annotations now when they are in a separate file. I'll do it as soon as you say so.

[Innokentiy-Alaytsev]

As a side note, I want to mention here another nice-to-have feature for the properties...

I think it should be possible. I guess the argument type for this single property should still satisfy some requirements. But with property traits as long as it is not property, annotation or annotation builder it should be possible to take this single property and build a pair-based property from it to use the existing mechanics or some special kind of meta_prop.

[skypjack]

I think it's possible to avoid those traits with a bit of template magic and a couple of overloads put directly within the factory class.
Before to proceed, what about defining exactly the goal without caring for the code? To sum up and correct me if I'm wrong, please:

• Allow setting key-only properties.
• Allow to use functions that return a pair as properties (annotations).

Is it?

[Innokentiy-Alaytsev]

Allow setting key-only properties.

Apparently, yes.

Allow using functions that return a pair as properties (annotations).

Yes. With the extension for functions returning multiple properties. This should allow creating some kind of premade concepts within the application and further eliminate code duplication.

I think it's possible to avoid those traits with a bit of template magic and a couple of overloads put directly within the factory class.

The good things I see about traits are:

• They are reusable. However, I don't know if they are or will be required anywhere else.

• I think that code with traits and if constexpr or std::enable_if may be simpler than the one with some other kind of template magic. However, I am likely wrong, since I haven't seen the code for any of those variants.

Anyway, I shall try to implement my idea with traits and if you don't like it I'll humbly ask for points at the solution you'd prefer me to implement.

[Innokentiy-Alaytsev]

How should the types for which tuple_size > 2 be processed? Should they be treated as (full) properties, i.e. key-value pair, or as a list of flag properties, i.e. key-only properties? Obviously, get<0> and get<1> work for such types.

[skypjack]

The risk of supporting everything is to make it counterintuitive. I'd probably reject them.

[Innokentiy-Alaytsev]

Reject like in silently ignore or static_assert?

[Innokentiy-Alaytsev]

BTW, what about multi-annotations returning multiple properties? As I mentioned, they are useful for defining concepts. Currently, I expect multi-annotation to return a tuple of properties. As long as only key-value pairs are allowed to define properties everything is fine and dandy: a tuple of pairs is a list of properties, a tuple of not (only) pairs is not a property list meaning that the entity returning it is not a multi-annotation. But with the addition of key-only (flag?) properties, this logic will no longer be correct, because single values are allowed to define a property. A possible workaround is to only allow such property lists to be returned by multi-annotation and not allowed to be passed directly to the meta_factory::data function. This is a bit limiting, but should not be a great problem - any time the user wants to pass a list of properties (i.e. a tuple consisting of both key-value pairs and keys) it is possible to wrap this list into a reusable function having a meaningful name.

[skypjack]

I cannot imagine the final API nor a proper use cases to be honest. Can you help me in this sense?

[Innokentiy-Alaytsev]

I see it this way:

auto name_annotation(entt::hashed_string const& i_name) {
    return std::make_pair ("name"_hs, std::string{i_name.data()});
}

auto json_serialized_annotation(entt::hashed_string const& i_name) {
    return std::make_tuple(
        name_annotation(i_name), // Name for JSON attribute
        "serialize_json"_hs);   // Flag (value-only) property
}

// Multi-annotation is useful for defining interdependent properties
struct int_value_range_annotation {
    int min = INT_MIN;
    int max = INT_MAX;

    auto operator()(entt::hashed_string const&) {
        return std::make_tuple(
            std::make_pair("min", min),
            std::make_pair("max", max));
    }
};

// Application-specific concept
auto randomized_attribute_buff_annotation (entt::hashed_string const& i_name) {
    return std::tuple_cat (
        json_serialized_annotation(i_name),         // < Reuse annotations
        int_value_range_annotation{10, 30}(i_name), // <-/
        std::make_tuple(                      // Additional properties, not
            std::make_pair("duration"_hs, 5), // defined by other annotations.
            "debuffable"_hs));
}

entt::reflect<blessing>("blessing"_hs)
    .data<&blessing::health_buff>("health_buff",
          &randomized_attribute_buff_annotation,           // Use concept
          std::make_pair("heals_crippled_limbs"_hs, true), // Add k-v property
          "dispellable"_hs)                                // Add flag property
    .data<&blessing::speed_buff>("speed_buff",
          &randomized_attribute_buff_annotation,
          std::make_tuple(  // << static_assert here, because property lists are not allowed in this context
              std::make_pair("fixes_broken_shoes"_hs, false), 
              "nondispellable"_hs));

[Innokentiy-Alaytsev]

I did not implement anything for key-only properties yet. Their detection should be simple: they are not key-value properties and are not property lists. I don't think that tuple-valued key-only properties are useful. If they are needed then their concepts most likely can be named which means they should be named types.

[Innokentiy-Alaytsev]

I've managed to implement my idea with annotations, returning multiple properties. For the sake of simplicity, annotations are required to return a tuple of properties, even if there is only one property to return. Without this limitation, it would be impossible to distinguish between key-only properties and key-value properties.

Passing a key-only property will result in static assertion because this feature is not supported yet. There is another static assertion in case something is terribly wrong and the property is neither key-value nor key-only.

Property traits are still there and are used in the implementation. They are greatly simplified by if constexpr and the idea of annotations returning tuples of properties.

What would be the best way of implementing key-only properties? Setting the property key is straight forward, but what about the value?

[Innokentiy-Alaytsev]

Updated sketch of API use:

auto name_annotation(entt::hashed_string const& i_name) {
    return std::make_tuple(
        std::make_pair ("name"_hs, std::string{i_name.data()}));
}

auto json_serialized_annotation(entt::hashed_string const& i_name) {
    return std::make_tuple(
        name_annotation(i_name), // Name for JSON attribute
        "serialize_json"_hs);   // Flag (value-only) property
}

// Multi-annotation is useful for defining interdependent properties
struct int_value_range_annotation {
    int min = INT_MIN;
    int max = INT_MAX;

    auto operator()(entt::hashed_string const&) {
        return std::make_tuple(
            std::make_pair("min", min),
            std::make_pair("max", max));
    }
};

// Application-specific concept
auto randomized_attribute_buff_annotation (entt::hashed_string const& i_name) {
    return std::tuple_cat (
        json_serialized_annotation(i_name),         // < Reuse annotations
        int_value_range_annotation{10, 30}(i_name), // <-/
        std::make_tuple(                      // Additional properties, not
            std::make_pair("duration"_hs, 5), // defined by other annotations.
            "debuffable"_hs));
}

entt::reflect<blessing>("blessing"_hs)
    .data<&blessing::health_buff>("health_buff",
          &randomized_attribute_buff_annotation,           // Use concept
          std::make_pair("heals_crippled_limbs"_hs, true), // Add k-v property
          "dispellable"_hs)                                // Add flag property
    .data<&blessing::speed_buff>("speed_buff",
          &randomized_attribute_buff_annotation,
          std::make_tuple(  // << static_assert here, because property lists are not allowed in this context
              std::make_pair("fixes_broken_shoes"_hs, false), 
              "nondispellable"_hs));

static_assert is not implemented yet. Firstly, because I've forgotten about it. Secondly, Because it seems like it should be possible to support this kind of use because with property traits it is possible to distinguish between singular properties and property lists. However, I'm not so sure that it is a good idea: the problem is property lists of two key-only properties, that would be identified as a key-value property. For this reason, I think it will be better to just strictly prohibit passing property lists.

[Innokentiy-Alaytsev]

Static assertion on tuples with size other than 2 lives. Just like before all my gibberish about annotations pairs are treated as key-value properties.

[Innokentiy-Alaytsev]

Meanwhile, nested annotations are also possible. In the example above, instead of

auto randomized_attribute_buff_annotation (entt::hashed_string const& i_name) {
    return std::tuple_cat (
        json_serialized_annotation(i_name),
        int_value_range_annotation{10, 30}(i_name),
        std::make_tuple(
            std::make_pair("duration"_hs, 5),
            "debuffable"_hs));
}

it should be possible to do

auto randomized_attribute_buff_annotation (entt::hashed_string const& i_name) {
    return std::make_tuple ( // No need for tuple_cat, just make tuple
        &json_serialized_annotation,        // < Pass annotation directly, without invoking it
        int_value_range_annotation{10, 30}, // <-/
        std::make_pair("duration"_hs, 5),
        "debuffable"_hs);
}

Implementing this requires a small adjustment to the is_property_list trait, allowing elements of the list to be annotations and not just properties.

[Innokentiy-Alaytsev]

if(auto p = my_type.prop(property::type); p && p.value<type>() == type::foobar) { ... }

I'm not sure about this variant, TBH. The problem I see is that one may want to set hashed_string-based property, like "serialized"_hs it may be a bit inconvenient to query the presence of the arbitrary property. With hashed_string in the example above it would only be possible to have one property of this type which may be a bit restrictive.

if(my_type.prop(property::foobar)) { ... }

This version seems to be better than the previous one. The only thing bothering me is using an invalid meta_any as a value of such property. I understand why this should be this way and even agree with it. It's just my constant fear of someone misusing the thing.

[Innokentiy-Alaytsev]

Key-only properties (only for data, as before) live now. They are implemented as having an invalid meta_any now. How do you think, would it be meaningful to have true as a value?

[skypjack]

May I ask you to create a branch in your fork where you squash all the changes or a PR here to use as inspiration for this POC? I'll close it when done but this way I can have all the diffs at once without troubles.

[Innokentiy-Alaytsev]

@skypjack I squashed implementation into a single commit, rebased it onto master and placed it in separate branch. Would you like me to create PR for convenience?

[skypjack]

That's fine this way. Thank you very much. :+1:

[skypjack]

So, the first review of the meta part is over. Let's try to close also this one now. Right now, these are the types of properties supported:

• Key only.
• Key/Value.
• std::get<1>()-table.

First of all, we could add a fourth option: invocable properties (as in std::is_invocable). This type of property is invoked and we use the result as an actual property. The result must be one of the options listed above or another invocable properties.

With these bricks available, we can finally introduce composable properties and therefore annotations. The idea is that of splitting the std::get<1>()-table property in two parts: std::pair based properties and std::tuple. The former is a property, nothing less and nothing more. The latter will be treated as a list of properties. It can contains all the ones above mentioned as well as another tuple if needed. This way, we can compose properties the way we prefer.

What do you think about? Does it sound like a good compromise? The nice part of it is that it would fit nicely with the current implementation and it will get the job.

[Innokentiy-Alaytsev]

🤔

[Innokentiy-Alaytsev]

A thought from the top of my head, I didn't try it or something: if the property is invocable (without arguments, I suppose) then it should be possible to take the result and use std::apply() to pass the unpacked result into the props function. Of course, the invocable property should return a list of properties in the form of std::tuple for this to work.

The "not so fun" task is separating flies from cutlets, i.e. std::tuple from std::pair. I had some insane property traits for this to work in the original POC implementation. The problem at that time was that I worked with the idea that a two-element tuple is a property too. If this limitation is relaxed and only pairs can be used to define separate properties then it boils down to a check like std::is_same_v<std::pair<decltype(std::get<0>(property)), decltype(std::get<1>(property))>, property >

[skypjack]

if the property is invocable (without arguments, I suppose) then it should be possible to take the result and use std::apply() to pass the unpacked result into the props function.

This is incredibly smart actually.

The "not so fun" task is separating flies from cutlets, i.e. std::tuple from std::pair.

Let me amaze you without using any trait class. :wink:

[Innokentiy-Alaytsev]

Have something that kinda works. I'll add tests to see how broken is it in reality.

[skypjack]

I've some changes to push. I hope they won't break your stuff.

[Innokentiy-Alaytsev]

Don't worry, I'll fix everything. Also, as long as prop and unpack are not drastically changed I've nothing to worry about)

[Innokentiy-Alaytsev]

I've looked at what you've done. After I make everything work properly with my version I'll push it the way it is for you to watch and then port it to your version.

[skypjack]

Ok, I must leave now. I've just pushed also the support for invocable properties. Afaik all we discussed in this issue is now supported but let me know if I'm missing something. We have annotations and we can compose them, we have key-only properties and also support for tuple. What else?

[Innokentiy-Alaytsev]

I'm sorry to say this but there is a bug in the current implementation of the prop. Trying to attach to properties for which both key and value have the same type results in duplicate assertion fail due to both properties using the same local static variables.

There is some failure in the latest wip build, I'll look into it.

[Innokentiy-Alaytsev]

Sorry, the failure in the wip build was some error in results interpretation by some of the GitHub internals - everything is fine.

[Innokentiy-Alaytsev]

C'mon, GH, show corrent information. The failure is there, I've just looked at the wrong green light.

[skypjack]

I see. Apparently clang interprets the code differently from GCC/MSVC. I thinks it's a bug of the older clang because locally (latest) I've no problems.

Let me know if you find the source of the problem. Otherwise I'll try to reproduce it later and to write a workaround that works also with clang 6. Fortunately we are in the wip branch. :wink:

[Innokentiy-Alaytsev]

The problem is with Clang only, yes. I'm installig it right now.

The problem with duplicate properties is supposedly everywhere. I've only tested MSVC right now but will also test Clang.

[skypjack]

The problem with duplicate properties is supposedly everywhere.

This is trivial though. Don't worry. The other one seems more annoying.

[Innokentiy-Alaytsev]

I've pushed my version of property tuples and annotations a.k.a. invocable properties to a new branch. It is not based on your latest commits because you've drastically changed unpack and implemented essentially the same feature.

[skypjack]

At a first glance, in your implementation you've the problem that a callable that returns a tuple that contains a callable doesn't work as expected, because unpack treats the second callable as a property rather than as something to invoke. Am I wrong?

[Innokentiy-Alaytsev]

I have the following annotations in tests and they seem to work.

auto nested_annotation() {
    return std::pair{"nested"_hs, 123};
}

auto nesting_annotation() {
    return
        std::tuple{
        std::pair{"nesting"_hs, true},
        &nested_annotation};
}

Unpack checks if the received object is an std::pair before deciding whether to further it is a key-value property or a list of properties to pass to the props function.

[Innokentiy-Alaytsev]

I've "fixed" the tests so that they now run properly:

1. Commented out the test assertions for those properties that are not added as a workaround for the duplicated key-value types pairs problem.

2. Fixed the lambda-annotation so that it does not require entt::hashed_string any more.

3. Changed the type of the name property value - using char[] that is the type of the "i" caused error from within meta. It was something about instance being nullptr when it should not.

[Innokentiy-Alaytsev]

I have a crazy idea about how to fix the duplicate problem:

1. Add some kind of parameter pack as a tag to the function that stores the property and the node.

2. Check if the property and node were initialised by the current invocation. I think it should be possible. If thee were initialised - return.

3. If the current invocation did not initialise property and node then perform a recursive call to this very function with all the same arguments but with the tag parameter pack expanded by one new value/type.

The idea is to build a list of functions with parameter packs of different length.

[Innokentiy-Alaytsev]

The idea is great but doesn't work because things are not exactly constexpr.

[Innokentiy-Alaytsev]

I came up with the following horrible solution:

    template<typename Key, typename... Value>
    void unpack(char, Key &&pkey, Value &&... pvalue) {
        gogogo (
            std::make_index_sequence<25>{},
            std::forward<Key>(pkey),
            std::forward<Value>(pvalue)...);
    }

    template<
        typename Key, typename... Value, std::size_t TagHead, std::size_t... TagTail>
    void gogogo(
        std::index_sequence<TagHead, TagTail...>, Key &&pkey, Value &&... pvalue) {
        static auto property{
            std::make_tuple(
                std::forward<Key>(pkey),
                std::forward<Value>(pvalue)...)};
        static bool node_used = false;

        static internal::meta_prop_node node{
            *curr,
            []() -> meta_any {
                return std::get<0>(property);
            },
            []() -> meta_any {
                if constexpr(sizeof...(Value) == 0) {
                    return {};
                } else {
                    return std::get<1>(property);
                }
            }
        };

        if (node_used) {
            gogogo (
                std::index_sequence<TagTail...>{},
                std::forward<Key>(pkey),
                std::forward<Value>(pvalue)...);
        }
        else {
            ENTT_ASSERT(!duplicate(node.key(), *curr));
            node_used = true;
            *curr = &node;
        }
    }

    template<typename Key, typename... Value>
    void gogogo(
        std::index_sequence<>, Key &&pkey, Value &&... pvalue) {
        static auto property{
            std::make_tuple(
                std::forward<Key>(pkey),
                std::forward<Value>(pvalue)...)};

        static internal::meta_prop_node node{
            *curr,
            []() -> meta_any {
                return std::get<0>(property);
            },
            []() -> meta_any {
                if constexpr(sizeof...(Value) == 0) {
                    return {};
                } else {
                    return std::get<1>(property);
                }
            }
        };

        ENTT_ASSERT(!duplicate(node.key(), *curr));
        *curr = &node;
    }

It has everything we love: needless recursion, code bloat and code duplication.

[skypjack]

Ok, so... I like it, uh... Especially the index sequence having length 95, you know... So... What about deleting the branch that contains it? :smile:

---

Put the jokes aside, to maintain a library like this one we must be able to say - stop. I wouldn't introduce fancy code that is hard to maintain only to face corner cases that are unlikely to happen in 95% of cases.

So, what are the feature that make the property part richer and without which we cannot live?

The current version on wip (once the couple of smalle issues are solved and it will be in a few hours probably) is really flexible imho and it allows to do property composition, that is a great idea to further reduce the amount of code required to reflect types.

[Innokentiy-Alaytsev]

First of all, it's not 95 - just 25)

This code above is nothing more than an attempt (obviously, a failed one) to fix the duplicate bug.

As for the actual implementation in the new-meta-annotations branch - it seems simpler than the one on wip to me. The only problem with it comes from the duplicate bug. However, if functionally they are the same then using the one from the wip is just fine with me.

[skypjack]

I'm from mobile at the moment, so forgive me if I've not read it right but it seems to me that it doesn't support tuples yet and it suffers from this problem of callable properties that return callable properties, because the detection is too early in the chain and the second callable isn't tested and is used directly as a non-invocable property. Am I wrong?