Skip to content

Commit

Permalink
C#: Cleanup Variant marshaling code in source/bindings generators
Browse files Browse the repository at this point in the history
This change aims to reduce the number of places that need to be changed
when adding or editing a Godot type to the bindings.

Since the addition of `Variant.From<T>/As<T>` and
`VariantUtils.CreateFrom<T>/ConvertTo<T>`, we can now replace a lot of
the previous code in the bindings generator and the source generators
that specify these conversions for each type manually.

The only exceptions are the generic Godot collections (`Array<T>` and
`Dictionary<TKey, TValue>`) which still use the old version, as that
one cannot be matched by our new conversion methods (limitation in the
language with generics, forcing us to use delegate pointers).

The cleanup applies to:

- Bindings generator:
  - `TypeInterface.cs_variant_to_managed`
  - `TypeInterface.cs_managed_to_variant`
- Source generators:
  - `MarshalUtils.AppendNativeVariantToManagedExpr`
  - `MarshalUtils.AppendManagedToNativeVariantExpr`
  - `MarshalUtils.AppendVariantToManagedExpr`
  - `MarshalUtils.AppendManagedToVariantExpr`
  • Loading branch information
neikeq committed Dec 2, 2022
1 parent f86c6b6 commit 17b2838
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 415 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,9 @@ MarshalUtils.TypeCache typeCache
if (parameters.Length > paramTypes.Length)
return null; // Ignore incompatible method

return new GodotMethodData(method, paramTypes, parameters
.Select(p => p.Type).ToImmutableArray(), retType, retSymbol);
return new GodotMethodData(method, paramTypes,
parameters.Select(p => p.Type).ToImmutableArray(),
retType != null ? (retType.Value, retSymbol) : null);
}

public static IEnumerable<GodotMethodData> WhereHasGodotCompatibleSignature(
Expand Down Expand Up @@ -330,10 +331,10 @@ MarshalUtils.TypeCache typeCache

public static string Path(this Location location)
=> location.SourceTree?.GetLineSpan(location.SourceSpan).Path
?? location.GetLineSpan().Path;
?? location.GetLineSpan().Path;

public static int StartLine(this Location location)
=> location.SourceTree?.GetLineSpan(location.SourceSpan).StartLinePosition.Line
?? location.GetLineSpan().StartLinePosition.Line;
?? location.GetLineSpan().StartLinePosition.Line;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,18 @@ namespace Godot.SourceGenerators
public readonly struct GodotMethodData
{
public GodotMethodData(IMethodSymbol method, ImmutableArray<MarshalType> paramTypes,
ImmutableArray<ITypeSymbol> paramTypeSymbols, MarshalType? retType, ITypeSymbol? retSymbol)
ImmutableArray<ITypeSymbol> paramTypeSymbols, (MarshalType MarshalType, ITypeSymbol TypeSymbol)? retType)
{
Method = method;
ParamTypes = paramTypes;
ParamTypeSymbols = paramTypeSymbols;
RetType = retType;
RetSymbol = retSymbol;
}

public IMethodSymbol Method { get; }
public ImmutableArray<MarshalType> ParamTypes { get; }
public ImmutableArray<ITypeSymbol> ParamTypeSymbols { get; }
public MarshalType? RetType { get; }
public ITypeSymbol? RetSymbol { get; }
public (MarshalType MarshalType, ITypeSymbol TypeSymbol)? RetType { get; }
}

public readonly struct GodotSignalDelegateData
Expand Down
374 changes: 30 additions & 344 deletions modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/MarshalUtils.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ INamedTypeSymbol symbol

source.Append("#pragma warning disable CS0109 // Disable warning about redundant 'new' keyword\n");

source.Append($" public new class MethodName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.MethodName {{\n");
source.Append(
$" public new class MethodName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.MethodName {{\n");

// Generate cached StringNames for methods and properties, for fast lookup

Expand Down Expand Up @@ -297,7 +298,7 @@ private static MethodInfo DetermineMethodInfo(GodotMethodData method)

