Add cs:type support #782
Add cs:type support #782
Conversation
|
|
||
| public static class MyTimeSpanExtensions | ||
| { | ||
| public static void Encode(ref IceRpc.Slice.SliceEncoder encoder, TimeSpan value) => |
There was a problem hiding this comment.
The proposal was to use EncodeMyTimeSpan and DecodeMyTimeSpan. We already use these EncodeXxx/DecodeXxx names for generated enums, and they have the advantage to avoid ambiguity, especially for Decode.
The ambiguity can be an issue not for the generated code but for hand-written code using these extension methods.
| @@ -16,9 +16,23 @@ module IceRpc::Tests::Slice | |||
| st: MyStruct, | |||
| } | |||
|
|
|||
| [cs:type("System.TimeSpan")] | |||
| struct MyTimeSpan | |||
There was a problem hiding this comment.
What's actually nice with the proposal is you can name this compact struct simply TimeSpan and use TimeSpan (IceRpc::TimeSpan or maybe define it in a sub-module) in your Slice definitions, and it's mapped to System.TimeSpan in C#. MyTimeSpan is fine for the test.
There was a problem hiding this comment.
Yeah I think it's fine; the MyXXX is practically standard for the tests at this point.
There was a problem hiding this comment.
Looks good to me!
We check struct_ref.definition().has_attribute("cs:type", false) alot, so if you want to, you could add a helper method for this in slicec_ext/typeref_ext. Something like
is_custom_type(struct_ref: &TypeRef<Struct>) { ... }
Maybe that's not worth it though. Up to you.
| @@ -16,9 +16,23 @@ module IceRpc::Tests::Slice | |||
| st: MyStruct, | |||
| } | |||
|
|
|||
| [cs:type("System.TimeSpan")] | |||
| struct MyTimeSpan | |||
There was a problem hiding this comment.
Yeah I think it's fine; the MyXXX is practically standard for the tests at this point.
Co-authored-by: Austin Henriksen <austin.r.henriksen79@gmail.com>
Co-authored-by: Austin Henriksen <austin.r.henriksen79@gmail.com>
Co-authored-by: Austin Henriksen <austin.r.henriksen79@gmail.com>
This PR adds support for
cs:typeattribute, see #674Currently it doesn't work with optional or tagged types, I will add that in a follow up PR.