From 81e0a931cf6116951e37b0fa763e1974bc753a83 Mon Sep 17 00:00:00 2001 From: Stefan <29021710+Saalvage@users.noreply.github.com> Date: Thu, 14 Mar 2024 11:28:02 +0100 Subject: [PATCH] Don't add values to enums that can't be parsed (#1836) * Don't add values to enums that can't be parsed * Fix test case * Oops --- src/Generator/Library.cs | 61 +++++++++++++++++------------ tests/dotnet/CSharp/CSharp.Tests.cs | 23 ++++++----- 2 files changed, 47 insertions(+), 37 deletions(-) diff --git a/src/Generator/Library.cs b/src/Generator/Library.cs index 1c866235e4..403c2e85e9 100644 --- a/src/Generator/Library.cs +++ b/src/Generator/Library.cs @@ -103,22 +103,29 @@ public static Enumeration GetEnumWithMatchingItem(this ASTContext context, strin /// as well as all previously discovered . The intent is to /// preserve sign information about the values held in . /// - public static Enumeration.Item GenerateEnumItemFromMacro(this Enumeration @enum, - MacroDefinition macro) + public static bool TryGenerateEnumItemFromMacro(this Enumeration @enum, + MacroDefinition macro, out Enumeration.Item item) { - var (value, type) = ParseEnumItemMacroExpression(macro, @enum); + if (!TryParseEnumItemMacroExpression(macro, @enum, out var result)) + { + item = null; + return false; + } + + var (value, type) = result; if (type > @enum.BuiltinType.Type) { @enum.BuiltinType = new BuiltinType(type); @enum.Type = @enum.BuiltinType; } - return new Enumeration.Item + item = new Enumeration.Item { Name = macro.Name, Expression = macro.Expression, Value = value, Namespace = @enum }; + return true; } static object EvaluateEnumExpression(string expr, Enumeration @enum) @@ -154,7 +161,7 @@ static object EvaluateEnumExpression(string expr, Enumeration @enum) /// evaluator biases towards returning values: it doesn't attempt to /// discover that a value could be stored in a . /// - static (ulong value, PrimitiveType type) ParseEnumItemMacroExpression(MacroDefinition macro, Enumeration @enum) + static bool TryParseEnumItemMacroExpression(MacroDefinition macro, Enumeration @enum, out (ulong value, PrimitiveType type) item) { var expression = macro.Expression; @@ -171,30 +178,31 @@ static object EvaluateEnumExpression(string expr, Enumeration @enum) // Verify we have some sort of integer type. Enums can have negative values: record // that fact so that we can set the underlying primitive type correctly in the enum // item. - switch (System.Type.GetTypeCode(eval.GetType())) + item = System.Type.GetTypeCode(eval.GetType()) switch { // Must unbox eval which is typed as object to be able to cast it to a ulong. - case TypeCode.SByte: return ((ulong)(sbyte)eval, PrimitiveType.SChar); - case TypeCode.Int16: return ((ulong)(short)eval, PrimitiveType.Short); - case TypeCode.Int32: return ((ulong)(int)eval, PrimitiveType.Int); - case TypeCode.Int64: return ((ulong)(long)eval, PrimitiveType.LongLong); + TypeCode.SByte => ((ulong)(sbyte)eval, PrimitiveType.SChar), + TypeCode.Int16 => ((ulong)(short)eval, PrimitiveType.Short), + TypeCode.Int32 => ((ulong)(int)eval, PrimitiveType.Int), + TypeCode.Int64 => ((ulong)(long)eval, PrimitiveType.LongLong), - case TypeCode.Byte: return ((byte)eval, PrimitiveType.UChar); - case TypeCode.UInt16: return ((ushort)eval, PrimitiveType.UShort); - case TypeCode.UInt32: return ((uint)eval, PrimitiveType.UInt); - case TypeCode.UInt64: return ((ulong)eval, PrimitiveType.ULongLong); + TypeCode.Byte => ((byte)eval, PrimitiveType.UChar), + TypeCode.UInt16 => ((ushort)eval, PrimitiveType.UShort), + TypeCode.UInt32 => ((uint)eval, PrimitiveType.UInt), + TypeCode.UInt64 => ((ulong)eval, PrimitiveType.ULongLong), - case TypeCode.Char: return ((char)eval, PrimitiveType.UShort); + TypeCode.Char => ((char)eval, PrimitiveType.UShort), // C++ allows booleans as enum values - they're translated to 1, 0. - case TypeCode.Boolean: return ((bool)eval ? 1UL : 0UL, PrimitiveType.UChar); + TypeCode.Boolean => ((bool)eval ? 1UL : 0UL, PrimitiveType.UChar), - default: throw new Exception($"Expression {expression} is not a valid expression type for an enum"); - } + _ => throw new Exception($"Expression {expression} is not a valid expression type for an enum") + }; + + return true; } catch (Exception ex) { - // TODO: This should just throw, but we have a pre-existing behavior that expects malformed - // macro expressions to default to 0, see CSharp.h (MY_MACRO_TEST2_0), so do it for now. + // TODO: There should be a toggle to just throw here instead. // Like other paths, we can however, write a diagnostic message to the console. @@ -202,7 +210,8 @@ static object EvaluateEnumExpression(string expr, Enumeration @enum) Diagnostics.Warning($"Unable to translate macro '{macro.Name}' to en enum value: {ex.Message}"); Diagnostics.PopIndent(); - return (0, PrimitiveType.Int); + item = default; + return false; } } @@ -311,16 +320,18 @@ TranslationUnit GetUnitFromItems(List list) if (@enum.Items.Exists(it => it.Name == macro.Name)) continue; - var item = @enum.GenerateEnumItemFromMacro(macro); - @enum.AddItem(item); - macro.Enumeration = @enum; + if (@enum.TryGenerateEnumItemFromMacro(macro, out var item)) + { + @enum.AddItem(item); + macro.Enumeration = @enum; + } } if (@enum.Items.Count <= 0) return @enum; // The value of @enum.BuiltinType has been adjusted on the fly via - // GenerateEnumItemFromMacro. However, its notion of PrimitiveType corresponds with + // TryGenerateEnumItemFromMacro. However, its notion of PrimitiveType corresponds with // what the ExpressionEvaluator returns. In particular, the expression "1" will result // in PrimitiveType.Int from the expression evaluator. Narrow the type to account for // types narrower than int. diff --git a/tests/dotnet/CSharp/CSharp.Tests.cs b/tests/dotnet/CSharp/CSharp.Tests.cs index f61e97ef62..9558ff10d6 100644 --- a/tests/dotnet/CSharp/CSharp.Tests.cs +++ b/tests/dotnet/CSharp/CSharp.Tests.cs @@ -1524,18 +1524,17 @@ public void TestMyMacroTestEnum() [Test] public void TestMyMacro2TestEnum() { - var a = (MyMacroTest2Enum)0; - var b = (MyMacroTest2Enum)1; - var c = (MyMacroTest2Enum)0x2; - var d = (MyMacroTest2Enum)(1 << 2); - var e = (MyMacroTest2Enum)(b | c); - var f = (MyMacroTest2Enum)(b | c | d); - var g = (MyMacroTest2Enum)(1 << 3); - var h = (MyMacroTest2Enum)((1 << 4) - 1); - Assert.IsTrue(a == MyMacroTest2Enum.MY_MACRO_TEST2_0 && b == MyMacroTest2Enum.MY_MACRO_TEST2_1 && - c == MyMacroTest2Enum.MY_MACRO_TEST2_2 && d == MyMacroTest2Enum.MY_MACRO_TEST2_3 && - e == MyMacroTest2Enum.MY_MACRO_TEST2_1_2 && f == MyMacroTest2Enum.MY_MACRO_TEST2_1_2_3 && - g == MyMacroTest2Enum.MY_MACRO_TEST2_4 && h == MyMacroTest2Enum.MY_MACRO_TEST2ALL); + var a = (MyMacroTest2Enum)1; + var b = (MyMacroTest2Enum)0x2; + var c = (MyMacroTest2Enum)(1 << 2); + var d = (MyMacroTest2Enum)(a | b); + var e = (MyMacroTest2Enum)(a | b | c); + var f = (MyMacroTest2Enum)(1 << 3); + var g = (MyMacroTest2Enum)((1 << 4) - 1); + Assert.IsTrue(a == MyMacroTest2Enum.MY_MACRO_TEST2_1 && b == MyMacroTest2Enum.MY_MACRO_TEST2_2 && + c == MyMacroTest2Enum.MY_MACRO_TEST2_3 && d == MyMacroTest2Enum.MY_MACRO_TEST2_1_2 && + e == MyMacroTest2Enum.MY_MACRO_TEST2_1_2_3 && f == MyMacroTest2Enum.MY_MACRO_TEST2_4 && + g == MyMacroTest2Enum.MY_MACRO_TEST2ALL); } [Test]