if (method.RetType != null)
{
returnVal = DeterminePropertyInfo(method.RetType.Value, name: string.Empty);
returnVal = DeterminePropertyInfo(method.RetType.Value.MarshalType, name: string.Empty);
}
else
{
Expand Down Expand Up @@ -391,7 +392,8 @@ StringBuilder source
{
source.Append(" ret = ");

source.AppendManagedToNativeVariantExpr("callRet", method.RetType.Value);
source.AppendManagedToNativeVariantExpr("callRet",
method.RetType.Value.TypeSymbol, method.RetType.Value.MarshalType);
source.Append(";\n");

source.Append(" return true;\n");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ INamedTypeSymbol symbol

source.Append("#pragma warning disable CS0109 // Disable warning about redundant 'new' keyword\n");

source.Append($" public new class PropertyName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.PropertyName {{\n");
source.Append(
$" public new class PropertyName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.PropertyName {{\n");

// Generate cached StringNames for methods and properties, for fast lookup

Expand Down Expand Up @@ -199,14 +200,14 @@ INamedTypeSymbol symbol
foreach (var property in godotClassProperties)
{
GeneratePropertyGetter(property.PropertySymbol.Name,
property.Type, source, isFirstEntry);
property.PropertySymbol.Type, property.Type, source, isFirstEntry);
isFirstEntry = false;
}

foreach (var field in godotClassFields)
{
GeneratePropertyGetter(field.FieldSymbol.Name,
field.Type, source, isFirstEntry);
field.FieldSymbol.Type, field.Type, source, isFirstEntry);
isFirstEntry = false;
}

Expand Down Expand Up @@ -303,6 +304,7 @@ bool isFirstEntry

