search

zeroc-ice / ice

All-in-one solution for creating networked applications with RPC, pub/sub, server deployment, and more.
https://zeroc.com
GNU General Public License v2.0
2.04k stars 592 forks source link

More Optional Class Cleanup #2262

Closed InsertCreativityHere closed 3 months ago

InsertCreativityHere commented 4 months ago

This PR removes the public API for encoding/decoding classes from all the languages that still have it. And changes our logic to throw an exception if we try to skip an optional class. Note that it is still possible for users to send/receive optional classes if they use the lower-level stream API's, but we can only do so much here. A sufficiently motivated user will always find a way to send these.

It also brings all the tests back into alignment with one another, and removes the last place where we were manually sending/checking for optional classes using the stream APIs.

The error messages that were added to C++ and C# were different, so I standardized on cannot skip optional class. This seemed the most consistent with our other error messages in the marshaling/unmarshaling logic: cannot deserialize object, cannot unmarshal a proxy without a communicator, invalid Object slice, empty indirection table, etc. Feel free to comment alternatives.

InsertCreativityHere commented 3 months ago

I believe you forgot to update the InputStream C# code to remove nullable.

Which function are you talking about specifically?

For the main function that I touched: internal override void readValue(System.Action<Value?> cb) I believe this is correct. The callback itself isn't nullable, but the value it receives is nullable because the decoding may of failed.

bernardnormier commented 3 months ago

For the main function that I touched: internal override void readValue(System.Action<Value?> cb) I believe this is correct. The callback itself isn't nullable, but the value it receives is nullable because the decoding may of failed.

My mistake, the code is correct.