From 15405db5a59d43cb8fbd374dc3a14f007f7bdffe Mon Sep 17 00:00:00 2001 From: John Sheu <sheu@google.com> Date: Mon, 13 May 2019 23:03:05 +0000 Subject: [PATCH] [banjo] Support more types as union fields This causes union fields to be generated using the same logic as struct fields, so that arrays and strings are properly handled. Note for now that vector fields are excluded from unions, as they actually generate two fields (a pointer and a count) which does not interact well with unions. Change-Id: I69a36903390740d5443f5e3ff0d00d3e49e423f8 --- tools/banjo/banjo/src/backends/c.rs | 167 ++++---- tools/banjo/banjo/test/ast/types.test.ast | 355 ++++++++++++++++++ tools/banjo/banjo/test/banjo/types.test.banjo | 31 ++ .../banjo/banjo/test/c/protocol-other-types.h | 2 +- tools/banjo/banjo/test/c/types.h | 32 +- 5 files changed, 504 insertions(+), 83 deletions(-) diff --git a/tools/banjo/banjo/src/backends/c.rs b/tools/banjo/banjo/src/backends/c.rs index 8569ece566b..685bd3380ba 100644 --- a/tools/banjo/banjo/src/backends/c.rs +++ b/tools/banjo/banjo/src/backends/c.rs @@ -167,6 +167,88 @@ fn struct_attrs_to_c_str(attributes: &Attrs) -> String { .join(" ") } +fn field_to_c_str( + attrs: &Attrs, + ty: &ast::Ty, + ident: &Ident, + indent: &str, + ast: &ast::BanjoAst, +) -> Result<String, Error> { + let mut accum = String::new(); + accum.push_str(get_doc_comment(attrs, 1).as_str()); + let prefix = if ty.is_reference() { "" } else { "const " }; + match ty { + ast::Ty::Vector { ty: ref inner_ty, .. } => { + let ty_name = ty_to_c_str(ast, &ty)?; + // TODO(surajmalhotra): Support multi-dimensional vectors. + let ptr = if inner_ty.is_reference() { "*" } else { "" }; + accum.push_str( + format!( + "{indent}{prefix}{ty}{ptr}* {c_name}_{buffer};\n\ + {indent}size_t {c_name}_{size};", + indent = indent, + buffer = name_buffer(&ty_name), + size = name_size(&ty_name), + c_name = to_c_name(ident.name()), + prefix = prefix, + ty = ty_name, + ptr = ptr, + ) + .as_str(), + ); + } + ast::Ty::Array { .. } => { + let bounds = array_bounds(ast, &ty).unwrap(); + accum.push_str( + format!( + "{indent}{ty} {c_name}{bounds};", + indent = indent, + c_name = to_c_name(ident.name()), + bounds = bounds, + ty = ty_to_c_str(ast, &ty)? + ) + .as_str(), + ); + } + ast::Ty::Str { ref size, .. } => { + if let Some(size) = size { + accum.push_str( + format!( + "{indent}char {c_name}[{size}];", + indent = indent, + c_name = to_c_name(ident.name()), + size = size, + ) + .as_str(), + ); + } else { + accum.push_str( + format!( + "{indent}{prefix}{ty} {c_name};", + indent = indent, + c_name = to_c_name(ident.name()), + prefix = prefix, + ty = ty_to_c_str(ast, &ty)? + ) + .as_str(), + ); + } + } + _ => { + accum.push_str( + format!( + "{indent}{ty} {c_name};", + indent = indent, + c_name = to_c_name(ident.name()), + ty = ty_to_c_str(ast, &ty)? + ) + .as_str(), + ); + } + } + Ok(accum) +} + fn get_first_param(ast: &BanjoAst, method: &ast::Method) -> Result<(bool, String), Error> { // Return parameter if a primitive type. if method.out_params.get(0).map_or(false, |p| p.1.is_primitive(&ast)) { @@ -475,18 +557,9 @@ impl<'a, W: io::Write> CBackend<'a, W> { let attrs = struct_attrs_to_c_str(attributes); let members = fields .iter() - .map(|f| { - let mut accum = String::new(); - accum.push_str(get_doc_comment(&f.attributes, 1).as_str()); - accum.push_str( - format!( - " {ty} {c_name};", - c_name = to_c_name(f.ident.name()), - ty = ty_to_c_str(ast, &f.ty)? - ) - .as_str(), - ); - Ok(accum) + .map(|f| match f.ty { + ast::Ty::Vector { .. } => Err(format_err!("unsupported for UnionField: {:?}", f)), + _ => field_to_c_str(&f.attributes, &f.ty, &f.ident, " ", &ast), }) .collect::<Result<Vec<_>, Error>>()? .join("\n"); @@ -533,75 +606,7 @@ impl<'a, W: io::Write> CBackend<'a, W> { let attrs = struct_attrs_to_c_str(attributes); let members = fields .iter() - .map(|f| { - let mut accum = String::new(); - accum.push_str(get_doc_comment(&f.attributes, 1).as_str()); - let prefix = if f.ty.is_reference() { "" } else { "const " }; - match f.ty { - ast::Ty::Vector { ty: ref inner_ty, .. } => { - let ty_name = ty_to_c_str(ast, &f.ty)?; - // TODO(surajmalhotra): Support multi-dimensional vectors. - let ptr = if inner_ty.is_reference() { "*" } else { "" }; - accum.push_str( - format!( - " {prefix}{ty}{ptr}* {c_name}_{buffer};\n size_t {c_name}_{size};", - buffer = name_buffer(&ty_name), - size = name_size(&ty_name), - c_name = to_c_name(f.ident.name()), - prefix = prefix, - ty = ty_name, - ptr = ptr, - ) - .as_str(), - ); - } - ast::Ty::Array { .. } => { - let bounds = array_bounds(ast, &f.ty).unwrap(); - accum.push_str( - format!( - " {ty} {c_name}{bounds};", - c_name = to_c_name(f.ident.name()), - bounds = bounds, - ty = ty_to_c_str(ast, &f.ty)? - ) - .as_str(), - ); - } - ast::Ty::Str { ref size, .. } => { - if let Some(size) = size { - accum.push_str( - format!( - " char {c_name}[{size}];", - c_name = to_c_name(f.ident.name()), - size = size, - ) - .as_str(), - ); - } else { - accum.push_str( - format!( - " {prefix}{ty} {c_name};", - c_name = to_c_name(f.ident.name()), - prefix = prefix, - ty = ty_to_c_str(ast, &f.ty)? - ) - .as_str(), - ); - } - } - _ => { - accum.push_str( - format!( - " {ty} {c_name};", - c_name = to_c_name(f.ident.name()), - ty = ty_to_c_str(ast, &f.ty)? - ) - .as_str(), - ); - } - } - Ok(accum) - }) + .map(|f| field_to_c_str(&f.attributes, &f.ty, &f.ident, " ", &ast)) .collect::<Result<Vec<_>, Error>>()? .join("\n"); let mut accum = String::new(); diff --git a/tools/banjo/banjo/test/ast/types.test.ast b/tools/banjo/banjo/test/ast/types.test.ast index 618baa275c0..4dfd5973888 100644 --- a/tools/banjo/banjo/test/ast/types.test.ast +++ b/tools/banjo/banjo/test/ast/types.test.ast @@ -4846,6 +4846,361 @@ BanjoAst { }, ], }, + Union { + attributes: Attrs( + [], + ), + name: Ident { + namespace: Some( + "banjo.examples.types", + ), + name: "union_types", + }, + fields: [ + UnionField { + attributes: Attrs( + [], + ), + ty: Bool, + ident: Ident { + namespace: None, + name: "b", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Int8, + ident: Ident { + namespace: None, + name: "i8", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Int16, + ident: Ident { + namespace: None, + name: "i16", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Int32, + ident: Ident { + namespace: None, + name: "i32", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Int64, + ident: Ident { + namespace: None, + name: "i64", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: UInt8, + ident: Ident { + namespace: None, + name: "u8", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: UInt16, + ident: Ident { + namespace: None, + name: "u16", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: UInt32, + ident: Ident { + namespace: None, + name: "u32", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: UInt64, + ident: Ident { + namespace: None, + name: "u64", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Float32, + ident: Ident { + namespace: None, + name: "f32", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Float64, + ident: Ident { + namespace: None, + name: "f64", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: Bool, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "b_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: Int8, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "i8_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: Int16, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "i16_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: Int32, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "i32_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: Int64, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "i64_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: UInt8, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "u8_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: UInt16, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "u16_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: UInt32, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "u32_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: UInt64, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "u64_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: Float32, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "f32_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: Float64, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "f64_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Array { + ty: Handle { + ty: Handle, + reference: false, + }, + size: Constant( + "1", + ), + }, + ident: Ident { + namespace: None, + name: "handle_0", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Str { + size: None, + nullable: false, + }, + ident: Ident { + namespace: None, + name: "str", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Identifier { + id: Ident { + namespace: Some( + "banjo.examples.types", + ), + name: "this_is_a_struct", + }, + reference: false, + }, + ident: Ident { + namespace: None, + name: "s", + }, + }, + UnionField { + attributes: Attrs( + [], + ), + ty: Identifier { + id: Ident { + namespace: Some( + "banjo.examples.types", + ), + name: "this_is_a_union", + }, + reference: false, + }, + ident: Ident { + namespace: None, + name: "u", + }, + }, + ], + }, ], }, } diff --git a/tools/banjo/banjo/test/banjo/types.test.banjo b/tools/banjo/banjo/test/banjo/types.test.banjo index d00e7ee3503..51fbb464965 100644 --- a/tools/banjo/banjo/test/banjo/types.test.banjo +++ b/tools/banjo/banjo/test/banjo/types.test.banjo @@ -337,3 +337,34 @@ struct interfaces { this_is_an_interface i; this_is_an_interface? nullable_i; }; + +union union_types { + bool b; + int8 i8; + int16 i16; + int32 i32; + int64 i64; + uint8 u8; + uint16 u16; + uint32 u32; + uint64 u64; + float32 f32; + float64 f64; + + array<bool>:1 b_0; + array<int8>:1 i8_0; + array<int16>:1 i16_0; + array<int32>:1 i32_0; + array<int64>:1 i64_0; + array<uint8>:1 u8_0; + array<uint16>:1 u16_0; + array<uint32>:1 u32_0; + array<uint64>:1 u64_0; + array<float32>:1 f32_0; + array<float64>:1 f64_0; + array<handle>:1 handle_0; + + string str; + this_is_a_struct s; + this_is_a_union u; +}; diff --git a/tools/banjo/banjo/test/c/protocol-other-types.h b/tools/banjo/banjo/test/c/protocol-other-types.h index 2a6dc35aa97..cd4e7fdacdf 100644 --- a/tools/banjo/banjo/test/c/protocol-other-types.h +++ b/tools/banjo/banjo/test/c/protocol-other-types.h @@ -44,7 +44,7 @@ struct this_is_astruct { }; union this_is_aunion { - char* s; + const char* s; }; typedef struct other_types_reference_protocol_ops { diff --git a/tools/banjo/banjo/test/c/types.h b/tools/banjo/banjo/test/c/types.h index 300f7b721b2..aa4d7ae1bd3 100644 --- a/tools/banjo/banjo/test/c/types.h +++ b/tools/banjo/banjo/test/c/types.h @@ -46,6 +46,7 @@ typedef uint8_t u8_enum_t; typedef struct this_is_an_interface_protocol this_is_an_interface_protocol_t; typedef struct structs structs_t; typedef struct unions unions_t; +typedef union union_types union_types_t; typedef struct interfaces interfaces_t; typedef struct interfaces interfaces_t; @@ -421,7 +422,7 @@ struct handles { }; union this_is_a_union { - char* s; + const char* s; }; typedef struct this_is_an_interface_protocol_ops { @@ -449,6 +450,35 @@ struct unions { this_is_a_union_t nullable_u; }; +union union_types { + bool b; + int8_t i8; + int16_t i16; + int32_t i32; + int64_t i64; + uint8_t u8; + uint16_t u16; + uint32_t u32; + uint64_t u64; + float f32; + double f64; + bool b_0[1]; + int8_t i8_0[1]; + int16_t i16_0[1]; + int32_t i32_0[1]; + int64_t i64_0[1]; + uint8_t u8_0[1]; + uint16_t u16_0[1]; + uint32_t u32_0[1]; + uint64_t u64_0[1]; + float f32_0[1]; + double f64_0[1]; + zx_handle_t handle_0[1]; + const char* str; + this_is_a_struct_t s; + this_is_a_union_t u; +}; + struct interfaces { this_is_an_interface_protocol_t nonnullable_interface; this_is_an_interface_protocol_t nullable_interface; -- GitLab