Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Variant upcasting #81

Closed
jgehring opened this issue Sep 6, 2017 · 15 comments
Closed

Variant upcasting #81

jgehring opened this issue Sep 6, 2017 · 15 comments

Comments

@jgehring
Copy link

jgehring commented Sep 6, 2017

Hi there! I'm trying to cast a variant hold a shared_ptr<Derived> of a derived class to a shared_ptr<Base> of its base class (i.e. upcasting). I've searched previous issues and it seems like others attempted this as well, e.g. in #56. Similar to #56, my use-case concerns object serialization: I would like to serialize and deserialize an object of Derived via a shared_ptr<Base>. Serialization is fine as I can easily obtain the most derived type of a given variant, but deserialization (obtaining shared_ptr<Base> from a reconstructed shared_ptr<Derived> RTTR variant) does not seem to work.

The documentation of variant::convert mentions that it supports "Conversion of raw pointers to its derived types, if a rttr_cast to the type described by target_type would succeed," but I'm wondering why there's no support for casting to base types.

@acki-m
Copy link
Contributor

acki-m commented Sep 6, 2017

Mhm...I think you have two options:

  1. You could extract the wrapped value and then convert it to the base class pointer and then convert it back to wrapped class, i.e. std::shared_ptr<T>

  2. Add a custom converter function, which does explicitly what you want.

Does this help?

@jgehring
Copy link
Author

jgehring commented Sep 6, 2017

Thanks for the quick reply! Option 2 is almost out of the question since the serializer should ideally not have to know about the types it will handle beforehand. Option 1 sounds good but the problem I'm seeing is how to retain ownership of the wrapped data. Based on local testing, it seems that the proposed series of conversions would yield a shared_ptr of the desired type (Base) but ownership would still be held by the original shared_ptr<Derived> instance. For example, if I extract the wrapped value of a shared_ptr<T> variant I obtain a T* variant, but if the shared_ptr<T> variant is destructed then the extracted variant becomes a dangling pointer. In the end, I have a shared_ptr<Base> that does not actually share ownership with the original shared_ptr<Derived> and will thus be invalid once the original variant gets out of scope. Any suggestions for workarounds would be welcome!

@acki-m
Copy link
Contributor

acki-m commented Sep 7, 2017

Why should the variant be destructed, when you still hold the original one, everything should be fine.
Tell me better, why do you want to convert to a base class anyway? When you have the most derived type, you also have access to all base class properties.

@jgehring
Copy link
Author

jgehring commented Sep 8, 2017

My use case is serialization and deserialization of a Derived object via a shared_ptr<Base>, e.g. something like

// Serialization via base class
std::shared_ptr<Base> obj = std::make_shared<Derived>();
serialize(ostream, obj);

// ...

// Deserialization via base class
std::shared_ptr<Base> obj;
deserialize(istream, obj);
// obj should now hold an instance of Derived

Regarding the above question of ownership, I can usually do casts like this:

auto dptr = std::make_shared<Derived>();
auto bptr = std::static_pointer_cast<Base>(dptr);
// bptr shares ownership with dptr so I can get rid of dptr
return bptr;

The same doesn't work when I work with variants and cast from shared_ptr<Derived> to Derived* to Base* to shared_ptr<Base>: the final shared pointer will not share ownership with the original one.

@acki-m
Copy link
Contributor

acki-m commented Sep 9, 2017

Okay, what I can read now from your last answer is, that you want to perform a downcast, not an upcast.
You have a shared_ptr<Base>, which is actually a shared_ptr<Derived>
And you serialization routines, get the base class, and you need to retrieve the most derived type, i.e. Derived. Is this correct?

Then take a look at my serialization code at:
https://github.com/rttrorg/rttr/blob/master/src/examples/json_serialization/to_json.cpp#L230
From an rttr::instance you can get the most derived type.

Does this help you?

@jgehring
Copy link
Author

The downcast during serialization is not an issue since, as you pointed out, rttr provides a method to determine the most derived type. However, during deserialization I'll start with the correct type (Derived) as I'm also serializing type information and will have to store the resulting value in a Base pointer.

The use-case is similar to the JSON serialization example that you linked to, but let's look at from_json.cpp#L179 instead. This wouldn't work if the serialized object was of type Derived but the instance passed to the function would be a shared_ptr<Base> (or the corresponding variant). In my case, I can reconstruct the Derived instance without issues from the data (i.e. as shared_ptr<Derived> with the default constructor policy) but I have no way to assign it to the instance that was passed to the function.

As another example, consider serializing a class with a shared_ptr<Base> member and multiple derived classes that are able to print their actual type via a virtual printType() method:

struct MyType {
  std::shared_ptr<Base> obj;
};

// Omitted: Code to register obj as a property of MyType

MyType foo1 = {std::make_shared<Derived1>();};
MyType foo2 = {std::make_shared<Derived2>();};
std::vector<MyType> foos = {foo1, foo2};
serialize(ostream, foos);

// ...
std::vector<MyType> bars;
deserialize(istream, &bars);
std::cout << bars.size() << std::endl; // 2
std::cout << bars[0].obj->printType() << std::endl; // "derived1"
std::cout << bars[1].obj->printType() << std::endl; // "derived2"

Again, the issue is not serialization but deserialization. I can create shared_ptr<Derived1> and shared_ptr<Derived2> from the data in the stream but I cannot assign it to the shared_ptr<Base> member variable.

@acki-m
Copy link
Contributor

acki-m commented Sep 11, 2017

This wouldn't work if the serialized object was of type Derived but the instance passed to the function would be a shared_ptr (or the corresponding variant).

Sure it would work.
Adjust the example to following snippet:

auto  c_1 = std::make_shared<circle>("Circle #1");
auto my_shape = std::static_pointer_cast<shape>(c_1);
....
json_string = io::to_json(my_shape); // serialize the circle to 'json_string'

Regarding the 2nd issue, this is indeed a problem. I have to think about it.

@jgehring
Copy link
Author

Sure it would work.
...
json_string = io::to_json(my_shape); // serialize the circle to 'json_string'

Yes, serialization isn't a problem. However, I won't be able to call from_json to deserialize the JSON string into a shared_ptr<shape>; I'd need to provide a shared_ptr<circle>. In my case, type information is also serialized so I can readily construct a shared_ptr<circle> and deserialize its properties -- I just can't assign it to a shared_ptr<shape> instance.

@acki-m
Copy link
Contributor

acki-m commented Sep 11, 2017

got it now....still have to think about a nice solution for it.

@jgehring
Copy link
Author

IMHO this would work well if it was possible to do convert<shared_ptr<shape>>() on a shared_ptr<circle> variant. However, I don't know rttr's internals too well so I can't judge whether this will break something or not.

@jgehring
Copy link
Author

@acki-m, any new plans on this?

@acki-m
Copy link
Contributor

acki-m commented Sep 19, 2017

I have something coded, it will basically add the conversion function, which you can already create by yourself, automatically. Is it urgent, do you use RTTR for some private project or work related?

@jgehring
Copy link
Author

Cool! It's not urgent (it's not blocking my current work) but would be nice to have eventually :) Would happy to use a workaround as well (e.g. a standalone conversion function).

@jgehring
Copy link
Author

jgehring commented Oct 2, 2017

Just wanted to check whether you've got plans to proceed on this one soon. Otherwise, I'm also happy to add a conversion function if that solves the upcasting issue.

@acki-m
Copy link
Contributor

acki-m commented Mar 13, 2018

It's done, and merged into master, I hope it's what you had in mind.

@acki-m acki-m closed this as completed Mar 13, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants