From 1d709b4bcf9baf28afb60b6fef1316177fdc3ca0 Mon Sep 17 00:00:00 2001 From: vsadov Date: Mon, 27 Mar 2017 11:41:52 -0700 Subject: [PATCH] CR feedback --- .../AsyncRewriter/AwaitExpressionSpiller.cs | 2 +- .../Lowering/LocalRewriter/LocalRewriter.cs | 25 +++++++------------ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs index 750d233869b..1306f545dbe 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs @@ -772,7 +772,7 @@ public override BoundNode VisitCall(BoundCall node) private static RefKind ReceiverSpillRefKind(BoundExpression receiver) { - return LocalRewriter.ReceiverCanBeAssigned(receiver) ? + return LocalRewriter.WouldBeAssignableIfUsedAsMethodReceiver(receiver) ? RefKind.Ref : RefKind.None; } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs index effbcaeb343..39ecbe0cbb3 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs @@ -500,31 +500,24 @@ private static bool ShouldOptimizeOutInitializer(BoundStatement initializer) } /// - /// Receivers of struct methods are alowed to be RValues or LValues. + /// Receivers of struct methods are required to be at least RValues but can be assignable variables. /// Whether the mutations from the method are propagated back to the - /// receiver instance is conditional on whether the receiver is an LValue. + /// receiver instance is conditional on whether the receiver is a variable that can be assigned. + /// If not, then the invokation is performed on a copy. /// - /// An inconvenient situation may arise when an the receiver is an RValue expression, + /// An inconvenient situation may arise when the receiver is an RValue expression (like a ternary operator), /// which is trivially reduced during lowering to one of its operands and - /// such operand happens to be an LValue. That operation alone would + /// such operand happens to be an assignable variable (like a local). That operation alone would /// expose the operand to mutations while it would not be exposed otherwise. /// I.E. the transformation becomes semantically observable. /// /// To prevent such situations, we will wrap the operand into a node whose only - /// purpose is to never be an LValue. + /// purpose is to never be an assignable expression. /// - private static BoundExpression EnsureNotLvalueReceiver(BoundExpression expr) + private static BoundExpression EnsureNotAssignableIfUsedAsMethodReceiver(BoundExpression expr) { // Leave as-is where receiver mutations cannot happen. - if (!ReceiverCanBeAssigned(expr)) - { - return expr; - } - - // - reference type receivers are byval - // - special value types do not have mutating members - var type = expr.Type.OriginalDefinition; - if (type.IsReferenceType || type.SpecialType != SpecialType.None) + if (!WouldBeAssignableIfUsedAsMethodReceiver(expr)) { return expr; } @@ -540,7 +533,7 @@ private static BoundExpression EnsureNotLvalueReceiver(BoundExpression expr) { WasCompilerGenerated = true }; } - internal static bool ReceiverCanBeAssigned(BoundExpression receiver) + internal static bool WouldBeAssignableIfUsedAsMethodReceiver(BoundExpression receiver) { // - reference type receivers are byval // - special value types (int32, Nullable, . .) do not have mutating members -- GitLab