From 0f1701abb4b8d2441bafcc32a2f9d03010247296 Mon Sep 17 00:00:00 2001 From: Adriano Carlos Verona Date: Fri, 18 Aug 2023 11:47:47 -0400 Subject: [PATCH] fix extra ldarg_0 instruction emmited on member access through conditional expressions (#208) Member access through conditional expressions were being considered to be equivalent to an unqualified access which requires 'this' to be implicitly loaded leading to extra 'ldlarg_0' to be emmited. --- .../Tests/Unit/OperatorsTests.cs | 19 ++++++++++++++ Cecilifier.Core/AST/ExpressionVisitor.cs | 25 +------------------ Cecilifier.Core/AST/SyntaxWalkerBase.cs | 3 +-- .../Extensions/SyntaxNodeExtensions.cs | 18 +++++++++++++ 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/Cecilifier.Core.Tests/Tests/Unit/OperatorsTests.cs b/Cecilifier.Core.Tests/Tests/Unit/OperatorsTests.cs index cc2432a2..bdbaa0bf 100644 --- a/Cecilifier.Core.Tests/Tests/Unit/OperatorsTests.cs +++ b/Cecilifier.Core.Tests/Tests/Unit/OperatorsTests.cs @@ -107,6 +107,25 @@ public void TestNullConditionalOperatorOnStorageTargets(string target, Code expe @"il_M_3.Append\(lbl_conditionEnd_7\);")); } + [TestCase("f", "Ldfld, fld_f_6", TestName = "Field")] + [TestCase("P", "Call, m_get_2", TestName = "Property")] + [TestCase("M()", "Call, m_M_4", TestName = "Method")] + public void TestNullConditionalOperator_OnQualifiedMemberAccess_DoesNoLoadThis(string member, string expected) + { + var result = RunCecilifier($$"""class Foo { private int P => 0; private int M() => 0; private int f; object Bar(Foo p) => p?.{{member}}; }"""); + Assert.That( + result.GeneratedCode.ReadToEnd(), + Does.Match( + $""" + (il_bar_\d+\.Emit\(OpCodes\.)Ldarg_1\); + (\s+\1){expected}\); + \2Newobj, .+ImportReference\(.+ResolveMethod\(typeof\(System\.Nullable\), ".ctor",.+, "System.Int32"\)\)\); + \s+il_bar_\d+.Append\(lbl_conditionEnd_\d+\); + \2Box,.+ImportReference\(typeof\(System.Nullable<>\)\)\.MakeGenericInstanceType\(assembly.MainModule.TypeSystem.Int32\)\); + \2Ret\); + """)); + } + [TestCase( "int i = 42; i -= 10;", @"(il_topLevelMain_\d\.Emit\(OpCodes\.)Ldc_I4, 42\);\s+\1Stloc, (l_i_\d+)\);\s+\1Ldloc, \2\);\s+\1Ldc_I4, 10\);\s+\1Sub\);\s+\1Stloc, \2\);\s+m_topLevelStatements_1\.Body\.Instructions\.Add\(.+OpCodes\.Ret\)\);", diff --git a/Cecilifier.Core/AST/ExpressionVisitor.cs b/Cecilifier.Core/AST/ExpressionVisitor.cs index 48d261bf..53576efe 100644 --- a/Cecilifier.Core/AST/ExpressionVisitor.cs +++ b/Cecilifier.Core/AST/ExpressionVisitor.cs @@ -1175,7 +1175,7 @@ private void ProcessProperty(SimpleNameSyntax node, IPropertySymbol propertySymb propertySymbol.EnsurePropertyExists(Context, node); var isAccessOnThisOrObjectCreation = node.Parent.IsAccessOnThisOrObjectCreation(); - if (IsUnqualifiedAccess(node, propertySymbol)) + if (!propertySymbol.IsStatic && !node.IsQualifiedAccessToTypeOrMember()) { // if this is an *unqualified* access we need to load *this* Context.EmitCilInstruction(ilVar, OpCodes.Ldarg_0); @@ -1212,29 +1212,6 @@ private void HandlePropertyGetAccess(SimpleNameSyntax node, IPropertySymbol prop } } - /// - /// Checks whether the given represents an unqualified reference (i.e, only a type name) or - /// a qualified one. For instance in `Foo f = new NS.Foo();`, - /// 1. Foo => Unqualified - /// 2. NS.Foo => Qualified - /// - /// - /// Node with the name of the property being accessed. - /// Property symbol representing the property being accessed. - /// true if represents an unqualified access to , false otherwise - /// - /// A NameColon syntax represents the `Length: 42` in an expression like `o as string { Length: 42 }` - /// A MemberBindExpression represents the null coalescing operator - /// - private static bool IsUnqualifiedAccess(SimpleNameSyntax node, IPropertySymbol propertySymbol) - { - var parentMae = node.Parent as MemberAccessExpressionSyntax; - return (parentMae == null || parentMae.Expression == node) - && !node.Parent.IsKind(SyntaxKind.NameColon) - && !node.Parent.IsKind(SyntaxKind.MemberBindingExpression) - && !propertySymbol.IsStatic; - } - private void ProcessMethodReference(SimpleNameSyntax node, IMethodSymbol method) { var invocationParent = node.Ancestors().OfType() diff --git a/Cecilifier.Core/AST/SyntaxWalkerBase.cs b/Cecilifier.Core/AST/SyntaxWalkerBase.cs index 321fd058..434b8cb8 100644 --- a/Cecilifier.Core/AST/SyntaxWalkerBase.cs +++ b/Cecilifier.Core/AST/SyntaxWalkerBase.cs @@ -499,8 +499,7 @@ protected void ProcessField(string ilVar, SimpleNameSyntax node, IFieldSymbol fi var fieldDeclarationVariable = fieldSymbol.EnsureFieldExists(Context, node); - var isTargetOfQualifiedAccess = (node.Parent is MemberAccessExpressionSyntax mae) && mae.Name == node; - if (!fieldSymbol.IsStatic && !isTargetOfQualifiedAccess) + if (!fieldSymbol.IsStatic && !node.IsQualifiedAccessToTypeOrMember()) 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())) diff --git a/Cecilifier.Core/Extensions/SyntaxNodeExtensions.cs b/Cecilifier.Core/Extensions/SyntaxNodeExtensions.cs index 4ea5415c..abb21577 100644 --- a/Cecilifier.Core/Extensions/SyntaxNodeExtensions.cs +++ b/Cecilifier.Core/Extensions/SyntaxNodeExtensions.cs @@ -13,6 +13,24 @@ namespace Cecilifier.Core.Extensions { public static class SyntaxNodeExtensions { + /// + /// Checks whether the given is the name in a qualified reference. + /// + /// Node with the name of the type/member to be tested. + /// true if is the name in a qualified access, false otherwise + /// + /// Examples of qualified / unqualified access: + /// 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 + /// + public static bool IsQualifiedAccessToTypeOrMember(this SimpleNameSyntax 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 + }; + /// /// Returns a human readable summary of the containing nodes/tokens until (including) first one with a new line trivia. ///