matus-chochlik / std_cpp_refl

Proposal to add static reflection to C++
14 stars 2 forks source link

Reflection of non-public members #7

Open matus-chochlik opened 8 years ago

matus-chochlik commented 8 years ago

Let's have the following class:

class C {
private:
    int a, b;
public:
    char c;
    double d;
protected:
    float e, f;
};

class D : public C {
    void foo(void);
};

using MetaC = reflexpr(C);

what we agreeed upon?:

A) For every kind of scope member (X), let's have three operations returning a metaobject sequence:

get_X<MetaClass>; // only public members
get_all_X<MetaClass>; // all members including the private and protected
get_accessible_X<MetaClass>; // all members accessible from the point of invocation.

For example:

void D::foo(void) {
    using MC = reflexpr(C);
    using MoS1 = get_data_members_m<MC>;
    using MoS2 = get_accessible_data_members_m<MC>;
    using MoS3 = get_all_data_members_m<MC>;

    assert(get_size_v<MoS1> == 2); // {c, d}
    assert(get_size_v<MoS2> == 4); // {c, d, e, f}
    assert(get_size_v<MoS3> == 6); // {a, b, c, d, e, f}
}

or

Á) variant of the above:

get_X<MetaClass>; // all members including the private and protected
get_accessible_X<MetaClass>; // all members accessible from the point of invocation.
get_public_X<MetaClass>; // only public members

For example:

void D::foo(void) {
    using MC = reflexpr(C);
    using MoS1 = get_data_members_m<MC>;
    using MoS2 = get_accessible_data_members_m<MC>;
    using MoS3 = get_public_data_members_m<MC>;

    assert(get_size_v<MoS1> == 6); // {a, b, c, d, e, f}
    assert(get_size_v<MoS2> == 4); // {c, d, e, f}
    assert(get_size_v<MoS3> == 2); // {c, d}
}

other options for the record:

B) Basic getter operation returning a metaobject sequence exposing only the public members + expose operations: expose_nonpublic and expose_accessible.

For example:

void D::foo(void) {
    using MC = reflexpr(C);
    using MoS = get_data_members_m<MC>;

    assert(get_size_v<MoS> == 2); // {c, d}
    assert(get_size_v<expose_accessible_m<MoS>> == 4); // {c, d, e, f}
    assert(get_size_v<expose_nonpublic_m<MoS>> == 6); // {a, b, c, d, e, f}
}

Pros: more versatile, separates the operation of obtaining the metaobject sequence from exposing the non-public parts.

Cons: makes code review and reasoning about whether encapsulation is broken harder.

C) Basic getter operation returning a metaobject sequence exposing all members + hide operations: hide_nonpublic and hide_inaccessible:

For example:

void D::foo(void) {
    using MC = reflexpr(C);
    using MoS = get_data_members_m<MC>;

    assert(get_size_v<MoS> == 6); // {a, b, c, d, e, f}
    assert(get_size_v<hide_inaccessible_m<MoS>> == 4); // {c, d, e, f}
    assert(get_size_v<hide_nonpublic_m<MoS>> == 2); // {c, d}
}

D) parametrized getter operations:

For example:

void D::foo(void) {
    using MC = reflexpr(C);
    using MoS1 = get_data_members_m<MC, meta::public_only>;
    using MoS2 = get_data_members_m<MC, meta::accessible>;
    using MoS3 = get_data_members_m<MC, meta::all>;

    assert(get_size_v<MoS1> == 2); // {c, d}
    assert(get_size_v<MoS2> == 4); // {c, d, e, f}
    assert(get_size_v<MoS3> == 6); // {a, b, c, d, e, f}
}
ricardofandrade commented 8 years ago

I'm disagree with the idea of offering three different levels of accessibility which do not match in name or behavior with C++'s regular accessibility rules. So none of the alternatives are what I think how reflection should work or how the operations should be called. Here is a long rationale behind my opinion, so please bear with me...

1 - ) If determining the accessibility from the point of invocation were a limiting factor: A & A') get_data_members should return be called get_public_data_members and to complement it also get_protected_data_members/get_private_data_members which could be combined by the user of the language to provide get_all_data_members if needed - more on that on the bottom. B) get_data_members remains returning only public members, but there should be expose_protected_members, expose_private_members which could be combined by the user of the language to create expose_all_members if needed. C) get_data_members remains, but there should be hide_private, hide_protected. D) If reflexpr can work with public/protected/private specifiers, I'd prefer to use them instead of a tag type of enum value. D') get_data_members remains, then pass reflexpr with one of public/protected/private. Along with the ObjectSequence, between one and three arguments should be accepted in any order to obtain the desired access level.

This way, if the responsibility of getting the member access right is all on the user of the language (remember, assuming reflection cannot determine the access by itself), at least naming the operations consistently and following the know rules for each access specifier makes the user's life easier and the behavior more predictable.

NOTE: if for one the idea of giving the option to access to private members without accessing the protected ones may sound absurd, I'd say that's a small price to pay in name of the clarity (you know what to expect) and consistency with the language (no made up names). There are use cases to support this granularity too, briefly mentioned later on.

In the context of "when I call this reflection doesn't know exactly what is the level of access I have" kind of thing, my opinion would be that: A & A') It's clear in what each operation does. However, three operations (with the possibility of a fourth if "all" is considered) seems a little excessive. B) Accessing only public members it's generally what one wants, that's the default which is good. I'd say that expose_* it's way more clear about intentions and I'd argue that it's much more searchable than get_all/get_<access specifier>. Besides, in my experience is very rare to see the word "expose" in code. Github seems to agree with me: only 135 occurrences in all available C++ code bases. C) I'd be against giving nuclear weapons by default if there are other options. D & D') It's less searchable than the others because one must look for both get_data_members and the access specifiers to find a violation of the access rules.

