improvements to templated streaming #76
Conversation
|
Thanks @reicheratwork! Changes look mostly fine. I'm currently in the process of getting API tests to run. Unfortunately not all of them pass yet. There's a memory leak and such. If you don't mind, I want to get that work in first, and then rebase this PR on that. If you're curious, the work is being done here: https://github.com/k0ekk0ek/cyclonedds-cxx/tree/port-ddscxx-tests. |
|
Thanks @reicheratwork. I'm mostly fine with the logic, but there are some comments. Also, I generally dislike PRs that contain commits with fixes that introduce mistakes made in the same PR. Please consolidate those. I'll have another read once the initial comments have been processed. |
- set travis' cyclone build branch to master, as this contains the version of idlc that is used by idlcxx - fix for streaming functions in inherit specs missing streamer parameter - fix for getter functions returning references to complex types, as described in C++ language mapping v1.4 section 6.14 - fix for not generating a constructor if there are no members in a struct, as this will conflict with the "default" constructor Signed-off-by: Martijn Reicher <martijn.reicher@adlinktech.com>
reicheratwork
force-pushed
the
templated_streaming_improved
branch
2 times, most recently
from
9775edd
to
a612187
Compare
There was a problem hiding this comment.
I think we're almost there. Couple of small remarks. If you fix those, I'll be happy to merge these changes. Thanks @reicheratwork!
Since some typedefs might map onto the same C++ types, which causes the compiler to have difficulty distinguishing, there are now streaming functions generated for each typedef in the C++ namespace where it is defined The stream setup and cleanup has been moved to their own functions, to reduce verbosity in the code Signed-off-by: Martijn Reicher <martijn.reicher@adlinktech.com>
reicheratwork
force-pushed
the
templated_streaming_improved
branch
2 times, most recently
from
64bc474
to
6c5f556
Compare
- This causes issues in the default constructors and getters/setters of members with array declarators as well as typedefs generated "skipping" the array part Signed-off-by: Martijn Reicher <martijn.reicher@adlinktech.com>
- removed sequence and array template streamers, these will be unrolled in the generated code, not the template streamer functions, the maximum length checks will also be done there - reset default template classes and includes to the STL versions - added more descriptive exceptions in streamers Signed-off-by: Martijn Reicher <martijn.reicher@adlinktech.com>
- added getter functions for the different accessor strings which
can be used IDL_PRINTA, avoiding heap allocation of accessor strings
- arrays are unrolled using simple auto ranged for loops
- sequences are unrolled in two different manners:
- for bounded sequences they create a dummy instance where the
contents of the stream are read into, and based on the entry
number in the sequence it will be added to the bounded sequence
- for unbounded sequences the sequence is resized at the beginning
of the read cycle to the number of entities indicated by the
stream, and reads are done on the entity at the index in the stream
Signed-off-by: Martijn Reicher <martijn.reicher@adlinktech.com>
- added check for presence of members to compare - added comparison of inherited types in structs - if there is nothing to compare, "true" is returned as default Signed-off-by: Martijn Reicher <martijn.reicher@adlinktech.com>
- first comparison between discriminators - if discriminators match, comparison between values for the branch - if there is no branch which matches the discriminator, return false Signed-off-by: Martijn Reicher <martijn.reicher@adlinktech.com>
reicheratwork
force-pushed
the
templated_streaming_improved
branch
from
6c5f556
to
faf8c9d
Compare
|
Regarding bounded sequences, like we discussed, we should reject the entire sample and not fill it as far as allowed, but let's worry about that in a follow-up PR. One minor thing you forgot to fix, other than that I'll happily accept the changes. |
|
Follow items are listed on #83. Some tests are known to fail, but fixing those was not part of this PR. |
Improvements for the following: