[PFR] Some questions

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PFR] Some questions

Boost - Dev mailing list
This is not (yet?) a review, although I guess this could be counted as a
partial review towards the current state of the docs; but after reading
them I have several questions.

1. Why is the term "reflection" used at all?  As far as I am aware, this
is primarily used to refer to accessing member names from the structure,
which is not something that this library provides at all, so at best
this seems highly misleading.  The original "magic get" name seems more
appropriate since it is primarily about extending tuple-get to basic
structures without boilerplate macros.

2. An up-front clarification on the limitations of supported structures
would be nice.  "Aggregate initialisable" is not a concept that everyone
is familiar with.

3. What is the motivation for "flat" reflection to exist, at all?  I
can't find any explanation of why one might want to do it; other than
completely disregarding type safety, which seems like a bad thing.  (I
assume there is some reason that I'm not aware of, but that's why an
explanation would be nice.)

4. Flat reflection is stated to be non-portable, raising further
questions as to why it exists at all.

5. Many of the intro pages talk about "disabling loophole" with no
explanation of what that is.  The configuration macros page finally
presents a link that doesn't really explain anything anyway, other than
suggesting it is a Dark Magic that was intended to be banned but nobody
had gotten around to it yet.

6. Speaking of the configuration macros page, it doesn't indicate what
values are the defaults, other than it "auto-detects your compiler".  I
assume from the surrounding text that it would prefer to use C++17 and
would use "loophole" (whatever that is) otherwise, but it would be good
to make that (or whatever it actually does instead) explicit.


Granted #3 can't get you into *too* much trouble with the limitation on
only supporting aggregate-initialised types... but on the other hand,
type hierarchies are still significant for aggregates (it can be
important to distinguish a "handle" from a plain int, or a Boost.Units
value from another with different unit).  And it feels like you're doing
C++ wrong if you're using aggregate types much; they're only a little
better than PODs.

(In all existing codebases I use, there are almost no aggregate types,
although there are a few almost-aggregates that have simple initialising
constructors, for example, or make member fields private and use a
get-set method pattern "just in case".  I imagine this is likely to be
true of most real-world codebases.)

Having said that, I can see some value in aggregate types as DTOs (for
database/json/etc translation) and for reducing usage of std::pair and
std::tuple, which is a good thing, though only if used in limited scope.
  But that usage doesn't explain "flat" either; the type hierarchy still
should be important.

Precise reflection, on the other hand, seems more potentially useful,
save for the unfortunate -- though understandable -- limitation on only
aggregates.  Having said that, I've personally never found a use-case
for a tuple-like get interface for anything, so perhaps I'm just not the
target audience for this library.

(I also have a strong dislike for aggregate initialisation being
order-based in the first place; I would have preferred something like
C99's named initialisation.  C++20 is adding something that they're
calling that, but is utterly useless and crippled instead of doing it
properly.)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [PFR] Some questions

Boost - Dev mailing list
Gavin Lambert wrote:

> Having said that, I've personally never found a use-case for a tuple-like
> get interface for anything, so perhaps I'm just not the target audience
> for this library.

You are surely familiar with the two classic examples, automatically
generating a hash function by memberwise iteration, and automatically
generating a serialization/deserialization function, by same.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [PFR] Some questions

Boost - Dev mailing list
On 5/10/2020 14:05, Peter Dimov wrote:
>> Having said that, I've personally never found a use-case for a
>> tuple-like get interface for anything, so perhaps I'm just not the
>> target audience for this library.
>
> You are surely familiar with the two classic examples, automatically
> generating a hash function by memberwise iteration, and automatically
> generating a serialization/deserialization function, by same.

Hash function might be a valid purpose, but I often find that there are
specific members that you want to exclude from equality/hash for various
reasons.  And hash function implementation should be tightly coupled to
the structure definition anyway, so there's less reason to abstract it away.

I don't consider this library suitable for (de)serialization purposes
since it provides no access to the member name.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [PFR] Some questions

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
pon., 5 paź 2020 o 02:30 Gavin Lambert via Boost <[hidden email]>
napisał(a):

