Skip to content

Commit

Permalink
Don't add values to enums that can't be parsed (#1836)
Browse files Browse the repository at this point in the history 
* Don't add values to enums that can't be parsed

* Fix test case

* Oops
  • Loading branch information
@Saalvage
Saalvage committed Mar 14, 2024
1 parent f2f48dc commit 81e0a93
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 37 deletions.
61 changes: 36 additions & 25 deletions src/Generator/Library.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,22 +103,29 @@ public static Enumeration GetEnumWithMatchingItem(this ASTContext context, strin
/// as well as all previously discovered <see cref="Enumeration.Items"/>. The intent is to
/// preserve sign information about the values held in <paramref name="enum"/>.
/// </remarks>
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)
Expand Down Expand Up @@ -154,7 +161,7 @@ static object EvaluateEnumExpression(string expr, Enumeration @enum)
/// evaluator biases towards returning <see cref="int"/> values: it doesn't attempt to
/// discover that a value could be stored in a <see cref="byte"/>.
/// </returns>
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;

Expand All @@ -171,38 +178,40 @@ 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.

Diagnostics.PushIndent();
Diagnostics.Warning($"Unable to translate macro '{macro.Name}' to en enum value: {ex.Message}");
Diagnostics.PopIndent();

return (0, PrimitiveType.Int);
item = default;
return false;
}
}

Expand Down Expand Up @@ -311,16 +320,18 @@ TranslationUnit GetUnitFromItems(List<Enumeration.Item> 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.
Expand Down
23 changes: 11 additions & 12 deletions tests/dotnet/CSharp/CSharp.Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit 81e0a93

Please sign in to comment.