Replies: 13 comments 4 replies
-
Make it variants? Like this |
Beta Was this translation helpful? Give feedback.
-
Ok. I've found another solution, make
But in wiki you said that is not optimal solution because of memory allocations. Is there is way to get json_t from already defined |
Beta Was this translation helpful? Give feedback.
-
I'm going to chime in with my opinion on the matter but its really up to @stephenberry so Ill talk to him later about if and how we want to handle inheritance and polymorphism. Much like the standard library we tend to rely more on templates rather than polymorphism hence why we haven't addressed this until now. Note that it is quite likely we might not add support for this period because there are a lot of problems that have to be worked out. Inheritance is not currently supported but wouldn't be that hard to handle. Polymorphism on the other hand is a massive pain since glaze relies heavily on knowing the type it is dealing with so It would need to be able to do visitation which would require knowing the types. Luckily we don't need to list all the types in one place like in a variant and we can leverage either a compile time or runtime type registry. I lean towards compile time but there are benefits to a runtime registry. In my proposed implementation after the meta struct you would need to specify what classes it inherits from. If we want a compiletime registry there isn't really a good way of adding such a thing to glz::meta and the code to set it up is quite boilerplatey so unfortunately it would need to use macros. //Something like this
//B meta
GLZ_INHERITANCE(B, A);
//C meta
GLZ_INHERITANCE(C, A);
//D meta
GLZ_INHERITANCE(D, A); And for full support they would likely all need names added to the metas (static constexpr std::string_view name = "B";) Alternatively we could add the base class to the meta and then just register the type: //Something like this
template <>
struct glz::meta<B>
{
using base = A; // We could handle multiple inheritance by adding something like glz::type_list
static constexpr std::string_view name = "B";
static constexpr auto value = glz::object("c", [](auto && v) { return v.c(); });
};
GLZ_REGISTER(B); The benefit of this is that we could add support for handling any registered object type in a std::any automatically and would be helpful if we wanted to add support for stuff like generating language bindings. This approach also wouldn't require registration unless polymorphism was used. I lean towards this approach. I feel like this will also work better when we have static reflection. Assuming we can agree on an approach while its not supper simple this can probably be handled in under a week. We would then be able to automatically handle inheritance/polymorphism for registered types. Here is some code I was playing around with to test out compiletime type registration. https://godbolt.org/z/MhasjrWo4 The errors you saw are likely another issue in which there are various sections of code that assume things are default constructible and we really should get rid of those. Another thing to point out in the code you posted is that since the lamdas in the glz::object are returning values they would only support writing out to json and not reading in. I think @stephenberry was planning on adding setter/getter support since it was the most general way of supporting bitfields but its not a pattern we normally use. I'm surprised noone has run into this yet.
I'm sorry I don't quite understand. |
Beta Was this translation helpful? Give feedback.
-
I asked about - is there is way to obtain json_t (or smthing like that) from meta and not build it in function from scratch?
Yeah, I understood. At this moment I need to write jsons only, not read. So for me it's Ok. |
Beta Was this translation helpful? Give feedback.
-
Anyway, thank you for quick response! |
Beta Was this translation helpful? Give feedback.
-
No unfortunately not. Ill keep it in mind though. Ill try to add the inheritance/polymorphism stuff in soon provided I get approval. Hopefully that will address your use case. |
Beta Was this translation helpful? Give feedback.
-
We might also be able to handle this through having the user supply a read/write function in the base class that is called and takes in the iterators for the buffer and the context. The derived class can then call write/read with the correct type. I didn't originally think of this because it doesn't allow deducing the type when reading and it would have to read to whatever type is currently stored but we cant entirely support changing the underlying type unless its being stored in a std::unique_ptr or std::sharered_ptr anyways (And the user may not want to let us change the type). I still prefer registering the type since it meshes better with the rest of the lib and I can anticipate other use cases. The only problems with the type registration is that it requires rtri (We just got rid of all rtii usage in glaze) and writing might require a linear search since typeids are not constexpr we would need to build a runtime mapping if we didn't want to do a linear search. Still way more performant than relying on something like json_t. Reading would be fine though since we wouldn't need to depend on rtti. A polymorphic write function would be much faster...... Edit: |
Beta Was this translation helpful? Give feedback.
-
I dislike json_t but I think we should add a way of doing this since like you pointed out we have the structure information from the meta so as long as two things share a similar structure (And json_t should match any structure) we can generate the code to copy the data out. Something like "auto json = glz::convert<json_t>(*this);" So I opened up a new issue #179 to track that separately and we can handle inheritance here. Also, we should probably add pmr support to address some of the performance concerns in json_t. |
Beta Was this translation helpful? Give feedback.
-
I talked with @stephenberry and we don't want to rush into inheritance/polymorphism. We don't want to be adding major features right now. Especially since we don't tend to use allot of polymorphism ourselves. We will likely initially just go with your suggestion of creating a json_t from a meta type for now as a stop gap as it is generally useful and straightforward. There are concerns with build times when using a compile time type registry to build the inheritance hierarchy and we don't want to use rtti but need something similar if we want to handle deduction. Like a virtual function that returns a glaze type hash if rtii is turned off. And there are concerns about how this would work across dll boundaries if we did use rtii. The biggest concern is that other potential enhancements like better custom read/write might overlap with this issue so we just want to wait and see for now. @fregate, I dont like leaving people hanging but we are unlikely to improve support in this area in the short term
You could use the glz::raw_json (Undocumented) type to get around this the contained string will be treated as raw json and will not be output as a string. glaze/include/glaze/core/common.hpp Line 183 in 2052a5d Would likely be faster than dealing with json_t
Yeah a variant of pointers to your derived types should work but would require you to know all the potential types. Probably what I would have tried as a workaround due to the fact it should handle reading/writing but I understand its not ideal. |
Beta Was this translation helpful? Give feedback.
-
Ok. Thank you for response and new features to implement in future. I will try with Anyway, I made some changes to compile it with gcc-10 (for my embedded-like project). Is it interesting or you targeting only for new compilers and standards? |
Beta Was this translation helpful? Give feedback.
-
I don't think we were motivated enough to update glaze to support gcc10 but it sounds like you already did the heavy lifting. If you make a pull request or publish your fork on github (didnt see it) I'll review the changes and merge them provided everything looks good. I'll set up a ci build for gcc10 but I can't promise we won't drop support for it later if it's missing something we want to use. |
Beta Was this translation helpful? Give feedback.
-
No macro solution to the type registry problem https://godbolt.org/z/onYTjhf66. Or maybe something like this https://godbolt.org/z/d8s3cK1rE. I think I prefer using static_asserts like this this https://godbolt.org/z/ncvGxxc5Y This essentially allows us to do the same things QT does in its meta system with qRegisterMetaType but compiletime instead of runtime. This is one half of the things required to handle polymorphism cleanly. A constexpr typeid paired with some sort of runtime typeid is the second (Either through rtii, a polymorphic function call, or if there was some standard way of getting the vtable pointer I wouldn't even need that). |
Beta Was this translation helpful? Give feedback.
-
Hi! Some progress on this issue? And second question is there some internal representation of object (not templates) which can be write as json or beve? |
Beta Was this translation helpful? Give feedback.
-
Hi!
I'm very exited about performance for encoding of this application and want to ask about how to use it in case like
What is the correct way to do this inheritance thing? In fact I have abstract class-interface and got errors like:
I tried to output json string, but in result string I see string, not object (of course)
"payload":"{\"num\":1}"
- and this is not correct for me.In tests I can't find any related cases.
Beta Was this translation helpful? Give feedback.
All reactions