diff --git a/pdl-compiler/src/analyzer.rs b/pdl-compiler/src/analyzer.rs index 9f61453..a6fa29e 100644 --- a/pdl-compiler/src/analyzer.rs +++ b/pdl-compiler/src/analyzer.rs @@ -137,7 +137,6 @@ pub enum ErrorCode { InvalidConditionIdentifier = 47, InvalidConditionValue = 48, E49 = 49, - ReusedConditionIdentifier = 50, InvalidFieldOffset = 51, InvalidFieldSize = 52, InvalidPacketSize = 53, @@ -1517,7 +1516,6 @@ fn check_optional_fields(file: &File) -> Result<(), Diagnostics> { let mut diagnostics: Diagnostics = Default::default(); for decl in &file.declarations { let mut local_scope: HashMap = HashMap::new(); - let mut condition_ids: HashMap = HashMap::new(); for field in decl.fields() { if let Some(ref cond) = field.cond { match &field.desc { @@ -1598,20 +1596,6 @@ fn check_optional_fields(file: &File) -> Result<(), Diagnostics> { ), _ => unreachable!(), } - if let Some(prev_field) = condition_ids.insert(cond.id.to_owned(), field) { - diagnostics.push( - Diagnostic::error() - .with_code(ErrorCode::ReusedConditionIdentifier) - .with_message("reused condition identifier".to_owned()) - .with_labels(vec![ - field.loc.primary(), - prev_field - .loc - .secondary() - .with_message(format!("`{}` is first used here", cond.id)), - ]), - ) - } } if let Some(id) = field.id() { local_scope.insert(id.to_owned(), field); @@ -1817,25 +1801,24 @@ fn desugar_flags(file: &mut File) { | DeclDesc::Struct { fields, .. } | DeclDesc::Group { fields, .. } => { // Gather information about condition flags. - let mut condition_ids: HashMap = HashMap::new(); + let mut condition_ids: HashMap> = HashMap::new(); for field in fields.iter() { if let Some(ref cond) = field.cond { - condition_ids.insert( - cond.id.to_owned(), - (field.id().unwrap().to_owned(), cond.value.unwrap()), + + condition_ids.entry(cond.id.to_owned()).or_default().push( + (field.id().unwrap().to_owned(), cond.value.unwrap()) ); } } // Replace condition flags in the fields. for field in fields.iter_mut() { - if let Some((optional_field_id, set_value)) = + if let Some(optional_field_ids) = field.id().and_then(|id| condition_ids.get(id)) { field.desc = FieldDesc::Flag { id: field.id().unwrap().to_owned(), - optional_field_id: optional_field_id.to_owned(), - set_value: *set_value, - } + optional_field_ids: optional_field_ids.to_owned(), + }; } } } @@ -3084,22 +3067,6 @@ mod test { ); } - #[test] - fn test_e50() { - raises!( - ReusedConditionIdentifier, - r#" - little_endian_packets - packet B { - c : 1, - _reserved_ : 7, - x : 8 if c = 1, - y : 8 if c = 0, - } - "# - ); - } - #[test] fn test_e51() { raises!( diff --git a/pdl-compiler/src/ast.rs b/pdl-compiler/src/ast.rs index e545c5e..859f6e4 100644 --- a/pdl-compiler/src/ast.rs +++ b/pdl-compiler/src/ast.rs @@ -144,7 +144,7 @@ pub enum FieldDesc { /// Special case of Scalar for fields used as condition for /// optional fields. The width is always 1. #[serde(rename = "flag_field")] - Flag { id: String, optional_field_id: String, set_value: usize }, + Flag { id: String, optional_field_ids: Vec<(String, usize)> }, #[serde(rename = "typedef_field")] Typedef { id: String, type_id: String }, #[serde(rename = "group_field")] diff --git a/pdl-compiler/src/backends/rust/encoder.rs b/pdl-compiler/src/backends/rust/encoder.rs index 891ce8e..2f10b07 100644 --- a/pdl-compiler/src/backends/rust/encoder.rs +++ b/pdl-compiler/src/backends/rust/encoder.rs @@ -237,12 +237,45 @@ impl Encoder { let shift = self.bit_shift; match &field.desc { - ast::FieldDesc::Flag { optional_field_id, set_value, .. } => { + ast::FieldDesc::Flag { optional_field_ids, .. } => { + let packet_name = &self.packet_name; + let field_id = field.id().unwrap(); + let (optional_field_id, set_value) = &optional_field_ids[0]; let optional_field_id = optional_field_id.to_ident(); + let cond_value_present = syn::parse_str::(&format!("{}", set_value)).unwrap(); let cond_value_absent = syn::parse_str::(&format!("{}", 1 - set_value)).unwrap(); + + if optional_field_ids.len() >= 2 { + + self.tokens.extend(quote! { + let mut cond_value_is_zero = false; + let mut cond_value_is_one = false; + }); + + for (optional_cond_field, value) in optional_field_ids { + let optional_cond_field_tok = optional_cond_field.to_ident(); + if *value == 1 { + self.tokens.extend(quote! { cond_value_is_one |= self.#optional_cond_field_tok.is_some();} ); + self.tokens.extend(quote! { cond_value_is_zero |= self.#optional_cond_field_tok.is_none();} ); + } else { + self.tokens.extend(quote! { cond_value_is_one |= self.#optional_cond_field_tok.is_none();} ); + self.tokens.extend(quote! { cond_value_is_zero |= self.#optional_cond_field_tok.is_some();} ); + } + } + + self.tokens.extend(quote! { + if cond_value_is_zero && cond_value_is_one { + return Err(EncodeError::InconsistentConditionValue { + packet: #packet_name, + field: #field_id, + }) + } + }); + } + self.bit_fields.push(BitField { value: quote! { if self.#optional_field_id.is_some() { diff --git a/pdl-compiler/src/backends/rust_legacy/serializer.rs b/pdl-compiler/src/backends/rust_legacy/serializer.rs index 2262d27..eaf8f74 100644 --- a/pdl-compiler/src/backends/rust_legacy/serializer.rs +++ b/pdl-compiler/src/backends/rust_legacy/serializer.rs @@ -143,7 +143,10 @@ impl<'a> FieldSerializer<'a> { let shift = self.shift; match &field.desc { - ast::FieldDesc::Flag { optional_field_id, set_value, .. } => { + ast::FieldDesc::Flag { optional_field_ids, .. } => { + assert!(optional_field_ids.len() == 1, "condition flag reuse not supported by legacy generator"); + + let (optional_field_id, set_value) = &optional_field_ids[0]; let optional_field_id = optional_field_id.to_ident(); let cond_value_present = syn::parse_str::(&format!("{}", set_value)).unwrap(); diff --git a/pdl-runtime/src/lib.rs b/pdl-runtime/src/lib.rs index 32d26dc..c4b26d4 100644 --- a/pdl-runtime/src/lib.rs +++ b/pdl-runtime/src/lib.rs @@ -76,6 +76,8 @@ pub enum EncodeError { expected_size: usize, element_index: usize, }, + #[error("{packet}.{field} value cannot be uniquely determined")] + InconsistentConditionValue { packet: &'static str, field: &'static str}, } /// Trait implemented for all toplevel packet declarations. diff --git a/pdl-tests/tests/semantic.rs b/pdl-tests/tests/semantic.rs new file mode 100644 index 0000000..c4a76b0 --- /dev/null +++ b/pdl-tests/tests/semantic.rs @@ -0,0 +1,80 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use pdl_derive::pdl_inline; + +#[pdl_inline( + r#" +little_endian_packets + +enum Enum16 : 16 { + X = 0x1234, + Y = 0x5678, +} + +packet CondTest { + cond: 1, + _reserved_ : 7, + a: 8 if cond = 0, + b: Enum16 if cond = 1, +} +"# +)] +#[cfg(test)] +mod optional_field { + #[test] + fn test_value_0() { + // Success + let test_value_0 = CondTest { + a: Some(255), + b: None, + }; + let mut buf = vec![]; + assert!(test_value_0.encode(&mut buf).is_ok()); + + let decoded_cond = CondTest::decode_full(&buf).unwrap(); + assert_eq!(decoded_cond.a, test_value_0.a); + assert_eq!(decoded_cond.b, test_value_0.b); + } + + #[test] + fn test_value_1() { + // Success + let test_value_1 = CondTest { + a: None, + b: Some(Enum16::X), + }; + let mut buf = vec![]; + assert!(test_value_1.encode(&mut buf).is_ok()); + + let decoded_cond = CondTest::decode_full(&buf).unwrap(); + assert_eq!(decoded_cond.a, test_value_1.a); + assert_eq!(decoded_cond.b, test_value_1.b); + } + + #[test] + fn test_value_inconsistent() { + let test_value_none = CondTest { + a: None, + b: None, + }; + assert!(matches!(test_value_none.encode_to_vec(), Err(EncodeError::InconsistentConditionValue { .. }))); + + let test_value_both = CondTest { + a: Some(255), + b: Some(Enum16::X), + }; + assert!(matches!(test_value_both.encode_to_vec(), Err(EncodeError::InconsistentConditionValue { .. }))); + } +}