From 790ea912261c2ebcca785753b71e45dd3f21e45f Mon Sep 17 00:00:00 2001 From: Salvage <29021710+Saalvage@users.noreply.github.com> Date: Wed, 13 Mar 2024 19:03:20 +0100 Subject: [PATCH 1/3] Don't add values to enums that can't be parsed --- src/Generator/Library.cs | 58 ++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/src/Generator/Library.cs b/src/Generator/Library.cs index 1c866235e4..9a77b4c87a 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,25 +178,27 @@ 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) { @@ -202,7 +211,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 +321,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. From 8524aac1a5098c6898115e5862c6e6c3f064111b Mon Sep 17 00:00:00 2001 From: Salvage <29021710+Saalvage@users.noreply.github.com> Date: Thu, 14 Mar 2024 03:07:29 +0100 Subject: [PATCH 2/3] Fix test case --- src/Generator/Library.cs | 3 +-- tests/dotnet/CSharp/CSharp.Tests.cs | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/Generator/Library.cs b/src/Generator/Library.cs index 9a77b4c87a..403c2e85e9 100644 --- a/src/Generator/Library.cs +++ b/src/Generator/Library.cs @@ -202,8 +202,7 @@ static bool TryParseEnumItemMacroExpression(MacroDefinition macro, Enumeration @ } 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. diff --git a/tests/dotnet/CSharp/CSharp.Tests.cs b/tests/dotnet/CSharp/CSharp.Tests.cs index f61e97ef62..5f4ad2ae2b 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)(b | c); + var e = (MyMacroTest2Enum)(b | c | d); + 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] From 935edc3225c58c6b3177ba3cd3a2c7220d2c2c02 Mon Sep 17 00:00:00 2001 From: Salvage <29021710+Saalvage@users.noreply.github.com> Date: Thu, 14 Mar 2024 03:19:36 +0100 Subject: [PATCH 3/3] Oops --- tests/dotnet/CSharp/CSharp.Tests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/dotnet/CSharp/CSharp.Tests.cs b/tests/dotnet/CSharp/CSharp.Tests.cs index 5f4ad2ae2b..9558ff10d6 100644 --- a/tests/dotnet/CSharp/CSharp.Tests.cs +++ b/tests/dotnet/CSharp/CSharp.Tests.cs @@ -1527,8 +1527,8 @@ public void TestMyMacro2TestEnum() var a = (MyMacroTest2Enum)1; var b = (MyMacroTest2Enum)0x2; var c = (MyMacroTest2Enum)(1 << 2); - var d = (MyMacroTest2Enum)(b | c); - var e = (MyMacroTest2Enum)(b | c | d); + 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 &&