> This is not (yet?) a review, although I guess this could be counted as a
> partial review towards the current state of the docs; but after reading
> them I have several questions.
>
> 1. Why is the term "reflection" used at all?  As far as I am aware, this
> is primarily used to refer to accessing member names from the structure,
> which is not something that this library provides at all, so at best
> this seems highly misleading.  The original "magic get" name seems more
> appropriate since it is primarily about extending tuple-get to basic
> structures without boilerplate macros.
>
> 2. An up-front clarification on the limitations of supported structures
> would be nice.  "Aggregate initialisable" is not a concept that everyone
> is familiar with.
>
> 3. What is the motivation for "flat" reflection to exist, at all?  I
> can't find any explanation of why one might want to do it; other than
> completely disregarding type safety, which seems like a bad thing.  (I
> assume there is some reason that I'm not aware of, but that's why an
> explanation would be nice.)
>
> 4. Flat reflection is stated to be non-portable, raising further
> questions as to why it exists at all.
>
> 5. Many of the intro pages talk about "disabling loophole" with no
> explanation of what that is.  The configuration macros page finally
> presents a link that doesn't really explain anything anyway, other than
> suggesting it is a Dark Magic that was intended to be banned but nobody
> had gotten around to it yet.
>
> 6. Speaking of the configuration macros page, it doesn't indicate what
> values are the defaults, other than it "auto-detects your compiler".  I
> assume from the surrounding text that it would prefer to use C++17 and
> would use "loophole" (whatever that is) otherwise, but it would be good
> to make that (or whatever it actually does instead) explicit.
>
>
> Granted #3 can't get you into *too* much trouble with the limitation on
> only supporting aggregate-initialised types... but on the other hand,
> type hierarchies are still significant for aggregates (it can be
> important to distinguish a "handle" from a plain int, or a Boost.Units
> value from another with different unit).  And it feels like you're doing
> C++ wrong if you're using aggregate types much; they're only a little
> better than PODs.
>
> (In all existing codebases I use, there are almost no aggregate types,
> although there are a few almost-aggregates that have simple initialising
> constructors, for example, or make member fields private and use a
> get-set method pattern "just in case".  I imagine this is likely to be
> true of most real-world codebases.)
>
> Having said that, I can see some value in aggregate types as DTOs (for
> database/json/etc translation) and for reducing usage of std::pair and
> std::tuple, which is a good thing, though only if used in limited scope.
>   But that usage doesn't explain "flat" either; the type hierarchy still
> should be important.
>
> Precise reflection, on the other hand, seems more potentially useful,
> save for the unfortunate -- though understandable -- limitation on only
> aggregates.  Having said that, I've personally never found a use-case
> for a tuple-like get interface for anything, so perhaps I'm just not the
> target audience for this library.
>
> (I also have a strong dislike for aggregate initialisation being
> order-based in the first place; I would have preferred something like
> C99's named initialisation.  C++20 is adding something that they're
> calling that, but is utterly useless and crippled instead of doing it
> properly.)
>

I think that the library serves a useful purpose -- admittedly, a small one
-- but it fails to document it properly.
The normal work flow would be something like the following. I am using a
library similar to Boost.Spirit or a clever DB access library. They all
need a user-provided tuple type or a type with tuple-like interface. So,
basically, I could use `std::tuple<std::string, int, double>`. But if I do
that, the individual types mean nothing, because there is no way to give
them names. At this point I decide I will use a Simple Aggregate (the
library should define this concept):

```
struct Record {
  std::string firstName;
  std::string lastName;
  int age;
  double salary;
};
```

Its only purpose is to be a tuple with named individual fields. But now I
have to provide index-based access. I could either specialize the
tuple_size and the other type traits, or I can use the PFR library which
does it for me.

The library should really document the above flow as a motivating use case.
Then everyone would be clear about its scope.

I agree that the library should not use the term "reflection", I agree that
"magic get" better reflected what it is doing, and I also do not see a use
case for flat reflection. I do however see a use case like this I have
three similar records:

```
struct Person {
  std::string firstName;
  std::string lastName;
  int age;
};

struct Employee : Person {
  double salary;
};

struct Prisoner: Person {
  int cellNumber;
};
```

And I would like to get index-based access to all of these. I understand
that PFR will not be able to provide it magically, but if there was a way
to define a tuple_size and tuple_get for them with as few declarations as
possible, that might have been of help.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [PFR] Some questions

Boost - Dev mailing list
pon., 5 paź 2020 o 09:50 Andrzej Krzemienski <[hidden email]>
napisał(a):

