Skip to content

Commit

Permalink
fixes crahes with some recursive patterns (#281)
Browse files Browse the repository at this point in the history
  • Loading branch information
adrianoc committed Jun 19, 2024
1 parent 254402a commit e54a93e
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 14 deletions.
37 changes: 31 additions & 6 deletions Cecilifier.Core.Tests/Tests/Unit/PatternExpressionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,36 @@ private static IEnumerable RecursivePatternsTestScenarios()
{ TestName = "Multiple Properties" };

// Issue #281
// yield return new TestCaseData(
// "void M(object o) { var r = o is System.Uri { Host.Length: 10 }; }",
// "TODO: //DEFINE EXPECTATION")
// {
// TestName = "Property (MemberAccessExpression)"
// };
yield return new TestCaseData(
"void M(object o) { var r = o is System.Uri { Host.Length: 10 }; }",
"""
//var r = o is System.Uri { Host.Length: 10 };
\s+var l_r_\d+ = new VariableDefinition\(assembly.MainModule.TypeSystem.Boolean\);
\s+m_M_\d+.Body.Variables.Add\(l_r_\d+\);
\s+il_M_\d+.Emit\(OpCodes.Ldarg_1\);
\s+var l_tmp_\d+ = new VariableDefinition\(assembly.MainModule.ImportReference\(typeof\(System.Uri\)\)\);
\s+m_M_\d+.Body.Variables.Add\(l_tmp_\d+\);
\s+var ldc_I4_0_\d+ = il_M_\d+.Create\(OpCodes.Ldc_I4_0\);
\s+var nop_\d+ = il_M_\d+.Create\(OpCodes.Nop\);
(\s+il_M_\d+\.Emit\(OpCodes\.)Isinst, assembly.MainModule.ImportReference\(typeof\(System.Uri\)\)\);
\1Stloc, l_tmp_\d+\);
\1Ldloc, l_tmp_\d+\);
\1Brfalse_S, ldc_I4_0_\d+\);
\1Ldloc, l_tmp_\d+\);
\1Callvirt, .+ImportReference\(.+ResolveMethod\(typeof\(System.Uri\), "get_Host",.+\)\)\);
\1Callvirt, .+ImportReference\(.+ResolveMethod\(typeof\(System.String\), "get_Length",.+\)\)\);
\1Ldc_I4, 10\);
\1Bne_Un, ldc_I4_0_\d+\);
\1Ldc_I4_1\);
\1Br_S, nop_\d+\);
\s+il_M_\d+.Append\(ldc_I4_0_\d+\);
\s+il_M_\d+.Append\(nop_\d+\);
\1Stloc, l_r_\d+\);
\1Ret\);
\s+//End of local function.
""")
{
TestName = "Property (MemberAccessExpression)"
};
}
}
4 changes: 2 additions & 2 deletions Cecilifier.Core/AST/ExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ public override void VisitRecursivePattern(RecursivePatternSyntax node)
Context.EmitCilInstruction(ilVar, OpCodes.Ldloc, localVar);
pattern.Accept(this);

var comparisonType = Context.SemanticModel.GetSymbolInfo(pattern.NameColon.Name).Symbol.GetMemberType();
var comparisonType = Context.SemanticModel.GetSymbolInfo(pattern.NameColon?.Name ?? pattern.ExpressionColon?.Expression).Symbol.GetMemberType();
var opEquality = comparisonType.GetMembers().FirstOrDefault(m => m.Kind == SymbolKind.Method && m.Name == "op_Equality");
if (opEquality != null)
{
Expand Down Expand Up @@ -1231,7 +1231,7 @@ private void ProcessProperty(SimpleNameSyntax node, IPropertySymbol propertySymb
{
propertySymbol.EnsurePropertyExists(Context, node);

if (!propertySymbol.IsStatic && !node.IsQualifiedAccessToTypeOrMember())
if (!propertySymbol.IsStatic && node.IsMemberAccessThroughImplicitThis())
{
// if this is an *unqualified* access we need to load *this*
Context.EmitCilInstruction(ilVar, OpCodes.Ldarg_0);
Expand Down
2 changes: 1 addition & 1 deletion Cecilifier.Core/AST/SyntaxWalkerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ protected void ProcessField(string ilVar, SimpleNameSyntax node, IFieldSymbol fi

var fieldDeclarationVariable = fieldSymbol.EnsureFieldExists(Context, node);

if (!fieldSymbol.IsStatic && !node.IsQualifiedAccessToTypeOrMember())
if (!fieldSymbol.IsStatic && node.IsMemberAccessThroughImplicitThis())
Context.EmitCilInstruction(ilVar, OpCodes.Ldarg_0);

if (HandleLoadAddressOnStorage(ilVar, fieldSymbol.Type, node, fieldSymbol.IsStatic ? OpCodes.Ldsflda : OpCodes.Ldflda, fieldSymbol.Name, VariableMemberKind.Field, fieldSymbol.ContainingType.ToDisplayString()))
Expand Down
11 changes: 6 additions & 5 deletions Cecilifier.Core/Extensions/SyntaxNodeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ public static class SyntaxNodeExtensions
/// 1. in, `Foo f = new NS.Foo();`, `Foo` : Foo f => Unqualified, NS.Foo => Qualified
/// 2. `o.field ?? otherField`, otherField => Unqualified, `field` in `o.field` => Qualified
/// </remarks>
public static bool IsQualifiedAccessToTypeOrMember(this SimpleNameSyntax node) => node.Parent switch
public static bool IsMemberAccessThroughImplicitThis(this SyntaxNode node) => node.Parent switch
{
MemberAccessExpressionSyntax mae => mae.Name == node,
MemberBindingExpressionSyntax mbe => mbe.Name == node, // A MemberBindExpression represents `?.` in the null conditional operator, for instance, `o?.member`
NameColonSyntax => true, // A NameColon syntax represents the `Length: 42` in an expression like `o as string { Length: 42 }`. In this case, `Length` is equivalent to `o.Length`
_ => false
MemberAccessExpressionSyntax mae => mae.Name != node && mae.IsMemberAccessThroughImplicitThis(),
MemberBindingExpressionSyntax mbe => mbe.Name != node, // A MemberBindExpression represents `?.` in the null conditional operator, for instance, `o?.member`
NameColonSyntax => false, // A NameColon syntax represents the `Length: 42` in an expression like `o as string { Length: 42 }`. In this case, `Length` is equivalent to `o.Length`
ExpressionColonSyntax => false, // `o as Uri { Host.Length : 10 }`. Parent of `Host.Length` (which is equivalent to `o.Host.Length`) is an ExpressionColonSyntax
_ => true
};

/// <summary>
Expand Down

0 comments on commit e54a93e

Please sign in to comment.