From 4c1de1de44dc9b3753587686983a9c93c3ca76ca Mon Sep 17 00:00:00 2001 From: Jeff Kim Date: Tue, 1 Oct 2024 21:09:45 +0900 Subject: [PATCH] Add VintfStability annotation support and passed the related test cases. --- rsbinder-aidl/src/const_expr.rs | 71 +++++++++++++++++++--- rsbinder-aidl/src/generator.rs | 65 ++++++++++++++++---- rsbinder-aidl/src/parser.rs | 2 + rsbinder-aidl/src/type_generator.rs | 17 ++++-- rsbinder-aidl/tests/test_aidl.rs | 92 +++++++++++++++++++++++++++++ rsbinder/src/lib.rs | 2 +- rsbinder/src/parcelable.rs | 2 +- rsbinder/src/parcelable_holder.rs | 22 +------ tests/src/test_client.rs | 40 ++++++------- 9 files changed, 245 insertions(+), 68 deletions(-) diff --git a/rsbinder-aidl/src/const_expr.rs b/rsbinder-aidl/src/const_expr.rs index 8f1797a..e53b97f 100644 --- a/rsbinder-aidl/src/const_expr.rs +++ b/rsbinder-aidl/src/const_expr.rs @@ -72,6 +72,52 @@ macro_rules! arithmetic_basic_op { } } +#[derive(Debug, Clone)] +pub(crate) struct InitParam { + pub is_const: bool, + pub is_fixed_array: bool, + pub is_nullable: bool, + pub is_vintf: bool, + pub crate_name: String, +} + +impl InitParam { + pub(crate) fn builder() -> Self { + Self { + is_const: false, + is_fixed_array: false, + is_nullable: false, + is_vintf: false, + crate_name: "rsbinder".into(), + } + } + + pub(crate) fn with_const(mut self, is_const: bool) -> Self { + self.is_const = is_const; + self + } + + pub(crate) fn with_fixed_array(mut self, is_fixed_array: bool) -> Self { + self.is_fixed_array = is_fixed_array; + self + } + + pub(crate) fn with_nullable(mut self, is_nullable: bool) -> Self { + self.is_nullable = is_nullable; + self + } + + pub(crate) fn with_vintf(mut self, is_vintf: bool) -> Self { + self.is_vintf = is_vintf; + self + } + + pub(crate) fn with_crate_name(mut self, crate_name: &str) -> Self { + self.crate_name = crate_name.to_owned(); + self + } +} + #[derive(Default, Debug, Clone)] pub enum ValueType { #[default] Void, @@ -280,10 +326,10 @@ impl ValueType { } } - pub fn to_init(&self, is_const: bool, is_fixed_array: bool, is_nullable: bool) -> String { + pub(crate) fn to_init(&self, param: InitParam) -> String { match self { ValueType::String(_) => { - if is_const { + if param.is_const { format!("\"{}\"", self.to_value_string()) } else { format!("\"{}\".into()", self.to_value_string()) @@ -294,13 +340,13 @@ impl ValueType { ValueType::Char(_) => format!("'{}' as u16", self.to_value_string()), ValueType::Name(_) => self.to_value_string(), ValueType::Array(v) => { - let mut res = if is_fixed_array { "[".to_owned() } else { "vec![".to_owned() }; + let mut res = if param.is_fixed_array { "[".to_owned() } else { "vec![".to_owned() }; for v in v { - let init_str = v.value.to_init(is_const, is_fixed_array, is_nullable); + let init_str = v.value.to_init(param.clone()); let some_str = if let ValueType::Array(_) = v.value { init_str - } else if is_nullable { + } else if param.is_nullable { format!("Some({})", init_str) } else { init_str @@ -313,13 +359,20 @@ impl ValueType { res } - // ValueType::Holder => { - // "rsbinder::ParcelableHolder::new(rsbinder::Stability::Local)".to_owned() - // } + ValueType::Holder => { + println!("Init Holder: {:?}", param); + if param.is_vintf { + format!("{}::ParcelableHolder::new({}::Stability::Vintf)", param.crate_name, param.crate_name) + } else { + "Default::default()".to_string() + } + } ValueType::Byte(_) | ValueType::Int32(_) | ValueType::Int64(_) | ValueType::Bool(_) | ValueType::Expr{ .. } | ValueType::Unary{ .. } => self.to_value_string(), - _ => "Default::default()".to_string(), + _ => { + "Default::default()".to_string() + } } } diff --git a/rsbinder-aidl/src/generator.rs b/rsbinder-aidl/src/generator.rs index 50c461c..7cc484b 100644 --- a/rsbinder-aidl/src/generator.rs +++ b/rsbinder-aidl/src/generator.rs @@ -8,6 +8,7 @@ use tera::Tera; use crate::{parser, add_indent, Namespace}; use crate::parser::Direction; +use crate::const_expr::{ConstExpr, InitParam, ValueType}; const ENUM_TEMPLATE: &str = r##" pub mod {{mod}} { @@ -77,6 +78,9 @@ pub mod {{mod}} { {{crate}}::impl_deserialize_for_parcelable!(r#{{union_name}}); impl {{crate}}::ParcelableMetadata for r#{{union_name}} { fn descriptor() -> &'static str { "{{ namespace }}" } + {%- if is_vintf %} + fn stability(&self) -> {{crate}}::Stability { {{crate}}::Stability::Vintf } + {%- endif %} } {{crate}}::declare_binder_enum! { Tag : [i32; {{ members|length }}] { @@ -136,6 +140,9 @@ pub mod {{mod}} { {{crate}}::impl_deserialize_for_parcelable!({{name}}); impl {{crate}}::ParcelableMetadata for {{name}} { fn descriptor() -> &'static str { "{{namespace}}" } + {%- if is_vintf %} + fn stability(&self) -> {{crate}}::Stability { {{crate}}::Stability::Vintf } + {%- endif %} } {%- if nested|length>0 %} {{nested}} @@ -214,7 +221,11 @@ pub mod {{mod}} { {%- endfor %} } let wrapped = Wrapper { _inner: inner, _rt: rt }; + {%- if is_vintf %} + let binder = {{crate}}::native::Binder::new_with_stability({{bn_name}}(Box::new(wrapped)), {{crate}}::Stability::Vintf); + {%- else %} let binder = {{crate}}::native::Binder::new_with_stability({{bn_name}}(Box::new(wrapped)), {{crate}}::Stability::default()); + {%- endif %} {{crate}}::Strong::new(Box::new(binder)) } } @@ -506,13 +517,19 @@ impl Generator { Self { enabled_async, is_crate } } - fn new_context(&self) -> tera::Context { - let mut context = tera::Context::new(); + fn get_crate_name(&self) -> &str { if self.is_crate { - context.insert("crate", "crate"); + "crate" } else { - context.insert("crate", "rsbinder"); + "rsbinder" } + } + + fn new_context(&self) -> tera::Context { + let mut context = tera::Context::new(); + + context.insert("crate", self.get_crate_name()); + context } @@ -561,13 +578,10 @@ impl Generator { } fn decl_interface(&self, arg_decl: &parser::InterfaceDecl, indent: usize) -> Result> { - let mut is_empty = false; let mut decl = arg_decl.clone(); - if parser::check_annotation_list(&decl.annotation_list, parser::AnnotationType::JavaOnly).0 { - is_empty = true; - // return Ok(String::new()) - } + let is_empty = parser::check_annotation_list(&decl.annotation_list, parser::AnnotationType::JavaOnly).0; + let is_vintf = parser::check_annotation_list(&decl.annotation_list, parser::AnnotationType::VintfStability).0; decl.pre_process(); @@ -578,7 +592,10 @@ impl Generator { for constant in decl.constant_list.iter() { let generator = constant.r#type.to_generator(); const_members.push((constant.const_identifier(), - generator.const_type_decl(), generator.init_value(constant.const_expr.as_ref(), true))); + generator.const_type_decl(), + generator.init_value(constant.const_expr.as_ref(), + InitParam::builder().with_const(true)) + )); } for method in decl.method_list.iter() { @@ -605,6 +622,7 @@ impl Generator { context.insert("oneway", &decl.oneway); context.insert("nested", &nested.trim()); context.insert("enabled_async", &enabled_async); + context.insert("is_vintf", &is_vintf); let rendered = TEMPLATES.render("interface", &context).expect("Failed to render interface template"); @@ -615,6 +633,8 @@ impl Generator { let mut is_empty = false; let mut decl = arg_decl.clone(); + let is_vintf = parser::check_annotation_list(&decl.annotation_list, parser::AnnotationType::VintfStability).0; + if parser::check_annotation_list(&decl.annotation_list, parser::AnnotationType::JavaOnly).0 { println!("Parcelable {} is only used for Java.", decl.name); is_empty = true; @@ -636,17 +656,30 @@ impl Generator { // Parse struct variables only. for decl in &decl.members { if let Some(var) = decl.is_variable() { + println!("Parcelable variable: {:?}", var); let generator = var.r#type.to_generator(); if var.constant { constant_members.push((var.const_identifier(), - generator.const_type_decl(), generator.init_value(var.const_expr.as_ref(), true))); + generator.const_type_decl(), + generator.init_value(var.const_expr.as_ref(), + InitParam::builder().with_const(true)) + )); } else { + let init_value = match generator.value_type { + ValueType::Holder => Some(ConstExpr::new(ValueType::Holder)), + _ => var.const_expr.clone() + }; + members.push( ( var.identifier(), generator.type_declaration(true), - generator.init_value(var.const_expr.as_ref(), false) + generator.init_value(init_value.as_ref(), + InitParam::builder().with_const(false) + .with_vintf(is_vintf) + .with_crate_name(self.get_crate_name()) + ) ) ) } @@ -669,6 +702,7 @@ impl Generator { context.insert("members", &members); context.insert("const_members", &constant_members); context.insert("nested", &nested.trim()); + context.insert("is_vintf", &is_vintf); let rendered = TEMPLATES.render("parcelable", &context).expect("Failed to render parcelable template"); @@ -710,6 +744,8 @@ impl Generator { return Ok(String::new()) } + let is_vintf = parser::check_annotation_list(&decl.annotation_list, parser::AnnotationType::VintfStability).0; + let mut constant_members = Vec::new(); let mut members = Vec::new(); @@ -718,7 +754,9 @@ impl Generator { let generator = var.r#type.to_generator(); if var.constant { constant_members.push((var.const_identifier(), - generator.const_type_decl(), generator.init_value(var.const_expr.as_ref(), true))); + generator.const_type_decl(), + generator.init_value(var.const_expr.as_ref(), + InitParam::builder().with_const(true)))); } else { members.push((var.union_identifier(), generator.type_declaration(true), var.identifier(), generator.default_value())); } @@ -738,6 +776,7 @@ impl Generator { context.insert("namespace", &namespace); context.insert("members", &members); context.insert("const_members", &constant_members); + context.insert("is_vintf", &is_vintf); let rendered = TEMPLATES.render("union", &context).expect("Failed to render union template"); diff --git a/rsbinder-aidl/src/parser.rs b/rsbinder-aidl/src/parser.rs index 4bfd212..a74ea98 100644 --- a/rsbinder-aidl/src/parser.rs +++ b/rsbinder-aidl/src/parser.rs @@ -496,11 +496,13 @@ pub enum AnnotationType { IsNullable, JavaOnly, RustDerive, + VintfStability, } pub fn check_annotation_list(annotation_list: &Vec, query_type: AnnotationType) -> (bool, String) { for annotation in annotation_list { match query_type { + AnnotationType::VintfStability if annotation.annotation == "@VintfStability" => return (true, "".to_owned()), AnnotationType::IsNullable if annotation.annotation == "@nullable" => return (true, "".to_owned()), AnnotationType::JavaOnly if annotation.annotation.starts_with("@JavaOnly") => return (true, "".to_owned()), AnnotationType::RustDerive if annotation.annotation == "@RustDerive" => { diff --git a/rsbinder-aidl/src/type_generator.rs b/rsbinder-aidl/src/type_generator.rs index 8dc4018..8123bc9 100644 --- a/rsbinder-aidl/src/type_generator.rs +++ b/rsbinder-aidl/src/type_generator.rs @@ -4,7 +4,7 @@ use std::sync::OnceLock; use crate::parser::{*, self}; -use crate::const_expr::{ValueType, ConstExpr}; +use crate::const_expr::{ValueType, ConstExpr, InitParam}; static CRATE_NAME: OnceLock = OnceLock::new(); @@ -591,15 +591,20 @@ impl TypeGenerator { } } - pub fn init_value(&self, const_expr: Option<&ConstExpr>, is_const: bool) -> String { + pub(crate) fn init_value(&self, const_expr: Option<&ConstExpr>, param: InitParam) -> String { match const_expr { Some(expr) => { let init_str = if let ValueType::Array(_) = self.value_type { let array_info = self.array_types.first().unwrap(); let is_nullable = self.is_nullable && Self::is_aidl_nullable(&array_info.value_type); - expr.calculate().convert_to(&array_info.value_type).value.to_init(is_const, array_info.is_fixed(), is_nullable) + expr.calculate().convert_to(&array_info.value_type).value.to_init( + param.with_fixed_array(array_info.is_fixed()) + .with_nullable(is_nullable)) } else { - expr.calculate().convert_to(&self.value_type).value.to_init(is_const, false, false) + expr.calculate().convert_to(&self.value_type).value.to_init( + param.with_fixed_array(false) + .with_nullable(false) + ) }; if self.is_nullable { format!("Some({})", init_str) @@ -607,7 +612,9 @@ impl TypeGenerator { init_str } } - None => ValueType::Void.to_init(is_const, false, false), + None => ValueType::Void.to_init(param.with_fixed_array(false) + .with_nullable(false) + ), } } } diff --git a/rsbinder-aidl/tests/test_aidl.rs b/rsbinder-aidl/tests/test_aidl.rs index ba797a5..11e58c9 100644 --- a/rsbinder-aidl/tests/test_aidl.rs +++ b/rsbinder-aidl/tests/test_aidl.rs @@ -1033,6 +1033,98 @@ pub mod ArrayOfInterfaces { "##) } +#[test] +fn test_parcelable_vintf() -> Result<(), Box> { + aidl_generator(r##" +package android.aidl.tests.vintf; + +@VintfStability +parcelable VintfExtendableParcelable { + ParcelableHolder ext; +} + "##, r#" +pub mod VintfExtendableParcelable { + #![allow(non_upper_case_globals, non_snake_case, dead_code)] + #[derive(Debug)] + pub struct VintfExtendableParcelable { + pub r#ext: rsbinder::ParcelableHolder, + } + impl Default for VintfExtendableParcelable { + fn default() -> Self { + Self { + r#ext: rsbinder::ParcelableHolder::new(rsbinder::Stability::Vintf), + } + } + } + impl rsbinder::Parcelable for VintfExtendableParcelable { + fn write_to_parcel(&self, _parcel: &mut rsbinder::Parcel) -> rsbinder::Result<()> { + _parcel.sized_write(|_sub_parcel| { + _sub_parcel.write(&self.r#ext)?; + Ok(()) + }) + } + fn read_from_parcel(&mut self, _parcel: &mut rsbinder::Parcel) -> rsbinder::Result<()> { + _parcel.sized_read(|_sub_parcel| { + self.r#ext = _sub_parcel.read()?; + Ok(()) + }) + } + } + rsbinder::impl_serialize_for_parcelable!(VintfExtendableParcelable); + rsbinder::impl_deserialize_for_parcelable!(VintfExtendableParcelable); + impl rsbinder::ParcelableMetadata for VintfExtendableParcelable { + fn descriptor() -> &'static str { "android.aidl.tests.vintf.VintfExtendableParcelable" } + fn stability(&self) -> rsbinder::Stability { rsbinder::Stability::Vintf } + } +} + "#)?; + + aidl_generator(r##" +package android.aidl.tests.vintf; + +@VintfStability +parcelable VintfParcelable { + int a; +} + "##, r#" +pub mod VintfParcelable { + #![allow(non_upper_case_globals, non_snake_case, dead_code)] + #[derive(Debug)] + pub struct VintfParcelable { + pub r#a: i32, + } + impl Default for VintfParcelable { + fn default() -> Self { + Self { + r#a: Default::default(), + } + } + } + impl rsbinder::Parcelable for VintfParcelable { + fn write_to_parcel(&self, _parcel: &mut rsbinder::Parcel) -> rsbinder::Result<()> { + _parcel.sized_write(|_sub_parcel| { + _sub_parcel.write(&self.r#a)?; + Ok(()) + }) + } + fn read_from_parcel(&mut self, _parcel: &mut rsbinder::Parcel) -> rsbinder::Result<()> { + _parcel.sized_read(|_sub_parcel| { + self.r#a = _sub_parcel.read()?; + Ok(()) + }) + } + } + rsbinder::impl_serialize_for_parcelable!(VintfParcelable); + rsbinder::impl_deserialize_for_parcelable!(VintfParcelable); + impl rsbinder::ParcelableMetadata for VintfParcelable { + fn descriptor() -> &'static str { "android.aidl.tests.vintf.VintfParcelable" } + fn stability(&self) -> rsbinder::Stability { rsbinder::Stability::Vintf } + } +} + "#)?; + Ok(()) +} + #[test] fn test_parcelable_const_name() -> Result<(), Box> { aidl_generator(r##" diff --git a/rsbinder/src/lib.rs b/rsbinder/src/lib.rs index a37c65f..c665abe 100644 --- a/rsbinder/src/lib.rs +++ b/rsbinder/src/lib.rs @@ -45,7 +45,7 @@ pub use proxy::*; pub use native::*; pub use parcelable::*; pub use file_descriptor::ParcelFileDescriptor; -pub use parcelable_holder::{ParcelableHolder, ParcelableMetadata}; +pub use parcelable_holder::ParcelableHolder; #[cfg(feature = "async")] pub use binder_async::{BinderAsyncPool, BinderAsyncRuntime, BoxFuture}; #[cfg(feature = "tokio")] diff --git a/rsbinder/src/parcelable.rs b/rsbinder/src/parcelable.rs index e60e349..71f97de 100644 --- a/rsbinder/src/parcelable.rs +++ b/rsbinder/src/parcelable.rs @@ -62,7 +62,7 @@ pub trait ParcelableMetadata { fn descriptor() -> &'static str; /// The Binder parcelable stability. - fn get_stability(&self) -> Stability { + fn stability(&self) -> Stability { Stability::Local } } diff --git a/rsbinder/src/parcelable_holder.rs b/rsbinder/src/parcelable_holder.rs index 8dd0619..29edfc9 100644 --- a/rsbinder/src/parcelable_holder.rs +++ b/rsbinder/src/parcelable_holder.rs @@ -20,7 +20,7 @@ use crate::binder::Stability; use crate::error::{StatusCode, Result}; use crate::{ - Deserialize, Parcel, Parcelable, Serialize, NON_NULL_PARCELABLE_FLAG, + Deserialize, Parcel, Parcelable, ParcelableMetadata, Serialize, NON_NULL_PARCELABLE_FLAG, NULL_PARCELABLE_FLAG, }; @@ -28,22 +28,6 @@ use downcast_rs::{impl_downcast, DowncastSync}; use std::any::Any; use std::sync::{Arc, Mutex}; -/// Metadata that `ParcelableHolder` needs for all parcelables. -/// -/// The compiler auto-generates implementations of this trait -/// for AIDL parcelables. -pub trait ParcelableMetadata { - /// The Binder parcelable descriptor string. - /// - /// This string is a unique identifier for a Binder parcelable. - fn descriptor() -> &'static str; - - /// The Binder parcelable stability. - fn get_stability(&self) -> Stability { - Stability::Local - } -} - trait AnyParcelable: DowncastSync + Parcelable + std::fmt::Debug {} impl_downcast!(sync AnyParcelable); impl AnyParcelable for T where T: DowncastSync + Parcelable + std::fmt::Debug {} @@ -106,11 +90,11 @@ impl ParcelableHolder { where T: Any + Parcelable + ParcelableMetadata + std::fmt::Debug + Send + Sync, { - if self.stability > p.get_stability() { + if self.stability > p.stability() { log::error!( "ParcelableHolder::set_parcelable: parcelable stability mismatch: {:?} > {:?}", self.stability, - p.get_stability()); + p.stability()); return Err(StatusCode::BadValue); } diff --git a/tests/src/test_client.rs b/tests/src/test_client.rs index fab8fea..08fe4d6 100644 --- a/tests/src/test_client.rs +++ b/tests/src/test_client.rs @@ -707,27 +707,27 @@ test_parcelable_holder_stability! { UnstableParcelable } -// #[test] -// fn test_vintf_parcelable_holder_cannot_contain_not_vintf_parcelable() { -// let mut holder = VintfExtendableParcelable::default(); -// let parcelable = Arc::new(NonVintfParcelable::default()); -// let result = holder.ext.set_parcelable(Arc::clone(&parcelable)); -// assert_eq!(result, Err(rsbinder::StatusCode::BadValue)); - -// let parcelable2 = holder.ext.get_parcelable::(); -// assert!(parcelable2.unwrap().is_none()); -// } +#[test] +fn test_vintf_parcelable_holder_cannot_contain_not_vintf_parcelable() { + let mut holder = VintfExtendableParcelable::default(); + let parcelable = Arc::new(NonVintfParcelable::default()); + let result = holder.ext.set_parcelable(Arc::clone(&parcelable)); + assert_eq!(result, Err(rsbinder::StatusCode::BadValue)); -// #[test] -// fn test_vintf_parcelable_holder_cannot_contain_unstable_parcelable() { -// let mut holder = VintfExtendableParcelable::default(); -// let parcelable = Arc::new(UnstableParcelable::default()); -// let result = holder.ext.set_parcelable(Arc::clone(&parcelable)); -// assert_eq!(result, Err(rsbinder::StatusCode::BadValue)); - -// let parcelable2 = holder.ext.get_parcelable::(); -// assert!(parcelable2.unwrap().is_none()); -// } + let parcelable2 = holder.ext.get_parcelable::(); + assert!(parcelable2.unwrap().is_none()); +} + +#[test] +fn test_vintf_parcelable_holder_cannot_contain_unstable_parcelable() { + let mut holder = VintfExtendableParcelable::default(); + let parcelable = Arc::new(UnstableParcelable::default()); + let result = holder.ext.set_parcelable(Arc::clone(&parcelable)); + assert_eq!(result, Err(rsbinder::StatusCode::BadValue)); + + let parcelable2 = holder.ext.get_parcelable::(); + assert!(parcelable2.unwrap().is_none()); +} #[test] #[ignore]