Skip to content

Commit 3c49e86

Browse files
Copilotjonathanvdc
Copilot
and
jonathanvdc
authored
Eliminate factory delegates from TypeDefinition (#149)
* Initial plan * Eliminate factory delegates from TypeDefinition Agent-Logs-Url: https://github.com/jonathanvdc/MLIR.NET/sessions/c2a7e51e-27a6-4171-b288-c6f9845233fa Co-authored-by: jonathanvdc <9839946+jonathanvdc@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jonathanvdc <9839946+jonathanvdc@users.noreply.github.com>
1 parent 65fadb9 commit 3c49e86

8 files changed

Lines changed: 81 additions & 131 deletions

File tree

‎src/MLIR.Generators/Emitters/Common/EmitterHelpers.cs‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -220,27 +220,21 @@ public static void AppendSeparated(StringBuilder builder, int count, Action<int>
220220
/// <c>AttributeDefinition</c>.
221221
/// </summary>
222222
/// <remarks>
223-
/// The helper keeps the common shape in one place: a definition name, an optional assembly
224-
/// format object, and an optional factory expression.
223+
/// The helper keeps the common shape in one place: a definition name and an optional assembly
224+
/// format object.
225225
/// </remarks>
226226
public static void AppendDefinitionConstructor(
227227
StringBuilder builder,
228228
string definitionTypeName,
229229
string name,
230-
string? assemblyFormatExpression = null,
231-
string? factoryExpression = null)
230+
string? assemblyFormatExpression = null)
232231
{
233232
builder.Append(" new " + definitionTypeName + "(" + ToCSharpStringLiteral(name));
234233
if (assemblyFormatExpression != null)
235234
{
236235
builder.Append(", " + assemblyFormatExpression);
237236
}
238237

239-
if (factoryExpression != null)
240-
{
241-
builder.Append(", factory: " + factoryExpression);
242-
}
243-
244238
builder.AppendLine(");");
245239
}
246240

‎src/MLIR.Generators/Emitters/TypeEmitter.cs‎

Lines changed: 7 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -68,24 +68,11 @@ private static bool EmitFloatBuiltinWrapper(StringBuilder builder, TypeModel typ
6868
builder.AppendLine(" public static TypeDefinition TypeDefinition { get; } =");
6969

7070
var assemblyFormatExpression = type.CsharpAssemblyFormat;
71-
var factoryExpression = assemblyFormatExpression == null
72-
? "static context => " + className + ".BindValue(context)"
73-
: null;
7471
EmitterHelpers.AppendDefinitionConstructor(
7572
builder,
7673
"TypeDefinition",
7774
type.Name,
78-
assemblyFormatExpression,
79-
factoryExpression);
80-
81-
if (factoryExpression != null)
82-
{
83-
builder.AppendLine();
84-
builder.AppendLine(" public static " + className + " BindValue(TypeReferenceConstructionContext context)");
85-
builder.AppendLine(" {");
86-
builder.AppendLine(" return new " + className + "(context.Syntax as BuiltinFloatTypeSyntax);");
87-
builder.AppendLine(" }");
88-
}
75+
assemblyFormatExpression);
8976

9077
// Derive the scalar mnemonic from the canonical type name (e.g., "builtin.f32" -> "f32").
9178
// This ensures FloatTypeReference.Name carries the MLIR spelling, not the qualified registry key.
@@ -117,24 +104,11 @@ private static bool EmitIndexBuiltinWrapper(StringBuilder builder, TypeModel typ
117104
builder.AppendLine(" public static TypeDefinition TypeDefinition { get; } =");
118105

119106
var assemblyFormatExpression = type.CsharpAssemblyFormat;
120-
var factoryExpression = assemblyFormatExpression == null
121-
? "static context => " + className + ".BindValue(context)"
122-
: null;
123107
EmitterHelpers.AppendDefinitionConstructor(
124108
builder,
125109
"TypeDefinition",
126110
type.Name,
127-
assemblyFormatExpression,
128-
factoryExpression);
129-
130-
if (factoryExpression != null)
131-
{
132-
builder.AppendLine();
133-
builder.AppendLine(" public static " + className + " BindValue(TypeReferenceConstructionContext context)");
134-
builder.AppendLine(" {");
135-
builder.AppendLine(" return new " + className + "(context.Syntax);");
136-
builder.AppendLine(" }");
137-
}
111+
assemblyFormatExpression);
138112

139113
builder.AppendLine();
140114
builder.AppendLine(" public " + className + "(TypeSyntax? syntax = null)");
@@ -153,24 +127,11 @@ private static bool EmitNoneBuiltinWrapper(StringBuilder builder, TypeModel type
153127
builder.AppendLine(" public static TypeDefinition TypeDefinition { get; } =");
154128

155129
var assemblyFormatExpression = type.CsharpAssemblyFormat;
156-
var factoryExpression = assemblyFormatExpression == null
157-
? "static context => " + className + ".BindValue(context)"
158-
: null;
159130
EmitterHelpers.AppendDefinitionConstructor(
160131
builder,
161132
"TypeDefinition",
162133
type.Name,
163-
assemblyFormatExpression,
164-
factoryExpression);
165-
166-
if (factoryExpression != null)
167-
{
168-
builder.AppendLine();
169-
builder.AppendLine(" public static " + className + " BindValue(TypeReferenceConstructionContext context)");
170-
builder.AppendLine(" {");
171-
builder.AppendLine(" return new " + className + "(context.Syntax);");
172-
builder.AppendLine(" }");
173-
}
134+
assemblyFormatExpression);
174135

175136
builder.AppendLine();
176137
builder.AppendLine(" public " + className + "(TypeSyntax? syntax = null)");
@@ -192,13 +153,7 @@ private static void EmitPlainTypeClass(StringBuilder builder, TypeModel type, st
192153
EmitterHelpers.AppendDefinitionConstructor(
193154
builder,
194155
"TypeDefinition",
195-
type.Name,
196-
factoryExpression: "static context => " + className + ".BindValue(context)");
197-
builder.AppendLine();
198-
builder.AppendLine(" public static " + className + " BindValue(TypeReferenceConstructionContext context)");
199-
builder.AppendLine(" {");
200-
builder.AppendLine(" return new " + className + "(context.Syntax);");
201-
builder.AppendLine(" }");
156+
type.Name);
202157
builder.AppendLine();
203158
builder.AppendLine(" public " + className + "(TypeSyntax? syntax = null)");
204159
builder.AppendLine(" : base(syntax, syntax?.Location ?? MLIR.Semantics.SourceLocation.Unknown)");
@@ -214,20 +169,16 @@ private static void EmitParametrisedTypeClass(StringBuilder builder, TypeModel t
214169
{
215170
var parameters = type.Parameters;
216171
var hasAssemblyFormat = type.AssemblyFormat != null;
217-
var syntaxClassName = hasAssemblyFormat ? className + "Syntax" : null;
218172
var formatClassName = hasAssemblyFormat ? className + "AssemblyFormat" : null;
219173
var assemblyFormatExpression = !string.IsNullOrEmpty(type.CsharpAssemblyFormat)
220174
? type.CsharpAssemblyFormat
221175
: formatClassName != null
222176
? "new " + formatClassName + "()"
223177
: null;
224-
var factoryExpression = formatClassName != null
225-
? "static context => " + formatClassName + ".BindValue(context.Syntax!)"
226-
: null;
227178

228179
builder.AppendLine("public partial class " + className + " : TypeReference");
229180
builder.AppendLine("{");
230-
EmitTypeDefinition(builder, type, assemblyFormatExpression, factoryExpression);
181+
EmitTypeDefinition(builder, type, assemblyFormatExpression);
231182
builder.AppendLine();
232183

233184
EmitTypeConstructor(builder, className, parameters);
@@ -250,11 +201,10 @@ private static void EmitParametrisedTypeClass(StringBuilder builder, TypeModel t
250201
private static void EmitTypeDefinition(
251202
StringBuilder builder,
252203
TypeModel type,
253-
string? assemblyFormatExpression,
254-
string? factoryExpression)
204+
string? assemblyFormatExpression)
255205
{
256206
builder.AppendLine(" public static TypeDefinition TypeDefinition { get; } =");
257-
EmitterHelpers.AppendDefinitionConstructor(builder, "TypeDefinition", type.Name, assemblyFormatExpression, factoryExpression);
207+
EmitterHelpers.AppendDefinitionConstructor(builder, "TypeDefinition", type.Name, assemblyFormatExpression);
258208
}
259209

260210
private static void EmitTypeConstructor(StringBuilder builder, string className, IReadOnlyList<AttrOrTypeParameterModel> parameters)

‎src/MLIR/Dialects/TypeDefinition.cs‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,26 @@
11
namespace MLIR.Dialects;
22

3-
using MLIR.Semantics;
4-
53
/// <summary>
64
/// Describes a concrete dialect-defined type (<c>TypeDef</c> in ODS).
75
/// </summary>
86
/// <remarks>
7+
/// <para>
98
/// Concrete type definitions are also valid type constraints, so this class derives from
109
/// <see cref="TypeConstraintDefinition"/>.
11-
/// </remarks>
12-
/// <remarks>
13-
/// Initializes a new instance of the <see cref="TypeDefinition"/> class.
10+
/// </para>
11+
/// <para>
12+
/// A <see cref="TypeDefinition"/> carries registered metadata (canonical name and optional assembly
13+
/// format) for a dialect type. Binding is driven by
14+
/// <see cref="ITypeAssemblyFormat.Bind(MLIR.Syntax.TypeSyntax, TypeDefinition, MLIR.Semantics.Binder)"/>
15+
/// when an assembly format is present. When no assembly format is registered, the binder falls back to
16+
/// producing an <c>UnknownTypeReference</c> with the definition attached.
17+
/// </para>
1418
/// </remarks>
1519
/// <param name="name">The canonical type name.</param>
1620
/// <param name="assemblyFormat">The optional custom assembly interpretation hook.</param>
17-
/// <param name="factory">The typed type-reference factory.</param>
1821
public sealed class TypeDefinition(
1922
string name,
20-
ITypeAssemblyFormat? assemblyFormat = null,
21-
System.Func<TypeReferenceConstructionContext, TypeReference>? factory = null)
23+
ITypeAssemblyFormat? assemblyFormat = null)
2224
: TypeConstraintDefinition(name, assemblyFormat)
2325
{
2426
/// <summary>
@@ -29,10 +31,4 @@ public sealed class TypeDefinition(
2931
/// concrete <c>TypeDef</c> registrations.
3032
/// </remarks>
3133
public new string Name { get; } = name;
32-
33-
/// <summary>
34-
/// Gets the typed type-reference factory.
35-
/// </summary>
36-
public System.Func<TypeReferenceConstructionContext, TypeReference> Factory { get; } =
37-
factory ?? (static context => new UnknownTypeReference(context.Syntax, context.Name, context.Definition, context.Location));
3834
}

‎src/MLIR/Semantics/Binder.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -633,9 +633,12 @@ private TypeReference BindStructuredTypeReference(TypeSyntax syntax)
633633
return null;
634634
}
635635

636+
// If an assembly format is registered, let it drive binding. Otherwise fall back to an
637+
// UnknownTypeReference that carries the definition metadata so callers can still identify
638+
// the type family.
636639
return definition.AssemblyFormat != null
637640
? definition.AssemblyFormat.Bind(syntax, definition, this)
638-
: definition.Factory(new TypeReferenceConstructionContext(syntax, canonicalName, definition, syntax.Location));
641+
: new UnknownTypeReference(syntax, canonicalName, definition, syntax.Location);
639642
}
640643

641644
/// <summary>
@@ -670,9 +673,12 @@ private TypeReference BindTypeReferenceCore(TypeSyntax syntaxNode, RawSyntaxText
670673
TypeReference type;
671674
if (definition != null)
672675
{
676+
// If an assembly format is registered, let it drive binding. Otherwise fall back to an
677+
// UnknownTypeReference that carries the definition metadata so callers can still identify
678+
// the type family.
673679
type = definition.AssemblyFormat != null
674680
? definition.AssemblyFormat.Bind(syntaxNode, definition, this)
675-
: definition.Factory(new TypeReferenceConstructionContext(syntaxNode, canonicalName, definition, syntaxNode.Location));
681+
: new UnknownTypeReference(syntaxNode, canonicalName, definition, syntaxNode.Location);
676682
}
677683
else
678684
{

‎src/MLIR/Semantics/TypeReferenceConstructionContext.cs‎

Lines changed: 0 additions & 45 deletions
This file was deleted.

‎tests/MLIR.Generators.Tests/DialectGeneratorTypedTypeTests.cs‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ public void TypeDefWithParametersAndAssemblyFormatGeneratesTypedSyntaxAndReferen
2626
AssertContainsAll(
2727
registrationSource,
2828
"new TypeDefinition(\"myp.opaque\",",
29-
"factory: static context => opaqueTypeAssemblyFormat.BindValue(",
3029
"TypeSyntax? syntax = null)",
3130
": base(syntax, syntax?.Location ?? MLIR.Semantics.SourceLocation.Unknown)",
3231
"TypeSyntax BuildCustomAssemblySyntax(TypeReference type, ConcreteSyntaxBuilderContext context)",
3332
"WritePrefix(writer);",
3433
"new DialectTypePrefix(bangToken, nameToken)");
3534
AssertDoesNotContainAny(
3635
registrationSource,
36+
"factory:",
3737
"public NamedAttribute",
3838
"DialectPrefixedAttributeValueSyntax");
3939
}
@@ -83,4 +83,33 @@ public void TypeDefWithParametersAndNoAssemblyFormatGeneratesTypedReferenceClass
8383
"OpaqueTypeSyntax",
8484
"OpaqueTypeAssemblyFormat");
8585
}
86+
87+
[Fact]
88+
public void PlainTypeDefWithNoParametersGeneratesNoFactoryDelegate()
89+
{
90+
var registrationSource = GenerateMyDialectRegistrationSource(
91+
[
92+
"include \"mlir/IR/AttrTypeBase.td\"",
93+
string.Empty,
94+
"class MyDialect_Type<string name> : TypeDef<MyDialect_Dialect, name> {",
95+
" let typeName = \"myp.\" # name;",
96+
"};",
97+
string.Empty,
98+
"def MyDialect_PlainType : MyDialect_Type<\"plain\"> {",
99+
" let summary = \"a plain type with no parameters\";",
100+
"};",
101+
]);
102+
103+
// Plain types without parameters must not emit a factory delegate. Binding falls
104+
// back to UnknownTypeReference when no assembly format is registered.
105+
AssertContainsAll(
106+
registrationSource,
107+
"new TypeDefinition(\"myp.plain\");",
108+
"TypeSyntax? syntax = null)");
109+
110+
AssertDoesNotContainAny(
111+
registrationSource,
112+
"factory:",
113+
"BindValue(TypeReferenceConstructionContext");
114+
}
86115
}

