Hi Edgar,
thanks for posting this impressive (and unexpected) line of work! As far as I know, this is the first time that someone other than me touched the RPC template code of Genode. ;-)
I especially like the changes of the return-value handling. I was never quite satisfied with this part. Your implementation is much nicer.
Software patterns like RAII [3] are a possible reason for removing that limitation. Quite often classes cannot provide a useful default-constructor which does more than merely setting the object state to "invalid". Because often there are no useful default values. So if a class provides a default-constructor which just sets the object to "invalid", the code complexity increases since care has to be taken that the object is fed with data up to a specific point of time.
I agree.
Unfortunately, my implementation does not remove default-constructor calls completely:
- Capabilities still need default-constructors. Msgbuf stores them
directly in an array and I did not find the time to check changes of this storage behavior for possible side effects.
This is not a problem because a default-constructed 'Capability' has a well-defined 'invalid' state, which is expected (like for the boost::optional types you cited).
- Output-only _arguments_ have to be default-constructible too.
Please note, that non-default-constructible _return_ types are allowed. So this affects only RPC arguments which are explicitly declared as Output-only, for example by adding custom specializations of Rpc_direction. The reason for this limitation is that my Pod_tuple version still stores the RPC arguments directly. While input arguments are copy-constructed, it is hard to copy from an object that does not yet exist.
I doubt that this is a problem in practice. We are not using custom specializations of 'Rpc_direction' at all. If one needs an output-only argument, it should be transferred as return value, which can be a POD struct to carry multiple values. The only use case where an output-only argument would make sense is the delegation of a capability (which cannot by carried in a POD struct) in addition to a return value. But as 'Capability' has a default constructor, this is not an issue.
I successfully tested the code with the repositories "hello_tutorial" and "demo" on base-linux. But if you decide to use my code contribution, please note that it is rather a proof of concept. There is room for improvements:
- I tried to minimize the code modifications. So some refactoring may
be useful:
Thanks. That is good for the review.
I expanded Meta::Empty by constructors and a typedef,
I haven't yet wrapped my head around this change yet. Give me some time to let it sink in.
I inserted possibly confusing forward declarations, and Meta::Pod_tuples and Meta::Pod_args could be renamed because they do not only accept POD types [6].
I agree. With your changes, the (renamed) template should be moved outside of 'meta.h', preferably to 'base/rpc.h'.
- I completely ignored the trace-related code. I do not know if there
is a need of modifications.
Thanks to Johannes Schlatow who gave me the idea of grappling with this topic.
I'd love to hear the backstory about this someday. :-)
For integrating your work upstream, I have just opened the following issue:
https://github.com/genodelabs/genode/issues/2150
Thank you again for submitting your contribution!
Cheers Norman