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

Reflect: generic type identification #83

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JohnTheCoolingFan
Copy link

@JohnTheCoolingFan JohnTheCoolingFan commented Sep 19, 2024

Rendered: https:/JohnTheCoolingFan/bevy-rfcs/blob/reflect_generic_id/rfcs/83-reflect_generic_id.md

Make type identification in the Reflection system generic, allowing multiple identification data types to coexist.

@JohnTheCoolingFan JohnTheCoolingFan changed the title Add reflect_generic_id rfc Reflect: generic type identification Sep 19, 2024
@SkiFire13
Copy link

I don't think this will solve the problem. Even if you manage to get some TypeData for a type, if the TypeId does not match the TypeData will fail to downcast and call the given method on the concrete type. And if you use anything not based on TypeId (e.g. UUIDs) in there too, then it will likely be unsound, since you have no way to guarantee that if a type is changed (even transitively, e.g. one of its field types is changed) then its UUID is changed too.

@JohnTheCoolingFan
Copy link
Author

These requirements would probably be part of the trait implementation requirements. TypeInfo and TypeData sets could probably be hashed and used to compare, but also there are places where using only TypeId for identification is not going to work. For example, if deserializing a set of data, a user wants to have something similar to an enum, but the set of keys is dynamic and based on what types are registered in a registry used for the de/serialization operation. Custom key types would be handy, and restricting it to type names and/or paths isn't making it much better.
TypeRegistration already contains a TypeId and could be used for checks. And if TypeIds don't match but the registry keys do, that might mean that the registration is from another binary/compilation and a conversion could be attempted using FromReflect and dynamic structs: clone_dynamic to get a dynamic struct from source data, FromReflect to convert to concrete type with expected TypeId.

Apologies if this comes off a bit chaotic. I might have only a set of specific applications in mind and also maybe some misunderstandings about the current reflect implementation and uses/connections.

@JohnTheCoolingFan
Copy link
Author

TypeId is a sound identification type in one library, and this guarantee is provided by the rust compiler. But, when loading from another binary (plugin) or loading data from a serialized data set (scene), other identification types are needed. It may be possible to still keep these identifications sound, have checks. For example, by using some kind of footprint, that is encoding the properties of the reflected type. A stable type id that hashes even package versions, features and all of the compilation context would encode too much for the use case.
And it may be possible to create mappings from one id type to another. For example, to identify the type that is basically identical but in two binaries has a different typeid and potentially different data layout. If there is an identification type that works for signaling across these two binaries, a mapping could be made for each one. And when data inevitably has to go from one to the other, a serialized common representation of that data can be sent, then deserialized on the other end.

@SkiFire13
Copy link

TypeInfo and TypeData sets could probably be hashed and used to compare

You likely don't want to hash TypeDatas, since they don't have a canonical representation (e.g. the function pointers inside them might be duplicated in different codegen units, so two TypeDatas for the same type might actually compare different).

TypeInfo contains TypeId so you're going to have the same problems as using TypeId.

And if TypeIds don't match but the registry keys do, that might mean that the registration is from another binary/compilation and a conversion could be attempted using FromReflect and dynamic structs

This cannot be done automatically on-the-fly when a TypeData is accessed, since it might be returning borrowed data or mutating the data passed in, and those won't work if you pass a temporary object that just got converted.

It might work if you perform this conversion the moment you create some type, essentially canonicalizing that type, but then you're just performing deserialization using reflection and type paths should be enough for that.

It may be possible to still keep these identifications sound, have checks. For example, by using some kind of footprint, that is encoding the properties of the reflected type. A stable type id that hashes even package versions, features and all of the compilation context would encode too much for the use case.

My point is that for many usecases this cannot be just a possibility, it has to be a guarantee or you have unsoundness.


> Why should we *not* do this?

This introduces a concept of multiple type identifications which might have no connection between them. There would be no singular central type registry, but instead a number of registries, each with a different id type and for a different purpose.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is probably one of the biggest issues with this approach. If we end up going with a generic type identification system, we need a way to interop between them such that we can have a central registry.

If we don't have that, then the onus to use the correct registry/identification falls on the user.

@MrGVSV
Copy link
Member

MrGVSV commented Sep 24, 2024

I don't think this will solve the problem. Even if you manage to get some TypeData for a type, if the TypeId does not match the TypeData will fail to downcast and call the given method on the concrete type. And if you use anything not based on TypeId (e.g. UUIDs) in there too, then it will likely be unsound, since you have no way to guarantee that if a type is changed (even transitively, e.g. one of its field types is changed) then its UUID is changed too.

Personally, I'd like to know more about how we intend to support cross-binary types. Are we passing around raw pointers, utilizing the dynamic types, allowing specially-made custom dynamic types, serializing-deserializing, or something else? Depending on how this is done, "downcasting"1 should be possible through type data.

In any case, it would be good to identify these particular methods so we can properly assess whether a generic identification system truly covers those cases, or if another solution needs to be explored.

Footnotes

  1. I put "downcasting" in quotes because—depending on how things are done—downcasting might be to the dynamic type and not actually the represented type.

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

Successfully merging this pull request may close these issues.

3 participants