Ensure null-coalescing LHS is evaluated only once (#12667)

PowerShell · May 16, 2020 · 3dfd95a · 3dfd95a
1 parent 270eabc
commit 3dfd95a
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 10 deletions.
diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs
@@ -982,19 +982,23 @@ private static Expression Coalesce(Expression left, Expression right)
             {
                 return left;
             }
-            else if (leftType == typeof(AutomationNull))
-            {
-                return right;
-            }
             else
             {
-                Expression lhs = left.Cast(typeof(object));
-                Expression rhs = right.Cast(typeof(object));
+                ParameterExpression lhsStoreVar = Expression.Variable(typeof(object));
+                var blockParameters = new ParameterExpression[] { lhsStoreVar };
+                var blockStatements = new Expression[]
+                {
+                    Expression.Assign(lhsStoreVar, left.Cast(typeof(object))),
+                    Expression.Condition(
+                        Expression.Call(CachedReflectionInfo.LanguagePrimitives_IsNull, lhsStoreVar),
+                        right.Cast(typeof(object)),
+                        lhsStoreVar),
+                };
 
-                return Expression.Condition(
-                    Expression.Call(CachedReflectionInfo.LanguagePrimitives_IsNull, lhs),
-                    rhs,
-                    lhs);
+                return Expression.Block(
+                    typeof(object),
+                    blockParameters,
+                    blockStatements);
             }
         }
 

diff --git a/test/powershell/Language/Operators/NullConditional.Tests.ps1 b/test/powershell/Language/Operators/NullConditional.Tests.ps1
@@ -172,6 +172,18 @@ Describe 'NullCoalesceOperations' -Tags 'CI' {
         It 'Lhs is $?' {
             {$???$false} | Should -BeTrue
         }
+
+        It 'Should only evaluate LHS once when it IS null' {
+            $testState = [pscustomobject]@{ Value = 0 }
+            (& { [void]$testState.Value++ }) ?? 'Nothing' | Should -BeExactly 'Nothing'
+            $testState.Value | Should -Be 1
+        }
+
+        It 'Should only evaluate LHS once when it is NOT null' {
+            $testState = [pscustomobject]@{ Value = 0 }
+            (& { 'Test'; [void]$testState.Value++ }) ?? 'Nothing' | Should -BeExactly 'Test'
+            $testState.Value | Should -Be 1
+        }
     }
 
     Context 'Null Coalesce ?? operator precedence' {