>
>
> pon., 5 paź 2020 o 02:30 Gavin Lambert via Boost <[hidden email]>
> napisał(a):
>
>> This is not (yet?) a review, although I guess this could be counted as a
>> partial review towards the current state of the docs; but after reading
>> them I have several questions.
>>
>> 1. Why is the term "reflection" used at all?  As far as I am aware, this
>> is primarily used to refer to accessing member names from the structure,
>> which is not something that this library provides at all, so at best
>> this seems highly misleading.  The original "magic get" name seems more
>> appropriate since it is primarily about extending tuple-get to basic
>> structures without boilerplate macros.
>>
>> 2. An up-front clarification on the limitations of supported structures
>> would be nice.  "Aggregate initialisable" is not a concept that everyone
>> is familiar with.
>>
>> 3. What is the motivation for "flat" reflection to exist, at all?  I
>> can't find any explanation of why one might want to do it; other than
>> completely disregarding type safety, which seems like a bad thing.  (I
>> assume there is some reason that I'm not aware of, but that's why an
>> explanation would be nice.)
>>
>> 4. Flat reflection is stated to be non-portable, raising further
>> questions as to why it exists at all.
>>
>> 5. Many of the intro pages talk about "disabling loophole" with no
>> explanation of what that is.  The configuration macros page finally
>> presents a link that doesn't really explain anything anyway, other than
>> suggesting it is a Dark Magic that was intended to be banned but nobody
>> had gotten around to it yet.
>>
>> 6. Speaking of the configuration macros page, it doesn't indicate what
>> values are the defaults, other than it "auto-detects your compiler".  I
>> assume from the surrounding text that it would prefer to use C++17 and
>> would use "loophole" (whatever that is) otherwise, but it would be good
>> to make that (or whatever it actually does instead) explicit.
>>
>>
>> Granted #3 can't get you into *too* much trouble with the limitation on
>> only supporting aggregate-initialised types... but on the other hand,
>> type hierarchies are still significant for aggregates (it can be
>> important to distinguish a "handle" from a plain int, or a Boost.Units
>> value from another with different unit).  And it feels like you're doing
>> C++ wrong if you're using aggregate types much; they're only a little
>> better than PODs.
>>
>> (In all existing codebases I use, there are almost no aggregate types,
>> although there are a few almost-aggregates that have simple initialising
>> constructors, for example, or make member fields private and use a
>> get-set method pattern "just in case".  I imagine this is likely to be
>> true of most real-world codebases.)
>>
>> Having said that, I can see some value in aggregate types as DTOs (for
>> database/json/etc translation) and for reducing usage of std::pair and
>> std::tuple, which is a good thing, though only if used in limited scope.
>>   But that usage doesn't explain "flat" either; the type hierarchy still
>> should be important.
>>
>> Precise reflection, on the other hand, seems more potentially useful,
>> save for the unfortunate -- though understandable -- limitation on only
>> aggregates.  Having said that, I've personally never found a use-case
>> for a tuple-like get interface for anything, so perhaps I'm just not the
>> target audience for this library.
>>
>> (I also have a strong dislike for aggregate initialisation being
>> order-based in the first place; I would have preferred something like
>> C99's named initialisation.  C++20 is adding something that they're
>> calling that, but is utterly useless and crippled instead of doing it
>> properly.)
>>
>
> I think that the library serves a useful purpose -- admittedly, a small
> one -- but it fails to document it properly.
> The normal work flow would be something like the following. I am using a
> library similar to Boost.Spirit or a clever DB access library. They all
> need a user-provided tuple type or a type with tuple-like interface. So,
> basically, I could use `std::tuple<std::string, int, double>`. But if I do
> that, the individual types mean nothing, because there is no way to give
> them names. At this point I decide I will use a Simple Aggregate (the
> library should define this concept):
>
> ```
> struct Record {
>   std::string firstName;
>   std::string lastName;
>   int age;
>   double salary;
> };
> ```
>
> Its only purpose is to be a tuple with named individual fields. But now I
> have to provide index-based access. I could either specialize the
> tuple_size and the other type traits, or I can use the PFR library which
> does it for me.
>
> The library should really document the above flow as a motivating use
> case. Then everyone would be clear about its scope.
>
> I agree that the library should not use the term "reflection", I agree
> that "magic get" better reflected what it is doing, and I also do not see a
> use case for flat reflection. I do however see a use case like this I have
> three similar records:
>
> ```
> struct Person {
>   std::string firstName;
>   std::string lastName;
>   int age;
> };
>
> struct Employee : Person {
>   double salary;
> };
>
> struct Prisoner: Person {
>   int cellNumber;
> };
> ```
>
> And I would like to get index-based access to all of these. I understand
> that PFR will not be able to provide it magically, but if there was a way
> to define a tuple_size and tuple_get for them with as few declarations as
> possible, that might have been of help.
>
> Regards,
> &rzej;
>

Having said that, I now actually see how the "flat" part can address my use
case. I need to define my types like this:

```
struct Person {
  std::string firstName;
  std::string lastName;
  int age;
};

struct Employee {
  Person person;
  double salary;
};

struct Prisoner {
  Person person
  int cellNumber;
};
```

This library is great (at what it is doing). It just does not demonstrate
it clear enough in the documentation.

I guess the key to understanding and appreciating it is to understand the
flow:
1. First, I have a library (Spirit, "Clever SQL") that requires structures
with index-based access.
2. Second, I define aggregates *only* for the purpose of working with this
library.
3. PFR helps me with plugging these aggregates into the library.

