Allow Settings Explicit Discriminants for Enums with Fields

src/diagnostics/errors.rs

@@ -74,11 +74,6 @@ pub enum Error {
         enumerator_value: i128,
     },
 
-    /// Enumerators cannot declare explicit values when their enclosing enum doesn't have an underlying type.
-    EnumeratorCannotDeclareExplicitValue {
-        enumerator_identifier: String,
-    },
-
     /// Enumerators cannot contain fields when their enclosing enum has an underlying type.
     EnumeratorCannotContainFields {
         enumerator_identifier: String,
@@ -553,12 +548,6 @@ implement_diagnostic_functions!(
         ExceptionSpecificationNotSupported,
         "exceptions can only be thrown by operations defined in Slice1 mode"
     ),
-    (
-        "E053",
-        EnumeratorCannotDeclareExplicitValue,
-        format!("invalid enumerator '{enumerator_identifier}': explicit values can only be declared within enums that specify an underlying type"),
-        enumerator_identifier
-    ),
     (
         "E054",
         EnumeratorCannotContainFields,

src/validators/enums.rs

@@ -14,14 +14,9 @@ pub fn validate_enum(enum_def: &Enum, diagnostics: &mut Diagnostics) {
     check_compact_modifier(enum_def, diagnostics);
     compact_enums_cannot_contain_tags(enum_def, diagnostics);
 
-    // 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_fields(enum_def, diagnostics);
-        } else {
-            cannot_contain_explicit_values(enum_def, diagnostics);
-        }
+    // Fields in Slice1 files are already rejected by `encoding_patcher`.
+    if enum_def.underlying.is_some() && !enum_def.supported_encodings().supports(Encoding::Slice1) {
+        cannot_contain_fields(enum_def, diagnostics);
     }
 }
 
@@ -44,9 +39,8 @@ fn backing_type_bounds(enum_def: &Enum, diagnostics: &mut Diagnostics) {
         }
     } else {
         // Enum was defined in a Slice2 file.
-        // Non-integrals are handled by `allowed_underlying_types`
-        fn check_bounds(enum_def: &Enum, underlying_type: &Primitive, diagnostics: &mut Diagnostics) {
-            let (min, max) = underlying_type.numeric_bounds().unwrap();
+
+        fn check_bounds(enum_def: &Enum, (min, max): (i128, i128), diagnostics: &mut Diagnostics) {
             enum_def
                 .enumerators()
                 .iter()
@@ -65,13 +59,15 @@ fn backing_type_bounds(enum_def: &Enum, diagnostics: &mut Diagnostics) {
         }
         match &enum_def.underlying {
             Some(underlying_type) => {
-                if underlying_type.is_integral() {
-                    check_bounds(enum_def, underlying_type, diagnostics);
+                // Non-integral underlying types are rejected by the `allowed_underlying_types` check.
+                if let Some(bounds) = underlying_type.numeric_bounds() {
+                    check_bounds(enum_def, bounds, diagnostics);
                 }
             }
             None => {
-                // No underlying type, the default is varint32 for Slice2.
-                check_bounds(enum_def, &Primitive::VarInt32, diagnostics);
+                // For enumerators in Slice2, values must fit within varint32 and be positive.
+                const VARINT32_MAX: i128 = i32::MAX as i128;
+                check_bounds(enum_def, (0, VARINT32_MAX), diagnostics);
             }
         }
     }
@@ -160,22 +156,6 @@ fn cannot_contain_fields(enum_def: &Enum, diagnostics: &mut Diagnostics) {
     }
 }
 
-/// Validate that this enum's enumerators don't specify any explicit values.
-/// This function should only be called for enums without underlying types.
-fn cannot_contain_explicit_values(enum_def: &Enum, diagnostics: &mut Diagnostics) {
-    debug_assert!(enum_def.underlying.is_none());
-
-    for enumerator in enum_def.enumerators() {
-        if matches!(enumerator.value, EnumeratorValue::Explicit(_)) {
-            Diagnostic::new(Error::EnumeratorCannotDeclareExplicitValue {
-                enumerator_identifier: enumerator.identifier().to_owned(),
-            })
-            .set_span(enumerator.span())
-            .push_into(diagnostics);
-        }
-    }
-}
-
 /// Validate that compact enums do not have an underlying type and are not marked as 'unchecked'.
 fn check_compact_modifier(enum_def: &Enum, diagnostics: &mut Diagnostics) {
     if enum_def.is_compact {

tests/enums/container.rs

@@ -71,25 +71,86 @@ mod associated_fields {
     }
 
     #[test]
-    fn explicit_values_are_not_allowed() {
+    fn explicit_values_are_allowed() {
         // Arrange
         let slice = "
             module Test
             enum E {
                 A
                 B = 7
-                C
+                C(a: int8)
+                D(b: bool) = 4
             }
         ";
 
         // Act
-        let diagnostics = parse_for_diagnostics(slice);
+        let ast = parse_for_ast(slice);
 
         // Assert
-        let expected = Diagnostic::new(Error::EnumeratorCannotDeclareExplicitValue {
-            enumerator_identifier: "B".to_owned(),
-        });
-        check_diagnostics(diagnostics, [expected]);
+        let enumerator_a = ast.find_element::<Enumerator>("Test::E::A").unwrap();
+        assert!(matches!(enumerator_a.value, EnumeratorValue::Implicit(0)));
+        assert_eq!(enumerator_a.value(), 0);
+
+        let enumerator_b = ast.find_element::<Enumerator>("Test::E::B").unwrap();
+        assert!(matches!(enumerator_b.value, EnumeratorValue::Explicit(_)));
+        assert_eq!(enumerator_b.value(), 7);
+
+        let enumerator_c = ast.find_element::<Enumerator>("Test::E::C").unwrap();
+        assert!(matches!(enumerator_c.value, EnumeratorValue::Implicit(8)));
+        assert_eq!(enumerator_c.value(), 8);
+
+        let enumerator_d = ast.find_element::<Enumerator>("Test::E::D").unwrap();
+        assert!(matches!(enumerator_d.value, EnumeratorValue::Explicit(_)));
+        assert_eq!(enumerator_d.value(), 4);
+    }
+
+    #[test]
+    fn explicit_values_must_be_within_range() {
+        // Arrange
+        let slice = "
+            module Test
+            enum E {
+                ImplicitOkay                           //  0
+                ExplicitNegative = -3                  // -3
+                ImplicitNegative(tag(4) s: string?)    // -2
+                Okay(b: bool) = 2_147_483_647          // 2_147_483_647
+                ImplicitOverflow                       // 2_147_483_648
+                ExplicitOverflow = 0x686921203A7629    // something big
+                ExplicitOkay(a: int8) = 79             // 79
+            }
+        ";
+
+        // Act
+        let diagnostics = parse_for_diagnostics(slice);
+
+        // Arrange
+        let expected = [
+            Diagnostic::new(Error::EnumeratorValueOutOfBounds {
+                enumerator_identifier: "ExplicitNegative".to_owned(),
+                value: -3,
+                min: 0,
+                max: i32::MAX as i128,
+            }),
+            Diagnostic::new(Error::EnumeratorValueOutOfBounds {
+                enumerator_identifier: "ImplicitNegative".to_owned(),
+                value: -2,
+                min: 0,
+                max: i32::MAX as i128,
+            }),
+            Diagnostic::new(Error::EnumeratorValueOutOfBounds {
+                enumerator_identifier: "ImplicitOverflow".to_owned(),
+                value: 2_147_483_648,
+                min: 0,
+                max: i32::MAX as i128,
+            }),
+            Diagnostic::new(Error::EnumeratorValueOutOfBounds {
+                enumerator_identifier: "ExplicitOverflow".to_owned(),
+                value: 0x686921203A7629,
+                min: 0,
+                max: i32::MAX as i128,
+            }),
+        ];
+        check_diagnostics(diagnostics, expected);
     }
 
     #[test]