Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typemap cleanup #1802

Merged
merged 12 commits into from
Dec 7, 2023
Merged

Typemap cleanup #1802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6268b1c
TypeMap: prepare refactoring into a modular design
deadlocklogic Dec 3, 2023
8271415
TypeMap: refactor C++ backend into common methods
deadlocklogic Dec 3, 2023
e667b2c
TypeMap: refactor CLI backend into common methods
deadlocklogic Dec 4, 2023
473ecf9
CLI.Gen.cs: fix omitted typemap from previous commit
deadlocklogic Dec 4, 2023
5cb553a
Common.Gen.cs: fixed silly modification while testing
deadlocklogic Dec 4, 2023
c1f392f
GeneratorKind: add FindGeneratorKindByID method
deadlocklogic Dec 5, 2023
bf5a6ea
TypeMapDatabase: heavy refactor: group typemaps by GeneratorKind
deadlocklogic Dec 5, 2023
15ecf3b
TypeMap: refactor CSharp backend into common methods + migration
deadlocklogic Dec 5, 2023
64c3b24
TypeMap: cleanup patches from previous commits
deadlocklogic Dec 5, 2023
502d77e
TypeMapDatabase: fix passing GeneratorKind to FindTypeMap calls
deadlocklogic Dec 5, 2023
fdf796d
Stdlib.CSharp.cs: move std::map typemap from Stdlib.CLI.cs
deadlocklogic Dec 5, 2023
d44b31c
TypeMapDatabase: improve parameter name
deadlocklogic Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Generator/AST/Utils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public static bool IsMappedToPrimitive(ITypeMapDatabase typeMaps, Type type)
return false;

var typePrinterContext = new TypePrinterContext { Type = type };
var mappedTo = typeMap.CSharpSignatureType(typePrinterContext);
var mappedTo = typeMap.SignatureType(typePrinterContext);
mappedTo = mappedTo.Desugar();
mappedTo = (mappedTo.GetFinalPointee() ?? mappedTo).Desugar();
return (mappedTo.IsPrimitiveType() ||
Expand Down
5 changes: 5 additions & 0 deletions src/Generator/GeneratorKind.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ public GeneratorKind(string id, string name, System.Type generatorType, System.T
Registered.Add(this);
}

public static GeneratorKind FindGeneratorKindByID(string id)
{
return Registered.Where(kind => kind.ID == id).First();
}

public Generator CreateGenerator(BindingContext context)
{
return (Generator)Activator.CreateInstance(GeneratorType, context);
Expand Down
14 changes: 7 additions & 7 deletions src/Generator/Generators/C/CppMarshal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public override bool VisitType(Type type, TypeQualifiers quals)
TypeMap typeMap;
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.CppMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return false;
}

Expand Down Expand Up @@ -173,7 +173,7 @@ public override bool VisitTypedefType(TypedefType typedef, TypeQualifiers quals)
typeMap.DoesMarshalling)
{
typeMap.Type = typedef;
typeMap.CppMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return typeMap.IsValueType;
}