private static void GeneratePropertyGetter(
string propertyMemberName,
ITypeSymbol propertyTypeSymbol,
MarshalType propertyMarshalType,
StringBuilder source,
bool isFirstEntry
Expand All @@ -317,7 +319,8 @@ bool isFirstEntry
.Append(propertyMemberName)
.Append(") {\n")
.Append(" value = ")
.AppendManagedToNativeVariantExpr("this." + propertyMemberName, propertyMarshalType)
.AppendManagedToNativeVariantExpr("this." + propertyMemberName,
propertyTypeSymbol, propertyMarshalType)
.Append(";\n")
.Append(" return true;\n")
.Append(" }\n");
Expand Down Expand Up @@ -376,7 +379,8 @@ private static IEnumerable<PropertyInfo> DetermineGroupingPropertyInfo(ISymbol m
if (propertyUsage != PropertyUsageFlags.Category && attr.ConstructorArguments.Length > 1)
hintString = attr.ConstructorArguments[1].Value?.ToString();

yield return new PropertyInfo(VariantType.Nil, name, PropertyHint.None, hintString, propertyUsage.Value, true);
yield return new PropertyInfo(VariantType.Nil, name, PropertyHint.None, hintString,
propertyUsage.Value, true);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ INamedTypeSymbol symbol
source.Append(" values.Add(PropertyName.");
source.Append(exportedMember.Name);
source.Append(", ");
source.AppendManagedToVariantExpr(defaultValueLocalName, exportedMember.Type);
source.AppendManagedToVariantExpr(defaultValueLocalName,
exportedMember.TypeSymbol, exportedMember.Type);
source.Append(");\n");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ INamedTypeSymbol symbol
source.Append(" info.AddProperty(PropertyName.")
.Append(propertyName)
.Append(", ")
.AppendManagedToVariantExpr(string.Concat("this.", propertyName), property.Type)
.AppendManagedToVariantExpr(string.Concat("this.", propertyName),
property.PropertySymbol.Type, property.Type)
.Append(");\n");
}

Expand All @@ -175,7 +176,8 @@ INamedTypeSymbol symbol
source.Append(" info.AddProperty(PropertyName.")
.Append(fieldName)
.Append(", ")
.AppendManagedToVariantExpr(string.Concat("this.", fieldName), field.Type)
.AppendManagedToVariantExpr(string.Concat("this.", fieldName),
field.FieldSymbol.Type, field.Type)
.Append(");\n");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ INamedTypeSymbol symbol

source.Append("#pragma warning disable CS0109 // Disable warning about redundant 'new' keyword\n");

source.Append($" public new class SignalName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.SignalName {{\n");
source.Append(
$" public new class SignalName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.SignalName {{\n");

// Generate cached StringNames for methods and properties, for fast lookup

Expand Down Expand Up @@ -236,7 +237,8 @@ INamedTypeSymbol symbol
.Append(signalName)
.Append(";\n");

source.Append($" /// <inheritdoc cref=\"{signalDelegate.DelegateSymbol.FullQualifiedNameIncludeGlobal()}\"/>\n");
source.Append(
$" /// <inheritdoc cref=\"{signalDelegate.DelegateSymbol.FullQualifiedNameIncludeGlobal()}\"/>\n");

source.Append(" public event ")
.Append(signalDelegate.DelegateSymbol.FullQualifiedNameIncludeGlobal())
Expand Down Expand Up @@ -351,7 +353,7 @@ private static MethodInfo DetermineMethodInfo(GodotSignalDelegateData signalDele

if (invokeMethodData.RetType != null)
{
returnVal = DeterminePropertyInfo(invokeMethodData.RetType.Value, name: string.Empty);
returnVal = DeterminePropertyInfo(invokeMethodData.RetType.Value.MarshalType, name: string.Empty);
}
else
{
Expand Down
61 changes: 14 additions & 47 deletions modules/mono/editor/bindings_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2837,9 +2837,6 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
itype.is_ref_counted = ClassDB::is_parent_class(type_cname, name_cache.type_RefCounted);
itype.memory_own = itype.is_ref_counted;

itype.cs_variant_to_managed = "(%1)VariantUtils.ConvertToGodotObject(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromGodotObject(%0)";

itype.c_out = "%5return ";
itype.c_out += C_METHOD_UNMANAGED_GET_MANAGED;
itype.c_out += itype.is_ref_counted ? "(%1.Reference);\n" : "(%1);\n";
Expand Down Expand Up @@ -3218,8 +3215,6 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
enum_itype.cname = StringName(enum_itype.name);
enum_itype.proxy_name = itype.proxy_name + "." + enum_proxy_name;
TypeInterface::postsetup_enum_type(enum_itype);
enum_itype.cs_variant_to_managed = "(%1)VariantUtils.ConvertToInt32(%0)";
enum_itype.cs_managed_to_variant = "VariantUtils.CreateFromInt((int)%0)";
enum_types.insert(enum_itype.cname, enum_itype);
}

Expand Down Expand Up @@ -3448,16 +3443,14 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {

TypeInterface itype;

#define INSERT_STRUCT_TYPE(m_type) \
{ \
itype = TypeInterface::create_value_type(String(#m_type)); \
itype.c_type_in = #m_type "*"; \
itype.c_type_out = itype.cs_type; \
itype.cs_in_expr = "&%0"; \
itype.cs_in_expr_is_unsafe = true; \
itype.cs_variant_to_managed = "VariantUtils.ConvertTo%2(%0)"; \
itype.cs_managed_to_variant = "VariantUtils.CreateFrom%2(%0)"; \
builtin_types.insert(itype.cname, itype); \
#define INSERT_STRUCT_TYPE(m_type) \
{ \
itype = TypeInterface::create_value_type(String(#m_type)); \
itype.c_type_in = #m_type "*"; \
itype.c_type_out = itype.cs_type; \
itype.cs_in_expr = "&%0"; \
itype.cs_in_expr_is_unsafe = true; \
builtin_types.insert(itype.cname, itype); \
}

INSERT_STRUCT_TYPE(Vector2)
Expand Down Expand Up @@ -3488,8 +3481,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_out = itype.c_type;
itype.c_arg_in = "&%s";
itype.c_in_vararg = "%5using godot_variant %1_in = VariantUtils.CreateFromBool(%1);\n";
itype.cs_variant_to_managed = "VariantUtils.ConvertToBool(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromBool(%0)";
builtin_types.insert(itype.cname, itype);

// Integer types
Expand All @@ -3510,8 +3501,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_in = itype.name; \
itype.c_type_out = itype.name; \
itype.c_in_vararg = "%5using godot_variant %1_in = VariantUtils.CreateFromInt(%1);\n"; \
itype.cs_variant_to_managed = "VariantUtils.ConvertTo" m_int_struct_name "(%0)"; \
itype.cs_managed_to_variant = "VariantUtils.CreateFromInt(%0)"; \
builtin_types.insert(itype.cname, itype); \
}

Expand Down Expand Up @@ -3547,8 +3536,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_in = itype.proxy_name;
itype.c_type_out = itype.proxy_name;
itype.c_in_vararg = "%5using godot_variant %1_in = VariantUtils.CreateFromFloat(%1);\n";
itype.cs_variant_to_managed = "VariantUtils.ConvertToFloat32(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromFloat(%0)";
builtin_types.insert(itype.cname, itype);

// double
Expand All @@ -3562,8 +3549,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_in = itype.proxy_name;
itype.c_type_out = itype.proxy_name;
itype.c_in_vararg = "%5using godot_variant %1_in = VariantUtils.CreateFromFloat(%1);\n";
itype.cs_variant_to_managed = "VariantUtils.ConvertToFloat64(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromFloat(%0)";
builtin_types.insert(itype.cname, itype);
}

Expand All @@ -3581,8 +3566,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_out = itype.cs_type;
itype.c_type_is_disposable_struct = true;
itype.c_in_vararg = "%5using godot_variant %1_in = VariantUtils.CreateFromString(%1);\n";
itype.cs_variant_to_managed = "VariantUtils.ConvertToStringObject(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromString(%0)";
builtin_types.insert(itype.cname, itype);

// StringName
Expand All @@ -3601,8 +3584,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_in_vararg = "%5using godot_variant %1_in = VariantUtils.CreateFromStringName(%1);\n";
itype.c_type_is_disposable_struct = false; // [c_out] takes ownership
itype.c_ret_needs_default_initialization = true;
itype.cs_variant_to_managed = "VariantUtils.ConvertToStringNameObject(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromStringName(%0)";
builtin_types.insert(itype.cname, itype);

// NodePath
Expand All @@ -3620,8 +3601,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_out = itype.cs_type;
itype.c_type_is_disposable_struct = false; // [c_out] takes ownership
itype.c_ret_needs_default_initialization = true;
itype.cs_variant_to_managed = "VariantUtils.ConvertToNodePathObject(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromNodePath(%0)";
builtin_types.insert(itype.cname, itype);

// RID
Expand All @@ -3634,8 +3613,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type = itype.cs_type;
itype.c_type_in = itype.c_type;
itype.c_type_out = itype.c_type;
itype.cs_variant_to_managed = "VariantUtils.ConvertToRID(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromRID(%0)";
builtin_types.insert(itype.cname, itype);

// Variant
Expand All @@ -3652,8 +3629,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_out = itype.cs_type;
itype.c_type_is_disposable_struct = false; // [c_out] takes ownership
itype.c_ret_needs_default_initialization = true;
itype.cs_variant_to_managed = "Variant.CreateCopyingBorrowed(%0)";
itype.cs_managed_to_variant = "%0.CopyNativeVariant()";
builtin_types.insert(itype.cname, itype);

// Callable
Expand All @@ -3666,8 +3641,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_in = "in " + itype.cs_type;
itype.c_type_out = itype.cs_type;
itype.c_type_is_disposable_struct = true;
itype.cs_variant_to_managed = "VariantUtils.ConvertToCallableManaged(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromCallable(%0)";
builtin_types.insert(itype.cname, itype);

// Signal
Expand All @@ -3684,8 +3657,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_in = "in " + itype.cs_type;
itype.c_type_out = itype.cs_type;
itype.c_type_is_disposable_struct = true;
itype.cs_variant_to_managed = "VariantUtils.ConvertToSignalInfo(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromSignalInfo(%0)";
builtin_types.insert(itype.cname, itype);

// VarArg (fictitious type to represent variable arguments)
Expand Down Expand Up @@ -3715,8 +3686,6 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_in = itype.proxy_name; \
itype.c_type_out = itype.proxy_name; \
itype.c_type_is_disposable_struct = true; \
itype.cs_variant_to_managed = "VariantUtils.ConvertAs%2ToSystemArray(%0)"; \
itype.cs_managed_to_variant = "VariantUtils.CreateFrom%2(%0)"; \
builtin_types.insert(itype.name, itype); \
}

Expand Down Expand Up @@ -3752,15 +3721,16 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_out = itype.cs_type;
itype.c_type_is_disposable_struct = false; // [c_out] takes ownership
itype.c_ret_needs_default_initialization = true;
itype.cs_variant_to_managed = "VariantUtils.ConvertToArrayObject(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromArray(%0)";
builtin_types.insert(itype.cname, itype);

// Array_@generic
// Re-use Array's itype
itype.name = "Array_@generic";
itype.cname = itype.name;
itype.cs_out = "%5return new %2(%0(%1));";
// For generic Godot collections, Variant.From<T>/As<T> is slower, so we need this special case
itype.cs_variant_to_managed = "VariantUtils.ConvertToArrayObject(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromArray(%0)";
builtin_types.insert(itype.cname, itype);

// Dictionary
Expand All @@ -3778,15 +3748,16 @@ void BindingsGenerator::_populate_builtin_type_interfaces() {
itype.c_type_out = itype.cs_type;
itype.c_type_is_disposable_struct = false; // [c_out] takes ownership
itype.c_ret_needs_default_initialization = true;
itype.cs_variant_to_managed = "VariantUtils.ConvertToDictionaryObject(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromDictionary(%0)";
builtin_types.insert(itype.cname, itype);

// Dictionary_@generic
// Re-use Dictionary's itype
itype.name = "Dictionary_@generic";
itype.cname = itype.name;
itype.cs_out = "%5return new %2(%0(%1));";
// For generic Godot collections, Variant.From<T>/As<T> is slower, so we need this special case
itype.cs_variant_to_managed = "VariantUtils.ConvertToDictionaryObject(%0)";
itype.cs_managed_to_variant = "VariantUtils.CreateFromDictionary(%0)";
builtin_types.insert(itype.cname, itype);

// void (fictitious type to represent the return type of methods that do not return anything)
Expand Down Expand Up @@ -3852,8 +3823,6 @@ void BindingsGenerator::_populate_global_constants() {
enum_itype.cname = ienum.cname;
enum_itype.proxy_name = enum_itype.name;
TypeInterface::postsetup_enum_type(enum_itype);
enum_itype.cs_variant_to_managed = "(%1)VariantUtils.ConvertToInt32(%0)";
enum_itype.cs_managed_to_variant = "VariantUtils.CreateFromInt((int)%0)";
enum_types.insert(enum_itype.cname, enum_itype);

int prefix_length = _determine_enum_prefix(ienum);
Expand Down Expand Up @@ -3886,8 +3855,6 @@ void BindingsGenerator::_populate_global_constants() {
enum_itype.cname = enum_cname;
enum_itype.proxy_name = enum_itype.name;
TypeInterface::postsetup_enum_type(enum_itype);
enum_itype.cs_variant_to_managed = "(%1)VariantUtils.ConvertToInt32(%0)";
enum_itype.cs_managed_to_variant = "VariantUtils.CreateFromInt((int)%0)";
enum_types.insert(enum_itype.cname, enum_itype);
}
}
Expand Down
Loading

0 comments on commit 17b2838

Please sign in to comment.