‎tests/MLIR.Tests/SemanticBindingTests.cs‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,26 @@ public void GeneratedScalarTypeDefinitionsUseAssemblyFormatNotFactory()
173173
Assert.NotNull(NoneType.TypeDefinition.AssemblyFormat);
174174
}
175175

176+
[Fact]
177+
public void RegisteredTypeDefinitionWithoutAssemblyFormatBindsToUnknownTypeReferenceWithDefinitionSet()
178+
{
179+
// A TypeDefinition registered without an assembly format should produce an
180+
// UnknownTypeReference whose Definition is non-null. The binder has no factory-delegate
181+
// fallback; the definition metadata is preserved so callers can still identify the family.
182+
var definition = new TypeDefinition("test.plain");
183+
var registry = new DialectRegistry();
184+
registry.RegisterDialect(new Dialect("test", [], [], [definition]));
185+
186+
var module = Binder.BindModule(
187+
Parser.ParseModule("%0 = \"test.op\"() : !test.plain", registry),
188+
registry);
189+
190+
var type = module.Operations[0].TypeSignatureReference;
191+
var unknown = Assert.IsType<UnknownTypeReference>(type);
192+
Assert.Equal("test.plain", unknown.Name);
193+
Assert.Same(definition, unknown.Definition);
194+
}
195+
176196
[Fact]
177197
public void RegisteredScalarTypesCompareEqualToFallbackScalarTypes()
178198
{
@@ -297,7 +317,7 @@ public void BindsAttributeAndTypeDefinitionsFromTheRegistry()
297317
"builtin",
298318
[],
299319
[new AttributeDefinition("dense", new DenseAttributeAssemblyFormat())],
300-
[new TypeDefinition("i32", new BuiltinIntegerTypeAssemblyFormat(), static context => new IntegerType(32, IntegerTypeSignedness.Signless, context.Syntax))]));
320+
[new TypeDefinition("i32", new BuiltinIntegerTypeAssemblyFormat())]));
301321

302322
var module = Binder.BindModule(
303323
Parser.ParseModule("%0 = \"test.op\"() {value = #dense<[1, 2]> : tensor<2xi32>} : i32", registry),
@@ -326,7 +346,7 @@ public void OperationAssemblyFormatCanParseAttributesUsingExpectedDefinition()
326346
"builtin",
327347
[],
328348
[i32AttributeDefinition],
329-
[new TypeDefinition("i32", new BuiltinIntegerTypeAssemblyFormat(), static context => new IntegerType(32, IntegerTypeSignedness.Signless, context.Syntax))]));
349+
[new TypeDefinition("i32", new BuiltinIntegerTypeAssemblyFormat())]));
330350
registry.RegisterDialect(
331351
Dialect.Create(
332352
"arith",

‎tests/MLIR.Tests/SemanticPrintingTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ public void AttributeAndTypeBindingCanReportDiagnostics()
291291
"builtin",
292292
[],
293293
[new AttributeDefinition("dense", new DenseAttributeAssemblyFormat())],
294-
[new TypeDefinition("i32", new BuiltinIntegerTypeAssemblyFormat(), static context => new IntegerType(32, IntegerTypeSignedness.Signless, context.Syntax))]));
294+
[new TypeDefinition("i32", new BuiltinIntegerTypeAssemblyFormat())]));
295295

296296
var module = Binder.BindModule(
297297
Parser.ParseModule("\"test.op\"() {value = #dense<[1, 2]>} : () -> i32"),

0 commit comments

Comments
 (0)