diff --git a/src/diagnostics/errors.rs b/src/diagnostics/errors.rs index af344ce4..decf5d7f 100644 --- a/src/diagnostics/errors.rs +++ b/src/diagnostics/errors.rs @@ -79,8 +79,8 @@ pub enum Error { enumerator_identifier: String, }, - /// Enumerators cannot declare associated fields when their enclosing enum has an underlying type. - EnumeratorCannotDeclareAssociatedFields { + /// Enumerators cannot contain fields when their enclosing enum has an underlying type. + EnumeratorCannotContainFields { enumerator_identifier: String, }, @@ -549,8 +549,8 @@ implement_diagnostic_functions!( ), ( "E054", - EnumeratorCannotDeclareAssociatedFields, - format!("invalid enumerator '{enumerator_identifier}': associated fields cannot be declared within enums that specify an underlying type"), + EnumeratorCannotContainFields, + format!("invalid enumerator '{enumerator_identifier}': fields cannot be declared within enums that specify an underlying type"), enumerator_identifier ) ); diff --git a/src/grammar/elements/enumerator.rs b/src/grammar/elements/enumerator.rs index fd11ef1d..75a8dadb 100644 --- a/src/grammar/elements/enumerator.rs +++ b/src/grammar/elements/enumerator.rs @@ -8,7 +8,7 @@ use crate::utils::ptr_util::WeakPtr; pub struct Enumerator { pub identifier: Identifier, pub value: EnumeratorValue, - pub associated_fields: Option>>, + pub fields: Option>>, pub parent: WeakPtr, pub scope: Scope, pub attributes: Vec>, @@ -24,10 +24,11 @@ impl Enumerator { } } - pub fn associated_fields(&self) -> Option> { - self.associated_fields + pub fn fields(&self) -> Vec<&Field> { + self.fields .as_ref() .map(|fields| fields.iter().map(WeakPtr::borrow).collect()) + .unwrap_or_default() } } @@ -39,7 +40,7 @@ pub enum EnumeratorValue { impl Container for Enumerator { fn contents(&self) -> Vec<&Field> { - self.associated_fields().unwrap_or_default() + self.fields() } } diff --git a/src/parsers/slice/grammar.rs b/src/parsers/slice/grammar.rs index c6bc84f3..01e8f8cf 100644 --- a/src/parsers/slice/grammar.rs +++ b/src/parsers/slice/grammar.rs @@ -396,7 +396,7 @@ fn construct_enumerator( parser: &mut Parser, (raw_comment, attributes): (RawDocComment, Vec>), identifier: Identifier, - associated_fields: Option>>, + fields: Option>>, enumerator_value: Option>, span: Span, ) -> OwnedPtr { @@ -413,7 +413,7 @@ fn construct_enumerator( let mut enumerator = OwnedPtr::new(Enumerator { identifier, value, - associated_fields: None, + fields: None, parent: WeakPtr::create_uninitialized(), // Patched by its container. scope: parser.current_scope.clone(), attributes, @@ -422,14 +422,14 @@ fn construct_enumerator( }); // Add any associated fields to the enumerator. - if let Some(fields) = associated_fields { + if let Some(fields) = fields { let downgraded = downgrade_as!(enumerator, dyn Container); unsafe { let converted_fields = fields.into_iter().map(|mut field| { field.borrow_mut().parent = downgraded.clone(); parser.ast.add_named_element(field) }); - enumerator.borrow_mut().associated_fields = Some(converted_fields.collect()); + enumerator.borrow_mut().fields = Some(converted_fields.collect()); } } diff --git a/src/patchers/encoding_patcher.rs b/src/patchers/encoding_patcher.rs index d2f86f87..6153218b 100644 --- a/src/patchers/encoding_patcher.rs +++ b/src/patchers/encoding_patcher.rs @@ -416,13 +416,13 @@ impl ComputeSupportedEncodings for Enum { } for enumerator in self.enumerators() { - // Enums with associated fields are not allowed in Slice1 mode. - if enumerator.associated_fields().is_some() { + // Enums with fields are not allowed in Slice1 mode. + if enumerator.fields.is_some() { supported_encodings.disable(Encoding::Slice1); if compilation_mode == CompilationMode::Slice1 { - return Some("enumerators declared in Slice1 mode cannot have associated fields"); + return Some("field syntax cannot be used with enumerators declared in Slice1 mode"); } - break; // Once we've found a single enumerator with associated fields, we can stop checking. + break; // Once we've found a single enumerator with fields, we can stop checking. } } diff --git a/src/validators/cycle_detection.rs b/src/validators/cycle_detection.rs index 7e8f43f4..676b121d 100644 --- a/src/validators/cycle_detection.rs +++ b/src/validators/cycle_detection.rs @@ -14,7 +14,7 @@ pub(super) fn detect_cycles(ast: &Ast, diagnostics: &mut Diagnostics) { for node in ast.as_slice() { cycle_detector.dependency_stack.clear(); // Make sure the detector is cleared between checks. match node { - // Only structs and enumerators (with associated fields) need to be checked for cycles. + // Only structs and enumerators (with fields) need to be checked for cycles. // It is safe for classes to contain cycles since they use reference semantics, // and exceptions can't cause cycles since they cannot be used as types. Node::Struct(struct_def) => cycle_detector.check_for_cycles(struct_def.borrow()), diff --git a/src/validators/dictionary.rs b/src/validators/dictionary.rs index babfdb7f..acac46b5 100644 --- a/src/validators/dictionary.rs +++ b/src/validators/dictionary.rs @@ -48,8 +48,21 @@ fn check_dictionary_key_type(type_ref: &TypeRef) -> Option { } true } + + // Only enums with underlying types can be used as dictionary keys. Fields aren't allowed. + Types::Enum(enum_def) => { + if enum_def.underlying.is_none() { + let error = Diagnostic::new(Error::KeyTypeNotSupported { + kind: formatted_kind(definition), + }) + .set_span(type_ref.span()) + .add_note("only enums with underlying types can be used as dictionary keys", None); + return Some(error); + } + true + } + Types::Class(_) => false, - Types::Enum(_) => true, Types::CustomType(_) => true, Types::Sequence(_) => false, Types::Dictionary(_) => false, @@ -70,9 +83,9 @@ fn check_dictionary_key_type(type_ref: &TypeRef) -> Option { } fn formatted_kind(definition: &dyn Type) -> String { - if let Types::Class(class_def) = definition.concrete_type() { - format!("class '{}'", class_def.identifier()) - } else { - definition.kind().to_owned() + match definition.concrete_type() { + Types::Class(class_def) => format!("class '{}'", class_def.identifier()), + Types::Enum(enum_def) => format!("enum '{}'", enum_def.identifier()), + _ => definition.kind().to_owned(), } } diff --git a/src/validators/enums.rs b/src/validators/enums.rs index 440b907e..251e8f36 100644 --- a/src/validators/enums.rs +++ b/src/validators/enums.rs @@ -12,11 +12,11 @@ pub fn validate_enum(enum_def: &Enum, diagnostics: &mut Diagnostics) { underlying_type_cannot_be_optional(enum_def, diagnostics); nonempty_if_checked(enum_def, diagnostics); - // If the enum wasn't defined in a Slice1 file, validate whether associated fields or explicit values are allowed, - // based on whether it has an underlying type. Associated fields in Slice1 files are rejected by `encoding_patcher`. + // If the enum wasn't defined in a Slice1 file, validate whether fields or explicit values are allowed, + // based on whether it has an underlying type. Fields in Slice1 files are already rejected by `encoding_patcher`. if !enum_def.supported_encodings().supports(Encoding::Slice1) { if enum_def.underlying.is_some() { - cannot_contain_associated_fields(enum_def, diagnostics); + cannot_contain_fields(enum_def, diagnostics); } else { cannot_contain_explicit_values(enum_def, diagnostics); } @@ -138,14 +138,14 @@ fn nonempty_if_checked(enum_def: &Enum, diagnostics: &mut Diagnostics) { } } -/// Validate that this enum's enumerators don't specify any associated fields. +/// Validate that this enum's enumerators don't specify any fields. /// This function should only be called for enums with underlying types. -fn cannot_contain_associated_fields(enum_def: &Enum, diagnostics: &mut Diagnostics) { +fn cannot_contain_fields(enum_def: &Enum, diagnostics: &mut Diagnostics) { debug_assert!(enum_def.underlying.is_some()); for enumerator in enum_def.enumerators() { - if enumerator.associated_fields().is_some() { - Diagnostic::new(Error::EnumeratorCannotDeclareAssociatedFields { + if enumerator.fields.is_some() { + Diagnostic::new(Error::EnumeratorCannotContainFields { enumerator_identifier: enumerator.identifier().to_owned(), }) .set_span(enumerator.span()) diff --git a/src/visitor.rs b/src/visitor.rs index d101bd45..e07bc17d 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -261,8 +261,8 @@ impl Enumerator { /// This function delegates to `visitor.visit_enumerator`. pub fn visit_with(&self, visitor: &mut impl Visitor) { visitor.visit_enumerator(self); - if let Some(associated_fields) = &self.associated_fields { - for field in associated_fields { + if let Some(fields) = &self.fields { + for field in fields { field.borrow().visit_with(visitor); } } diff --git a/tests/dictionaries/key_type.rs b/tests/dictionaries/key_type.rs index 27a67e5d..245fb68c 100644 --- a/tests/dictionaries/key_type.rs +++ b/tests/dictionaries/key_type.rs @@ -91,7 +91,8 @@ fn collections_are_disallowed(key_type: &str, key_kind: &str) { check_diagnostics(diagnostics, [expected]); } -#[test_case("MyEnum", "unchecked enum MyEnum: int8 {}" ; "enums")] +#[test_case("MyEnum", "enum MyEnum: int8 { A }" ; "simple enums")] +#[test_case("MyEnum", "unchecked enum MyEnum: int8 {}" ; "unchecked simple enums")] #[test_case("MyCustom", "custom MyCustom" ; "custom_types")] fn allowed_constructed_types(key_type: &str, key_type_def: &str) { // Arrange @@ -107,15 +108,17 @@ fn allowed_constructed_types(key_type: &str, key_type_def: &str) { assert_parses(slice); } -#[test_case("MyClass", "class", "Slice1"; "classes")] -fn disallowed_constructed_types(key_type: &str, key_kind: &str, mode: &str) { +#[test_case("MyEnum", "enum MyEnum { A }", "enum", "Slice2" ; "enums")] +#[test_case("MyEnum", "unchecked enum MyEnum {}", "enum", "Slice2" ; "unchecked enums")] +#[test_case("MyClass", "class MyClass {}", "class", "Slice1"; "classes")] +fn disallowed_constructed_types(key_type: &str, key_type_def: &str, key_kind: &str, mode: &str) { // Arrange let slice = format!( " mode = {mode} module Test - {key_kind} {key_type} {{}} + {key_type_def} typealias Dict = Dictionary<{key_type}, uint8> " ); diff --git a/tests/enums/container.rs b/tests/enums/container.rs index 83ec4d76..59a822d5 100644 --- a/tests/enums/container.rs +++ b/tests/enums/container.rs @@ -92,19 +92,19 @@ mod associated_fields { // Assert let a = ast.find_element::("Test::E::A").unwrap(); assert!(matches!(a.value, EnumeratorValue::Implicit(0))); - assert!(a.associated_fields().is_none()); + assert!(a.fields.is_none()); let b = ast.find_element::("Test::E::B").unwrap(); assert!(matches!(b.value, EnumeratorValue::Implicit(1))); - assert!(b.associated_fields().unwrap().len() == 1); + assert!(b.fields.as_ref().unwrap().len() == 1); let c = ast.find_element::("Test::E::C").unwrap(); assert!(matches!(c.value, EnumeratorValue::Implicit(2))); - assert!(c.associated_fields().unwrap().len() == 2); + assert!(c.fields.as_ref().unwrap().len() == 2); let d = ast.find_element::("Test::E::D").unwrap(); assert!(matches!(d.value, EnumeratorValue::Implicit(3))); - assert!(d.associated_fields().unwrap().len() == 0); + assert!(d.fields.as_ref().unwrap().len() == 0); } #[test_case("unchecked enum", true ; "unchecked")] @@ -207,7 +207,7 @@ mod underlying_type { let diagnostics = parse_for_diagnostics(slice); // Assert - let expected = Diagnostic::new(Error::EnumeratorCannotDeclareAssociatedFields { + let expected = Diagnostic::new(Error::EnumeratorCannotContainFields { enumerator_identifier: "B".to_owned(), }); check_diagnostics(diagnostics, [expected]); diff --git a/tests/enums/mode_compatibility.rs b/tests/enums/mode_compatibility.rs index 1e27bded..264a4212 100644 --- a/tests/enums/mode_compatibility.rs +++ b/tests/enums/mode_compatibility.rs @@ -54,7 +54,7 @@ mod slice1 { check_diagnostics(diagnostics, [expected]); } - /// Verifies that associated fields are disallowed in Slice1 mode. + /// Verifies that enumerators with fields are disallowed in Slice1 mode. #[test] fn associated_fields_fail() { // Arrange @@ -77,13 +77,13 @@ mod slice1 { mode: CompilationMode::Slice1, }) .add_note( - "enumerators declared in Slice1 mode cannot have associated fields", + "field syntax cannot be used with enumerators declared in Slice1 mode", None, ); check_diagnostics(diagnostics, [expected]); } - /// Verifies that even empty associated field lists are disallowed in Slice1 mode. + /// Verifies that even empty field lists are disallowed in Slice1 mode. #[test] fn empty_associated_fields_fail() { // Arrange @@ -106,7 +106,7 @@ mod slice1 { mode: CompilationMode::Slice1, }) .add_note( - "enumerators declared in Slice1 mode cannot have associated fields", + "field syntax cannot be used with enumerators declared in Slice1 mode", None, ); check_diagnostics(diagnostics, [expected]);