Unless the implementation of B) is problematic in terms of performance, it would be my choice. Otherwise, D & D') has the potential to offer the best performance since the access level is given. If searching for it is a concern, then A).

2 - ) If determining the accessibility from the point of invocation is possible and it doesn't requires too many computational resources: For all alternatives, get_data_members should remain and it must follow the languages access rules. A & A') If the choice is to not offer a lot of options, get_all_data_members should exist as get_inaccessible_data_members which could be combined by the user of the language to create get_all_data_members if needed - more on that on the bottom. B) Same as above. expose_all_members or in this case, expose_inaccessible_members would have a result be equivalent of a get_all_data_members. C & D') IMO they do not apply in this context. Unless you want to use them to filter out the members you don't want exposed. I'd argue that filtering should not be done by reflection. That belongs in the application code.

NOTE: operations to obtain which is the actual access level of each listed member should exist in order to support filtering by public/protect/private members in the context of accessible/inaccessible duality.

Based on the alternatives given originally, I'd assume that 2 - ) is feasible and the way the proposal is heading to. So, it's a question of A & A') vs B).

A & A') Pros:

A & A') Cons:

B) Pros:

B) Cons:

Here B) would also be my first option. If B) problems are too hard do solve, then A). However... Hopefully the 3x performance penalty of B) is not that bad and calling expose_* is just matter of "flipping a flag" on the ObjectSequence for the desired level of access. One way of eliminating the 3 calls is stealing the idea of D & D') Then there would be only expose_inaccessible_members taking protected, private, which eliminates the needs of a user-defined operation "all". That would be done without incurring the search-ability concerns of D & D') as mentioned above, "expose" is clear about intentions by itself. This design does allow some level of filtering but because of the regular accessibility rules at play it varies on the context. Real filtering by access specifier still needs to be done by the application. NOTE: public might also be desired as a valid option. For consistency only possibly but perhaps also to allow some kind of access to public members in a private inheritance situation (one wants access public members of the base of its own base class, which was been inherited privately). If these are valid arguments in favor of B) then it would need the same number of "calls" than A) to get all members (respectively: 2 = accessible, then expose all , 2 = accessible + inaccessible).

Why accessing "all" should be done by the user of the language: mainly because it should not be that easy. In addition to it, because searching for "inaccessible" or "expose" or even in the context of 1 - ) as "<public|private|protected>" is much easier than searching for "getall<data|type|constant|static|function|template|...>_members". When I say easier consider "strstr" is easier/simpler than "regex". The default access will be sufficient in most of the cases. If you need more, you should first decide how much more. As mentioned above, I'm aware of situations involving derived classes (or acting as one) where for both protected and private are needed. But completely outside of a class hierarchy, the intentions are much more unclear and probably what's wanted is reaching only the inaccessible members. Still, a careful decision regarding how much access is welcome (as in B) or not having the accessible members together in the same ObjectSequence is not a concern (a in A). I believe the performance penalty of keeping separate operations for accessible and inaccessible members (either A or B) should not be sufficiently relevant to invalidate this argument.

Finally, all things considered, how it would be my favorite design of the class members accessor: 1 - ) If determining the accessibility from the point of invocation is not feasible:


void D::foo(void) {
    using MC = reflexpr(C);
    using MoS = get_data_members_m<MC> ;
    assert(get_size_v<MoS> == 2); // {c, d} - context is not considered, only c and d are public.
    assert(get_size_v<expose_inaccessible_members_m<MoS, reflexpr(private)>> == 4); // {a, b, c, d} - exposed private a and b
    assert(get_size_v<expose_inaccessible_members_m<MoS, reflexpr(protected), reflexpr(private)>> == 6); // {a, b, c, d, e, f} - exposed all
}

2 - ) If determining the accessibility from the point of invocation is feasible:


void D::foo(void) {
    using MC = reflexpr(C);
    using MoS = get_data_members_m<MC> ;
    assert(get_size_v<MoS> == 2); // {c, d, e, f} - in the context of a publicly-derived class e and f are already accessible.
    assert(get_size_v<expose_inaccessible_members_m<MoS, reflexpr(private)>> == 6); // {a, b, c, d, e, f} - exposed inaccessible a and b
    assert(get_size_v<expose_inaccessible_members_m<MoS, reflexpr(protected), reflexpr(private)>> == 6); // {a, b, c, d, e, f} - all already accessible
}

And I will leave a question... should there be a restrict_accessible_members, the complement operation of expose_inaccessible_members?

Axel-Naumann commented 8 years ago

@ricardofandrade - I don't (yet?) have the time to read your whole post, but the order of data members is a critical bit of info. Think of a class with a private (priv1), then public (pub1), then private (priv2) member; we need to know the order (priv1, pub1, priv2), if reflection accessibility makes private members visible.

matus-chochlik commented 8 years ago

I agree that the order of the reflected class members must be maintained in the resulting metaobject sequence.

ricardofandrade commented 8 years ago

@karies Good point. I understand that all alternatives presented by @matus-chochlik as well my final suggestion of design would have such attribute. The other designs I considered in my post where the user would be responsible for implementing access to "all" members would not have such attribute and that's a sufficiently strong argument against them.

ricardofandrade commented 8 years ago

In the std proposal forum it has been discussed that for some companies have problems with the name "std::launder" (as for "money laundering"). I'm not sure if the same concerns would apply to "expose". If that ever comes to be a problem, list_inacessible_members (specially the work "inaccessible") would still be very searchable.