diff --git a/src/ZeroC.Slice.Generator/BasicEnumGenerator.cs b/src/ZeroC.Slice.Generator/BasicEnumGenerator.cs index 68fd7f340d..143c904912 100644 --- a/src/ZeroC.Slice.Generator/BasicEnumGenerator.cs +++ b/src/ZeroC.Slice.Generator/BasicEnumGenerator.cs @@ -226,8 +226,7 @@ private static string GetArticle(string word) => private static bool NeedsHashSetValidation(BasicEnum enumDef) where T : struct, INumber { // If the enumerator count covers the full range of the underlying type, every value is valid - // and no validation is needed. This also prevents overflow when computing max - min below - // for small types like sbyte where MaxValue - MinValue (127 - (-128) = 255) overflows. + // and no validation is needed. int? bitSize = enumDef.Underlying.Kind switch { BuiltinKind.Int8 or BuiltinKind.UInt8 => 8, @@ -244,6 +243,10 @@ private static bool NeedsHashSetValidation(BasicEnum enumDef) where T : st T min = enumDef.Enumerators.Min(e => e.Value); T max = enumDef.Enumerators.Max(e => e.Value); - return T.CreateChecked(enumDef.Enumerators.Count - 1) < max - min; + // Promote to BigInteger so the subtraction doesn't wrap when min/max span (or near-span) the + // underlying type's full range — otherwise a sparse enum like `int8 { Min = -128, Max = 127 }` + // is misclassified as contiguous and the emitted range check accepts every underlying value. + BigInteger span = BigInteger.CreateChecked(max) - BigInteger.CreateChecked(min); + return enumDef.Enumerators.Count - 1 < span; } } diff --git a/src/ZeroC.Slice.Symbols/SymbolConverter.cs b/src/ZeroC.Slice.Symbols/SymbolConverter.cs index 4577e5d339..24e2828963 100644 --- a/src/ZeroC.Slice.Symbols/SymbolConverter.cs +++ b/src/ZeroC.Slice.Symbols/SymbolConverter.cs @@ -318,37 +318,42 @@ private ISymbol ConvertBasicEnum(Compiler.BasicEnum raw, Module module) raw, module, builtin, - (abs, isNegative) => isNegative ? (sbyte)-(long)abs : (sbyte)abs), + (abs, isNegative) => checked((sbyte)(isNegative ? -(long)abs : (long)abs))), BuiltinKind.UInt8 => CreateBasicEnum( raw, module, builtin, - (abs, _) => (byte)abs), + (abs, _) => checked((byte)abs)), BuiltinKind.Int16 => CreateBasicEnum( raw, module, builtin, - (abs, isNegative) => isNegative ? (short)-(long)abs : (short)abs), + (abs, isNegative) => checked((short)(isNegative ? -(long)abs : (long)abs))), BuiltinKind.UInt16 => CreateBasicEnum( raw, module, builtin, - (abs, _) => (ushort)abs), + (abs, _) => checked((ushort)abs)), BuiltinKind.Int32 or BuiltinKind.VarInt32 => CreateBasicEnum( raw, module, builtin, - (abs, isNegative) => isNegative ? (int)-(long)abs : (int)abs), + (abs, isNegative) => checked((int)(isNegative ? -(long)abs : (long)abs))), BuiltinKind.UInt32 or BuiltinKind.VarUInt32 => CreateBasicEnum( raw, module, builtin, - (abs, _) => (uint)abs), + (abs, _) => checked((uint)abs)), BuiltinKind.Int64 or BuiltinKind.VarInt62 => CreateBasicEnum( raw, module, builtin, - (abs, isNegative) => isNegative ? -(long)abs : (long)abs), + // slicec's descriptor format reports each enumerator as (AbsoluteValue: ulong, HasNegativeValue: bool). + // long.MinValue's absolute value is 2^63, which doesn't fit in long — handle it explicitly so the + // checked cast below doesn't throw on a legitimate value. + (abs, isNegative) => isNegative + ? (abs == (ulong)long.MaxValue + 1 ? long.MinValue : checked(-(long)abs)) + : checked((long)abs)), BuiltinKind.UInt64 or BuiltinKind.VarUInt62 => CreateBasicEnum( raw, module, diff --git a/tests/ZeroC.Slice.Generator.Tests/EnumTests.cs b/tests/ZeroC.Slice.Generator.Tests/EnumTests.cs index fe96474e2c..7293b8c059 100644 --- a/tests/ZeroC.Slice.Generator.Tests/EnumTests.cs +++ b/tests/ZeroC.Slice.Generator.Tests/EnumTests.cs @@ -46,6 +46,41 @@ public void As_enum_for_an_enum_with_contiguous_enumerators(int value, MyEnum ex public void As_enum_for_an_enum_with_contiguous_enumerators_fails_for_invalid_values(int value) => Assert.That(() => value.AsMyEnum(), Throws.TypeOf()); + // Boundary enums: declared min/max span the full underlying range. The valid enumerators decode, + // and intermediate values that are not declared must be rejected. + [TestCase((sbyte)-128, MyInt8BoundaryEnum.Min)] + [TestCase((sbyte)127, MyInt8BoundaryEnum.Max)] + public void As_enum_int8_boundary_accepts_declared_values(sbyte value, MyInt8BoundaryEnum expected) => + Assert.That(value.AsMyInt8BoundaryEnum(), Is.EqualTo(expected)); + + [TestCase((sbyte)-127)] + [TestCase((sbyte)0)] + [TestCase((sbyte)126)] + public void As_enum_int8_boundary_rejects_undeclared_values(sbyte value) => + Assert.That(() => value.AsMyInt8BoundaryEnum(), Throws.TypeOf()); + + [TestCase(int.MinValue, MyInt32BoundaryEnum.Min)] + [TestCase(int.MaxValue, MyInt32BoundaryEnum.Max)] + public void As_enum_int32_boundary_accepts_declared_values(int value, MyInt32BoundaryEnum expected) => + Assert.That(value.AsMyInt32BoundaryEnum(), Is.EqualTo(expected)); + + [TestCase(int.MinValue + 1)] + [TestCase(0)] + [TestCase(int.MaxValue - 1)] + public void As_enum_int32_boundary_rejects_undeclared_values(int value) => + Assert.That(() => value.AsMyInt32BoundaryEnum(), Throws.TypeOf()); + + [TestCase(long.MinValue, MyInt64BoundaryEnum.Min)] + [TestCase(long.MaxValue, MyInt64BoundaryEnum.Max)] + public void As_enum_int64_boundary_accepts_declared_values(long value, MyInt64BoundaryEnum expected) => + Assert.That(value.AsMyInt64BoundaryEnum(), Is.EqualTo(expected)); + + [TestCase(long.MinValue + 1)] + [TestCase(0L)] + [TestCase(long.MaxValue - 1)] + public void As_enum_int64_boundary_rejects_undeclared_values(long value) => + Assert.That(() => value.AsMyInt64BoundaryEnum(), Throws.TypeOf()); + [TestCase(sizeof(MyFixedSizeEnum), sizeof(short))] [TestCase(sizeof(MyUncheckedEnum), sizeof(uint))] public void Enum_has_the_expected_size(int size, int expectedSize) => diff --git a/tests/ZeroC.Slice.Generator.Tests/EnumTests.slice b/tests/ZeroC.Slice.Generator.Tests/EnumTests.slice index 2a39d4cb0e..207e29e9d8 100644 --- a/tests/ZeroC.Slice.Generator.Tests/EnumTests.slice +++ b/tests/ZeroC.Slice.Generator.Tests/EnumTests.slice @@ -32,3 +32,21 @@ unchecked enum MyUncheckedEnum : uint32 { } unchecked enum MyVarInt62Alias : varint62 {} + +// Sparse enums whose declared min/max span the full underlying range. Exercises both the BigInteger-based +// contiguity test (must classify these as sparse → HashSet validation) and the checked casts in the +// symbol converter (boundary values must round-trip without overflow). +enum MyInt8BoundaryEnum : int8 { + Min = -128 + Max = 127 +} + +enum MyInt32BoundaryEnum : int32 { + Min = -2147483648 + Max = 2147483647 +} + +enum MyInt64BoundaryEnum : int64 { + Min = -9223372036854775808 + Max = 9223372036854775807 +}