icerpc / icerpc-csharp

A C# RPC framework built for QUIC, with bidirectional streaming, first-class async/await, and Protobuf support.
https://docs.icerpc.dev
Apache License 2.0
102 stars 13 forks source link

Remove `TypeContext` #3890

Closed InsertCreativityHere closed 7 months ago

InsertCreativityHere commented 7 months ago

This PR Removes the need for ‘TypeContext’ from slicec-cs. It’s purely refactoring; there should be no changes to the logic. We used this enum for 2 purposes:

Purpose 1

cs_type_string would change the mapping of sequences and dictionaries based on this field.
 Now, instead of 1 function which matches 3 possible TypeContexts, we have 3 separate functions: cs_type_string(… TypeContext::Field) -> field_type_string
 cs_type_string(… TypeContext::IncomingParam) -> incoming_type_string 
cs_type_string(… TypeContext::OutgoingParam) -> outgoing_type_string

incoming_type_string and outgoing_type_string just delegate to field_type_string, for types other than seuqence or dictionary.

Purpose 2


Various parameter-code used it to tell whether we were on the dispatch side or not.

Now, we just pass a bool named is_dispatch which we already had in some functions anyways. IncomingParam -> false, OutgoingParam -> true Usually these matches matched TypeContext::Field to unreachable!. This change lets us remove all these dead branches.

Additionally, in encoding.rs we used TypeContext to tell whether we generated for fields or parameters. Instead of TypeContext, these functions now also take a bool: is_outgoing_param OutgoingParam -> true and Field -> false. Because this is all encoding code, we never encounter IncomingParam.

externl commented 7 months ago

I prefer to keep an enum for this distinction between incoming and outgoing, using a bool is just too confusing when reading the code.

InsertCreativityHere commented 7 months ago

Closed in favor of #3893