IOW, you appreciate the PFR library when you create aggregates as an
intermediate representation of data from libraries like Boost.Spirit.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [PFR] Some questions

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
пн, 5 окт. 2020 г. в 03:30, Gavin Lambert via Boost <[hidden email]>:

>
> This is not (yet?) a review, although I guess this could be counted as a
> partial review towards the current state of the docs; but after reading
> them I have several questions.
>
> 1. Why is the term "reflection" used at all?  As far as I am aware, this
> is primarily used to refer to accessing member names from the structure,
> which is not something that this library provides at all, so at best
> this seems highly misleading.  The original "magic get" name seems more
> appropriate since it is primarily about extending tuple-get to basic
> structures without boilerplate macros.

Any name is bad:
* magic get - gives no clue on what the library does
* reflection - according to wiki "reflection is the ability of a
process to examine the type or properties of an object, and modify its
own structure and behavior.". PFR library gives you the ability to do
that, but not on such a great level as some Java/C# users are used to.
* "tuple representation" - that's quite good, but I'd like to give a
hint to users, that the library does simple reflection

Here comes the plan:
* s/reflection/tuple representation/g
* add a few sentences in the Motivation section and Readme to clarify
that we are talking about very simple reflection cases

> 2. An up-front clarification on the limitations of supported structures
> would be nice.  "Aggregate initialisable" is not a concept that everyone
> is familiar with.

I'll add an example.


> 3. What is the motivation for "flat" reflection to exist, at all?  I
> can't find any explanation of why one might want to do it; other than
> completely disregarding type safety, which seems like a bad thing.  (I
> assume there is some reason that I'm not aware of, but that's why an
> explanation would be nice.)

It's quite useful for some use cases (hashing).


> 4. Flat reflection is stated to be non-portable, raising further
> questions as to why it exists at all.
>
> 5. Many of the intro pages talk about "disabling loophole" with no
> explanation of what that is.  The configuration macros page finally
> presents a link that doesn't really explain anything anyway, other than
> suggesting it is a Dark Magic that was intended to be banned but nobody
> had gotten around to it yet.

I'll try to improve the docs.


> 6. Speaking of the configuration macros page, it doesn't indicate what
> values are the defaults, other than it "auto-detects your compiler".  I
> assume from the surrounding text that it would prefer to use C++17 and
> would use "loophole" (whatever that is) otherwise, but it would be good
> to make that (or whatever it actually does instead) explicit.

Defaults change depending on the compiler minor/major version, C++
standard version, standard library implementation and version.

I'll add a short description on prefered implementations.

> Granted #3 can't get you into *too* much trouble with the limitation on
> only supporting aggregate-initialised types... but on the other hand,
> type hierarchies are still significant for aggregates (it can be
> important to distinguish a "handle" from a plain int, or a Boost.Units
> value from another with different unit).  And it feels like you're doing
> C++ wrong if you're using aggregate types much; they're only a little
> better than PODs.
>
> (In all existing codebases I use, there are almost no aggregate types,
> although there are a few almost-aggregates that have simple initialising
> constructors, for example, or make member fields private and use a
> get-set method pattern "just in case".  I imagine this is likely to be
> true of most real-world codebases.)

Yep, unfortunately C++ provokes you to write such code. That's a bad
practice that could be avoided with PFR, see "Motivation and Examples"
at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2141r0.html

Probably I should add that motivation from the paper to the PFR docs.


> Having said that, I can see some value in aggregate types as DTOs (for
> database/json/etc translation) and for reducing usage of std::pair and
> std::tuple, which is a good thing, though only if used in limited scope.
>   But that usage doesn't explain "flat" either; the type hierarchy still
> should be important.
>
> Precise reflection, on the other hand, seems more potentially useful,
> save for the unfortunate -- though understandable -- limitation on only
> aggregates.  Having said that, I've personally never found a use-case
> for a tuple-like get interface for anything, so perhaps I'm just not the
> target audience for this library.
>
> (I also have a strong dislike for aggregate initialisation being
> order-based in the first place; I would have preferred something like
> C99's named initialisation.  C++20 is adding something that they're
> calling that, but is utterly useless and crippled instead of doing it
> properly.)

I'll try to update docs in a few hours to address the above issues

--
Best regards,
Antony Polukhin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [PFR] Some questions

Boost - Dev mailing list
пн, 5 окт. 2020 г. в 16:53, Antony Polukhin <[hidden email]>:
<...>
> I'll try to update docs in a few hours to address the above issues

Done. Also fixed a bunch of broken links

--
Best regards,
Antony Polukhin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost