Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove TypeContext #3890

Closed
wants to merge 7 commits into from
Closed

Remove TypeContext #3890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ee7e846
Create context-aware cs_type_string functions.
InsertCreativityHere Jan 25, 2024
68f4427
Replaced our old calls to 'cs_type_string'.
InsertCreativityHere Jan 25, 2024
8c3718c
Factored out adding the optional modifier.
InsertCreativityHere Jan 25, 2024
b0e56fb
Slightly improved formatting.
InsertCreativityHere Jan 25, 2024
44b7a98
Formatting.
InsertCreativityHere Jan 25, 2024
8ba80d3
Fix swapped conditional branches.
InsertCreativityHere Jan 25, 2024
4da2c6e
Fixed the names of some 'is_dispatch's.
InsertCreativityHere Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 22 additions & 34 deletions tools/slicec-cs/src/builders.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::slicec_ext::*;
use slicec::code_block::CodeBlock;
use slicec::grammar::{Class, Commentable, Encoding, Entity, Operation, *};
use slicec::supported_encodings::SupportedEncodings;
use slicec::utils::code_gen_util::TypeContext;

pub trait Builder {
fn build(&self) -> CodeBlock;
Expand Down Expand Up @@ -142,9 +141,7 @@ impl ContainerBuilder {

pub fn add_fields(&mut self, fields: &[&Field]) -> &mut Self {
for field in fields {
let type_string = field
.data_type()
.cs_type_string(&field.namespace(), TypeContext::Field, false);
let type_string = field.data_type().field_type_string(&field.namespace(), false);

self.add_field(
&field.field_name(),
Expand Down Expand Up @@ -321,7 +318,7 @@ impl FunctionBuilder {
self
}

pub fn add_operation_parameters(&mut self, operation: &Operation, context: TypeContext) -> &mut Self {
pub fn add_operation_parameters(&mut self, operation: &Operation, is_outgoing: bool) -> &mut Self {
let parameters = operation.parameters();

// Find an index such that all parameters after it are optional (but not streamed)
Expand All @@ -338,10 +335,10 @@ impl FunctionBuilder {
};

for (index, parameter) in parameters.iter().enumerate() {
let parameter_type = parameter.cs_type_string(&operation.namespace(), context, false);
let parameter_type = parameter.cs_type_string(&operation.namespace(), false, is_outgoing);
let parameter_name = parameter.parameter_name();

let default_value = if context == TypeContext::OutgoingParam && (index >= trailing_optional_parameters_index) {
let default_value = if is_outgoing && (index >= trailing_optional_parameters_index) {
match parameter.data_type.concrete_typeref() {
// Sequences of fixed-size numeric types are mapped to `ReadOnlyMemory<T>` and have to use
// 'default' as their default value. Other optional types are mapped to nullable types and
Expand All @@ -366,34 +363,26 @@ impl FunctionBuilder {
);
}

match context {
TypeContext::IncomingParam => {
self.add_parameter(
"IceRpc.Features.IFeatureCollection",
&escape_parameter_name(&parameters, "features"),
None,
Some("The dispatch features.".to_owned()),
);
}
TypeContext::OutgoingParam => {
self.add_parameter(
"IceRpc.Features.IFeatureCollection?",
&escape_parameter_name(&parameters, "features"),
Some("null"),
Some("The invocation features.".to_owned()),
);
}
_ => panic!("Unexpected context value"),
if is_outgoing {
self.add_parameter(
"IceRpc.Features.IFeatureCollection?",
&escape_parameter_name(&parameters, "features"),
Some("null"),
Some("The invocation features.".to_owned()),
);
} else {
self.add_parameter(
"IceRpc.Features.IFeatureCollection",
&escape_parameter_name(&parameters, "features"),
None,
Some("The dispatch features.".to_owned()),
);
}

self.add_parameter(
"global::System.Threading.CancellationToken",
&escape_parameter_name(&parameters, "cancellationToken"),
if context == TypeContext::OutgoingParam {
Some("default")
} else {
None
},
is_outgoing.then_some("default"),
Some("A cancellation token that receives the cancellation requests.".to_owned()),
);

Expand Down Expand Up @@ -443,10 +432,9 @@ impl FunctionBuilder {
// Generate documentation for "void" returns. This is only done for void operations
// since they can't have '@returns' tags in their doc comments.
if operation.return_type.is_empty() {
let comment = match context {
TypeContext::IncomingParam => "A value task that completes when this implementation completes.",
TypeContext::OutgoingParam => "A task that completes when the response is received.",
_ => unreachable!("Unexpected context value"),
let comment = match is_outgoing {
true => "A task that completes when the response is received.",
false => "A value task that completes when this implementation completes.",
};
self.add_comment("returns", comment);
}
Expand Down
41 changes: 17 additions & 24 deletions tools/slicec-cs/src/decoding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::slicec_ext::*;
use convert_case::Case;
use slicec::code_block::CodeBlock;
use slicec::grammar::*;
use slicec::utils::code_gen_util::{get_bit_sequence_size, TypeContext};
use slicec::utils::code_gen_util::get_bit_sequence_size;

/// Compute how many bits are needed to decode the provided members, and if more than 0 bits are needed,
/// this generates code that creates a new `BitSequenceReader` with the necessary capacity.
Expand Down Expand Up @@ -75,7 +75,7 @@ pub fn default_activator(encoding: Encoding) -> &'static str {
fn decode_member(member: &impl Member, namespace: &str, encoding: Encoding) -> CodeBlock {
let mut code = CodeBlock::default();
let data_type = member.data_type();
let type_string = data_type.cs_type_string(namespace, TypeContext::IncomingParam, true);
let type_string = data_type.incoming_type_string(namespace, true);

if data_type.is_optional {
match data_type.concrete_type() {
Expand Down Expand Up @@ -176,11 +176,12 @@ fn decode_dictionary(dictionary_ref: &TypeRef<Dictionary>, namespace: &str, enco
write!(
decode_value,
" as {}",
value_type.cs_type_string(namespace, TypeContext::Field, true),
// TODO change this to a cast, so we can pass `ignore_optional = false`.
value_type.field_type_string(namespace, true),
);
}

let dictionary_type = dictionary_ref.cs_type_string(namespace, TypeContext::IncomingParam, true);
let dictionary_type = dictionary_ref.incoming_type_string(namespace, true);
let decode_key = decode_key.indent();
let decode_value = decode_value.indent();

Expand Down Expand Up @@ -231,23 +232,19 @@ fn decode_sequence(sequence_ref: &TypeRef<Sequence>, namespace: &str, encoding:
if !has_cs_type_attribute && matches!(element_type.concrete_type(), Types::Sequence(_)) {
// For nested sequences we want to cast Foo[][] returned by DecodeSequence to IList<Foo>[]
// used in the request and response decode methods.
write!(
code,
"({}[])",
element_type.cs_type_string(namespace, TypeContext::Field, false),
);
write!(code, "({}[])", element_type.field_type_string(namespace, false));
};

if has_cs_type_attribute {
let sequence_type = sequence_ref.cs_type_string(namespace, TypeContext::IncomingParam, true);
let sequence_type = sequence_ref.incoming_type_string(namespace, true);

let arg: Option<String> = match element_type.concrete_type() {
Types::Primitive(primitive) if primitive.fixed_wire_size().is_some() && !element_type.is_optional => {
// We always read an array even when mapped to a collection, as it's expected to be
// faster than decoding the collection elements one by one.
Some(format!(
"decoder.DecodeSequence<{}>({})",
element_type.cs_type_string(namespace, TypeContext::IncomingParam, true),
element_type.incoming_type_string(namespace, true),
if matches!(primitive, Primitive::Bool) {
"checkElement: SliceDecoder.CheckBoolValue"
} else {
Expand All @@ -265,14 +262,14 @@ fn decode_sequence(sequence_ref: &TypeRef<Sequence>, namespace: &str, encoding:
if enum_def.is_unchecked {
Some(format!(
"decoder.DecodeSequence<{}>()",
element_type.cs_type_string(namespace, TypeContext::IncomingParam, true),
element_type.incoming_type_string(namespace, true),
))
} else {
Some(format!(
"\
decoder.DecodeSequence(
({enum_type_name} e) => _ = {underlying_extensions_class}.As{name}(({underlying_type})e))",
enum_type_name = element_type.cs_type_string(namespace, TypeContext::IncomingParam, false),
enum_type_name = element_type.incoming_type_string(namespace, false),
underlying_extensions_class = enum_def.escape_scoped_identifier_with_suffix(
&format!(
"{}Extensions",
Expand Down Expand Up @@ -332,7 +329,7 @@ decoder.DecodeSequenceOfOptionals(
write!(
code,
"decoder.DecodeSequence<{}>({})",
element_type.cs_type_string(namespace, TypeContext::IncomingParam, true),
element_type.incoming_type_string(namespace, true),
if matches!(primitive, Primitive::Bool) {
"checkElement: SliceDecoder.CheckBoolValue"
} else {
Expand All @@ -345,15 +342,15 @@ decoder.DecodeSequenceOfOptionals(
write!(
code,
"decoder.DecodeSequence<{}>()",
element_type.cs_type_string(namespace, TypeContext::IncomingParam, true),
element_type.incoming_type_string(namespace, true),
)
} else {
write!(
code,
"\
decoder.DecodeSequence(
({enum_type} e) => _ = {underlying_extensions_class}.As{name}(({underlying_type})e))",
enum_type = element_type.cs_type_string(namespace, TypeContext::IncomingParam, false),
enum_type = element_type.incoming_type_string(namespace, false),
underlying_extensions_class = enum_def.escape_scoped_identifier_with_suffix(
&format!(
"{}Extensions",
Expand Down Expand Up @@ -400,11 +397,7 @@ fn decode_result_field(type_ref: &TypeRef, namespace: &str, encoding: Encoding)

// TODO: it's lame to have to do this here. We should provide a better API.
if matches!(type_ref.concrete_type(), Types::Sequence(_) | Types::Dictionary(_)) {
write!(
decode_func,
" as {}",
type_ref.cs_type_string(namespace, TypeContext::Field, false),
);
write!(decode_func, " as {}", type_ref.field_type_string(namespace, false));
}

if type_ref.is_optional {
Expand All @@ -427,7 +420,7 @@ pub fn decode_func(type_ref: &TypeRef, namespace: &str, encoding: Encoding) -> C

fn decode_func_body(type_ref: &TypeRef, namespace: &str, encoding: Encoding) -> CodeBlock {
let mut code = CodeBlock::default();
let type_name = type_ref.cs_type_string(namespace, TypeContext::IncomingParam, true);
let type_name = type_ref.incoming_type_string(namespace, true);

// When we decode the type, we decode it as a non-optional.
// If the type is supposed to be optional, we cast it after decoding.
Expand Down Expand Up @@ -503,7 +496,7 @@ pub fn decode_operation(operation: &Operation, dispatch: bool) -> CodeBlock {
// For optional value types we have to use the full type as the compiler cannot
// disambiguate between null and the actual value type.
let param_type_string = match param_type.is_optional && param_type.is_value_type() {
true => param_type.cs_type_string(&namespace, TypeContext::IncomingParam, false),
true => param_type.incoming_type_string(&namespace, false),
false => "var".to_owned(),
};

Expand All @@ -530,7 +523,7 @@ pub fn decode_operation_stream(
) -> CodeBlock {
let cs_encoding = encoding.to_cs_encoding();
let param_type = stream_member.data_type();
let param_type_str = param_type.cs_type_string(namespace, TypeContext::IncomingParam, false);
let param_type_str = param_type.incoming_type_string(namespace, false);
let fixed_wire_size = param_type.fixed_wire_size();

match param_type.concrete_type() {
Expand Down
Loading
Loading