Skip to content

Commit

Permalink
fixes 'constrained' opcode not being emited on calls on generic inlin…
Browse files Browse the repository at this point in the history
…e array elements

also adds a disabled test for accessing fields of such inline arrays (looks like the code around
accessing fields on elements of  generic arrays is broken)
  • Loading branch information
adrianoc committed Mar 27, 2024
1 parent 65de94e commit 4b62f94
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 32 deletions.
19 changes: 13 additions & 6 deletions Cecilifier.Core.Tests/Tests/Unit/InlineArrayTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,13 @@ public struct Buffer { private CustomStruct _element0; }
[TestCase("T M<T>(Buffer<T> b) => b[0];",
"""
(\s+il_M_\d+\.Emit\(OpCodes\.)Call, gi_inlineArrayFirstElementRef_\d+\);
\1Ldobj, gp_T_14\);
\1Ldobj, gp_T_\d+\);
""", TestName = "Open generic method")]

[TestCase("int M(Buffer<int> bi) => bi[0];",
"""
\s+il_M_14.Emit\(OpCodes.Call, gi_inlineArrayFirstElementRef_\d+\);
\s+il_M_14.Emit\(OpCodes.Ldind_I4\);
(\s+il_M_\d+.Emit\(OpCodes\.)Call, gi_inlineArrayFirstElementRef_\d+\);
\1Ldind_I4\);
""", TestName = "Closed generic type (primitive type)")]

[TestCase("CustomStruct M(Buffer<CustomStruct> b) => b[0];",
Expand All @@ -289,19 +289,26 @@ public struct Buffer { private CustomStruct _element0; }
\3Ldobj, \2\);
""", TestName = "Interface as Type Parameter")]

//TODO: accessing a property/method/event from an interface on a generic type must be constrained
// The code is handling 'non inline array' (see example https://cutt.ly/constrained_non_inlinearray)
[TestCase("int M<T>(Buffer<T> b) where T : Itf => b[0].Value;",
"""
(gi_inlineArrayFirstElementRef_\d+).GenericArguments.Add\((gp_T_\d+)\);
(\s+il_M_\d+\.Emit\(OpCodes\.)Call, \1\);
\3Constrained, \2\);
\3Callvirt, m_get_\d+\);
""", IgnoreReason = "Accessing a property/method/event from an interface on a generic type must be constrained", TestName = "Member access on interface")]
""", TestName = "Member access on interface")]

[TestCase("int M<T>(Buffer<T> b) where T : CustomClass => b[0].Value;",
"""
(gi_inlineArrayFirstElementRef_\d+).GenericArguments.Add\((gp_T_\d+)\);
(\s+il_M_\d+\.Emit\(OpCodes\.)Call, \1\);
xxxx\3Constrained, \2\);
\3Callvirt, m_get_\d+\);
""", IgnoreReason = "https://github.com/adrianoc/cecilifier/issues/274", TestName = "Field access on class")]
public void InlineArray_ElementAccess_OnGenericType(string toBeTested, string expecetdIL)
{
var result = RunCecilifier($$"""
struct CustomStruct { public int Value; }
class CustomClass { public int Value; }
public interface Itf { int Value { get; set; } }
Expand Down
20 changes: 14 additions & 6 deletions Cecilifier.Core/AST/ExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,22 @@ public override void VisitElementAccessExpression(ElementAccessExpressionSyntax
// the case of that target being an inline array element, that means that the address of
// the entry should be at the top of the stack which is exactly how
// TryHandleInlineArrayElementAccess() will leave the stack so in this case there's nothing.
// else to be done.
// Otherwise, we need to take the top of the stack (address of the element) and load the
// actual instance to the stack.
if (!node.Parent.IsKind(SyntaxKind.SimpleMemberAccessExpression))
// else to be done ...
if (node.Parent.IsKind(SyntaxKind.SimpleMemberAccessExpression))
{
var loadOpCode = elementType.LdindOpCodeFor();
Context.EmitCilInstruction(ilVar, loadOpCode, loadOpCode == OpCodes.Ldobj ? Context.TypeResolver.Resolve(elementType) : null);
if (UsageVisitor.GetInstance(Context).Visit(node.Parent) == UsageKind.CallTarget)
{
Context.SetFlag(Constants.ContextFlags.MemberReferenceRequiresConstraint, Context.TypeResolver.Resolve(elementType));
return;
}

if (elementType.TypeKind != TypeKind.TypeParameter)
return;
}

// ... otherwise, we need to take the top of the stack (address of the element) and load the actual instance to the stack.
var loadOpCode = elementType.LdindOpCodeFor();
Context.EmitCilInstruction(ilVar, loadOpCode, loadOpCode == OpCodes.Ldobj ? Context.TypeResolver.Resolve(elementType) : null);
return;
}

Expand Down
36 changes: 17 additions & 19 deletions Cecilifier.Core/AST/InlineArrayProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,28 +84,26 @@ internal static bool TryHandleInlineArrayElementAccess(IVisitorContext context,
return false;

var inlineArrayType = context.SemanticModel.GetSymbolInfo(elementAccess.Expression).Symbol.EnsureNotNull().GetMemberType();
if (inlineArrayType.TryGetAttribute<InlineArrayAttribute>(out _))
{
ExpressionVisitor.Visit(context, ilVar, elementAccess.Expression);
Debug.Assert(elementAccess.ArgumentList.Arguments.Count == 1);

var method = string.Empty;
if (elementAccess.ArgumentList.Arguments[0].Expression.TryGetLiteralValueFor(out int index) && index == 0)
{
method = InlineArrayFirstElementRefMethodFor(context, inlineArrayType);
}
else
{
context.EmitCilInstruction(ilVar, OpCodes.Ldc_I4, index);
method = InlineArrayElementRefMethodFor(context, inlineArrayType);
}
context.EmitCilInstruction(ilVar, OpCodes.Call, method);
if (!inlineArrayType.TryGetAttribute<InlineArrayAttribute>(out _))
return false;

ExpressionVisitor.Visit(context, ilVar, elementAccess.Expression);
Debug.Assert(elementAccess.ArgumentList.Arguments.Count == 1);

elementType = InlineArrayElementTypeFrom(inlineArrayType);
return true;
var method = string.Empty;
if (elementAccess.ArgumentList.Arguments[0].Expression.TryGetLiteralValueFor(out int index) && index == 0)
{
method = InlineArrayFirstElementRefMethodFor(context, inlineArrayType);
}
else
{
context.EmitCilInstruction(ilVar, OpCodes.Ldc_I4, index);
method = InlineArrayElementRefMethodFor(context, inlineArrayType);
}
context.EmitCilInstruction(ilVar, OpCodes.Call, method);

return false;
elementType = InlineArrayElementTypeFrom(inlineArrayType);
return true;

static string InlineArrayFirstElementRefMethodFor(IVisitorContext context, ITypeSymbol inlineArrayType)
{
Expand Down
2 changes: 1 addition & 1 deletion Cecilifier.Core/Extensions/MethodExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Mono.Cecil;
using MethodAttributes=Mono.Cecil.MethodAttributes;
using static Cecilifier.Core.Misc.Utils;

namespace Cecilifier.Core.Extensions
Expand Down

0 comments on commit 4b62f94

Please sign in to comment.