Expand All @@ -193,7 +193,7 @@ public override bool VisitTemplateSpecializationType(TemplateSpecializationType
if (Context.Context.TypeMaps.FindTypeMap(template, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.Type = template;
typeMap.CppMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return true;
}

Expand Down Expand Up @@ -341,7 +341,7 @@ public override bool VisitType(Type type, TypeQualifiers quals)
TypeMap typeMap;
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.CppMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return false;
}

Expand Down Expand Up @@ -478,7 +478,7 @@ public override bool VisitTypedefType(TypedefType typedef, TypeQualifiers quals)
if (Context.Context.TypeMaps.FindTypeMap(decl.Type, out typeMap) &&
typeMap.DoesMarshalling)
{
typeMap.CppMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return typeMap.IsValueType;
}

Expand Down Expand Up @@ -516,7 +516,7 @@ public override bool VisitTemplateSpecializationType(TemplateSpecializationType
if (Context.Context.TypeMaps.FindTypeMap(template, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.Type = template;
typeMap.CppMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return true;
}

Expand Down Expand Up @@ -563,7 +563,7 @@ private void MarshalRefClass(Class @class)
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) &&
typeMap.DoesMarshalling)
{
typeMap.CppMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Generator/Generators/C/CppTypePrinter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public virtual bool FindTypeMap(CppSharp.AST.Type type, out TypePrinterResult re
typePrinter.PushContext(ContextKind);
typePrinter.PushScope(ScopeKind);

var typeName = typeMap.CppSignatureType(typePrinterContext).Visit(typePrinter);
var typeName = typeMap.SignatureType(typePrinterContext).Visit(typePrinter);
result = new TypePrinterResult(typeName) { TypeMap = typeMap };

return true;
Expand Down
14 changes: 7 additions & 7 deletions src/Generator/Generators/CLI/CLIMarshal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public override bool VisitType(Type type, TypeQualifiers quals)
TypeMap typeMap;
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.CLIMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return false;
}

Expand Down Expand Up @@ -215,7 +215,7 @@ public override bool VisitTypedefType(TypedefType typedef, TypeQualifiers quals)
typeMap.DoesMarshalling)
{
typeMap.Type = typedef;
typeMap.CLIMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return typeMap.IsValueType;
}

Expand All @@ -240,7 +240,7 @@ public override bool VisitTemplateSpecializationType(TemplateSpecializationType
if (Context.Context.TypeMaps.FindTypeMap(template, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.Type = template;
typeMap.CLIMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return true;
}

Expand Down Expand Up @@ -406,7 +406,7 @@ public override bool VisitType(Type type, TypeQualifiers quals)
TypeMap typeMap;
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.CLIMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return false;
}

Expand Down Expand Up @@ -605,7 +605,7 @@ public override bool VisitTypedefType(TypedefType typedef, TypeQualifiers quals)
if (Context.Context.TypeMaps.FindTypeMap(decl.Type, out typeMap) &&
typeMap.DoesMarshalling)
{
typeMap.CLIMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return typeMap.IsValueType;
}

Expand Down Expand Up @@ -641,7 +641,7 @@ public override bool VisitTemplateSpecializationType(TemplateSpecializationType
if (Context.Context.TypeMaps.FindTypeMap(template, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.Type = template;
typeMap.CLIMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return true;
}

Expand Down Expand Up @@ -688,7 +688,7 @@ private void MarshalRefClass(Class @class)
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) &&
typeMap.DoesMarshalling)
{
typeMap.CLIMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return;
}

Expand Down
8 changes: 4 additions & 4 deletions src/Generator/Generators/CLI/CLITypePrinter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public override TypePrinterResult VisitTagType(TagType tag, TypeQualifiers quals
if (TypeMapDatabase.FindTypeMap(tag, out typeMap))
{
var typePrinterContext = new TypePrinterContext { Type = tag };
return typeMap.CLISignatureType(typePrinterContext).ToString();
return typeMap.SignatureType(typePrinterContext).ToString();
}

Declaration decl = tag.Declaration;
Expand Down Expand Up @@ -112,7 +112,7 @@ public override TypePrinterResult VisitPointerType(PointerType pointer,
Type = pointer
};

return typeMap.CLISignatureType(typePrinterContext).Visit(this);
return typeMap.SignatureType(typePrinterContext).Visit(this);
}

var pointee = pointer.Pointee.Desugar();
Expand Down Expand Up @@ -217,7 +217,7 @@ public override TypePrinterResult VisitTypedefType(TypedefType typedef,
{
typeMap.Type = typedef;
var typePrinterContext = new TypePrinterContext { Type = typedef };
return typeMap.CLISignatureType(typePrinterContext).ToString();
return typeMap.SignatureType(typePrinterContext).ToString();
}

FunctionType func;
Expand All @@ -241,7 +241,7 @@ public override TypePrinterResult VisitTemplateSpecializationType(
if (TypeMapDatabase.FindTypeMap(template, out typeMap) && !typeMap.IsIgnored)
{
var typePrinterContext = new TypePrinterContext { Type = template };
return typeMap.CLISignatureType(typePrinterContext).ToString();
return typeMap.SignatureType(typePrinterContext).ToString();
}

return decl.Name;
Expand Down
4 changes: 2 additions & 2 deletions src/Generator/Generators/CSharp/CSharpMarshal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public override bool VisitType(Type type, TypeQualifiers quals)
TypeMap typeMap;
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.CSharpMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return false;
}

Expand Down Expand Up @@ -471,7 +471,7 @@ public override bool VisitType(Type type, TypeQualifiers quals)
TypeMap typeMap;
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.CSharpMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Generator/Generators/CSharp/CSharpSources.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3158,7 +3158,7 @@ public void GenerateFunctionCall(string functionName, Function function,
Type = indirectRetType.Type.Desugar()
};

WriteLine("{0} {1};", typeMap.CSharpSignatureType(typePrinterContext),
WriteLine("{0} {1};", typeMap.SignatureType(typePrinterContext),
Helpers.ReturnIdentifier);
}
else
Expand Down
20 changes: 10 additions & 10 deletions src/Generator/Generators/CSharp/CSharpTypePrinter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public override TypePrinterResult VisitTagType(TagType tag, TypeQualifiers quals
return string.Empty;

TypeMap typeMap;
if (TypeMapDatabase.FindTypeMap(tag, out typeMap))
if (TypeMapDatabase.FindTypeMap(tag, GeneratorKind.CSharp, out typeMap))
Copy link
Contributor Author

@deadlocklogic deadlocklogic Dec 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we explicitly specify GeneratorKind.CSharp because CSharpTypePrinter is used in CLI. Many similar cases are found in this file, src/Generator/Passes/ExpressionHelper.cs and src/Generator/Passes/ValidateOperatorsPass.cs

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I totally understand why, why is it a problem to pass GeneratorKind.CLI in these cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a bit of analysis and tests I found out that the CLI backend depends on CSharp typemaps in some ways.
Consider the DelegatesPass which is common for CLI and CSharp, it uses CSharpTypePrinter which itself uses the CSharp typemaps. Not passing the GeneratorKind.CSharp explicitly will make the DelegatesPass uses CLI typemaps, which is not desirable in this case.
Consider the Employee.h example:
Using CLI typemap (undesired):

// Employee.h CLI generated header
[::System::Runtime::InteropServices::UnmanagedFunctionPointer(::System::Runtime::InteropServices::CallingConvention::Cdecl)]
delegate ::System::String^ Func__System_String^ ___IntPtr(::System::IntPtr __instance);

Using C# typemaps (desired):

// Employee.h CLI generated header
[::System::Runtime::InteropServices::UnmanagedFunctionPointer(::System::Runtime::InteropServices::CallingConvention::Cdecl)]
delegate ::System::String^ Func_std_basic_string___Internalc__N_std_S_basic_string__C___N_std_S_char_traits__C___N_std_S_allocator__C___IntPtr(::System::IntPtr __instance);

Apparently this signature must match the CSharp one (in the generated .cs file) in order for the project to compile.
To he honest, I was a bit confused why such mess is going on, but then I focused on refactoring rather than changing semantics.
This is why I am working on refactoring the project into modular design because with its current status it is nearly impossible to afford more generators with new semantics/heuristics. Isolating generators is a big win for future development, allowing reusable components as the current status which is also good (but we should add remarks/best practices).
Maybe someone with deeper knowledge on how the CSharp/CLI backend is operating can help us too. I can invest some time into these backends and see how they can be improved (and you should know that there is a lot of room for improvements), but I am thinking of sparing my efforts for newer generators.
By the way, I was thinking if we can standardize/document the typemap concept in details because it is a major concept in binding generation and currently I am finding it a bit confusing when comparing its usage across different generators. Maybe a kind of README/pseudocode can help a lot for users/collaborators.

{
typeMap.Type = tag;

Expand All @@ -47,7 +47,7 @@ public override TypePrinterResult VisitTagType(TagType tag, TypeQualifiers quals
Type = tag
};

return typeMap.CSharpSignatureType(typePrinterContext).ToString();
return typeMap.SignatureType(typePrinterContext).ToString();
}

return base.VisitTagType(tag, quals);
Expand Down Expand Up @@ -150,7 +150,7 @@ public override TypePrinterResult VisitArrayType(ArrayType array,
public override TypePrinterResult VisitBuiltinType(BuiltinType builtin, TypeQualifiers quals)
{
TypeMap typeMap;
if (TypeMapDatabase.FindTypeMap(builtin, out typeMap))
if (TypeMapDatabase.FindTypeMap(builtin, GeneratorKind.CSharp, out typeMap))
{
var typePrinterContext = new TypePrinterContext()
{
Expand All @@ -159,7 +159,7 @@ public override TypePrinterResult VisitBuiltinType(BuiltinType builtin, TypeQual
Type = builtin,
Parameter = Parameter
};
return typeMap.CSharpSignatureType(typePrinterContext).Visit(this);
return typeMap.SignatureType(typePrinterContext).Visit(this);
}
return base.VisitBuiltinType(builtin, quals);
}
Expand All @@ -183,15 +183,15 @@ public override TypePrinterResult VisitPointerType(PointerType pointer,
if (allowStrings && pointer.IsConstCharString())
{
TypeMap typeMap;
TypeMapDatabase.FindTypeMap(pointer, out typeMap);
TypeMapDatabase.FindTypeMap(pointer, GeneratorKind.CSharp, out typeMap);
var typePrinterContext = new TypePrinterContext()
{
Kind = ContextKind,
MarshalKind = MarshalKind,
Type = pointer.Pointee,
Parameter = Parameter
};
return typeMap.CSharpSignatureType(typePrinterContext).Visit(this);
return typeMap.SignatureType(typePrinterContext).Visit(this);
}

var pointee = pointer.Pointee.Desugar();
Expand Down Expand Up @@ -258,7 +258,7 @@ public override TypePrinterResult VisitTypedefType(TypedefType typedef,
var decl = typedef.Declaration;

TypeMap typeMap;
if (TypeMapDatabase.FindTypeMap(typedef, out typeMap))
if (TypeMapDatabase.FindTypeMap(typedef, GeneratorKind.CSharp, out typeMap))
{
typeMap.Type = typedef;

Expand All @@ -270,7 +270,7 @@ public override TypePrinterResult VisitTypedefType(TypedefType typedef,
Parameter = Parameter
};

return typeMap.CSharpSignatureType(typePrinterContext).ToString();
return typeMap.SignatureType(typePrinterContext).ToString();
}

FunctionType func;
Expand Down Expand Up @@ -299,7 +299,7 @@ public override TypePrinterResult VisitTemplateSpecializationType(
template.Template.TemplatedDecl;

TypeMap typeMap;
if (!TypeMapDatabase.FindTypeMap(template, out typeMap))
if (!TypeMapDatabase.FindTypeMap(template, GeneratorKind.CSharp, out typeMap))
{
if (ContextKind == TypePrinterContextKind.Managed &&
decl == template.Template.TemplatedDecl &&
Expand Down Expand Up @@ -330,7 +330,7 @@ public override TypePrinterResult VisitTemplateSpecializationType(
MarshalKind = MarshalKind
};

return typeMap.CSharpSignatureType(typePrinterContext).ToString();
return typeMap.SignatureType(typePrinterContext).ToString();
}

public override TypePrinterResult VisitDependentTemplateSpecializationType(
Expand Down
7 changes: 2 additions & 5 deletions src/Generator/Generators/ExtensionMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,9 @@ public static Type GetMappedType(this Type type, TypeMapDatabase typeMaps,
Type = typeMap.Type
};

switch (generatorKind)
if (generatorKind == GeneratorKind.CLI || generatorKind == GeneratorKind.CSharp)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this if now?

Copy link
Contributor Author

@deadlocklogic deadlocklogic Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I am not totally sure, I can try removing it and run the tests and see what goes on. Here I am refactoring while keeping the same semantics.
We can investigate desired behavior thought, I don't exactly know if this behavior is intended exclusively for CLI/CSharp or can be extended for other generators.
Isolating the current tightly coupled architecture needs many steps, which this PR addresses one of them.

{
case var _ when ReferenceEquals(generatorKind, GeneratorKind.CLI):
return typeMap.CLISignatureType(typePrinterContext).Desugar();
case var _ when ReferenceEquals(generatorKind, GeneratorKind.CSharp):
return typeMap.CSharpSignatureType(typePrinterContext).Desugar();
return typeMap.SignatureType(typePrinterContext).Desugar();
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/Generator/Generators/NAPI/NAPIMarshal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public override bool VisitType(Type type, TypeQualifiers quals)
TypeMap typeMap;
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.CppMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return false;
}

Expand Down Expand Up @@ -194,7 +194,7 @@ public override bool VisitTypedefType(TypedefType typedef, TypeQualifiers quals)
typeMap.DoesMarshalling)
{
typeMap.Type = typedef;
typeMap.CppMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return typeMap.IsValueType;
}

Expand All @@ -214,7 +214,7 @@ public override bool VisitTemplateSpecializationType(TemplateSpecializationType
if (Context.Context.TypeMaps.FindTypeMap(template, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.Type = template;
typeMap.CppMarshalToManaged(Context);
typeMap.MarshalToManaged(Context);
return true;
}

Expand Down Expand Up @@ -343,7 +343,7 @@ public override bool VisitType(Type type, TypeQualifiers quals)
TypeMap typeMap;
if (Context.Context.TypeMaps.FindTypeMap(type, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.CppMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return false;
}

Expand Down Expand Up @@ -591,7 +591,7 @@ public override bool VisitTypedefType(TypedefType typedef, TypeQualifiers quals)
if (Context.Context.TypeMaps.FindTypeMap(decl.Type, out typeMap) &&
typeMap.DoesMarshalling)
{
typeMap.CppMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return typeMap.IsValueType;
}

Expand Down Expand Up @@ -628,7 +628,7 @@ public override bool VisitTemplateSpecializationType(TemplateSpecializationType
if (Context.Context.TypeMaps.FindTypeMap(template, out typeMap) && typeMap.DoesMarshalling)
{
typeMap.Type = template;
typeMap.CppMarshalToNative(Context);
typeMap.MarshalToNative(Context);
return true;
}

Expand Down
Loading