8000 Ensure null-coalescing LHS is evaluated only once by rjmholt · Pull Request #12667 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Ensure null-coalescing LHS is evaluated only once#12667

Merged
iSazonov merged 4 commits intoPowerShell:masterfrom
rjmholt:fix-null-coalescing-evaluation
May 16, 2020
Merged

Ensure null-coalescing LHS is evaluated only once#12667
iSazonov merged 4 commits intoPowerShell:masterfrom
rjmholt:fix-null-coalescing-evaluation

Conversation

@rjmholt
Copy link
Collaborator
@rjmholt rjmholt commented May 14, 2020

PR Summary

Fixes #12655.

The null coalescing operator currently double-evaluates its LHS, which is an issue if it has side effects. This changes evaluates the LHS, saves the result to a temporary variable and then proceeds with the conditional operator using the variable.

This PR also removes a redundant type check for AutomationNull that could never be satisfied due to the implementation of AutomationNull.

This PR should be considered for servicing in PS 7

PR Context

LHS in null coalescing expressions is currently double evaluated. It should only be evaluated once.

PR Checklist

/cc @SeeminglyScience

@rjmholt rjmholt requested a review from daxian-dbw as a code owner May 14, 2020 21:54
@ghost ghost assigned iSazonov May 14, 2020
@rjmholt rjmholt requested a review from adityapatwardhan May 14, 2020 21:55
@rjmholt rjmholt force-pushed the fix-null-coalescing-evaluation branch from 0407a9c to fddb646 Compare May 14, 2020 22:12
@rjmholt rjmholt added the WG-Engine core PowerShell engine, interpreter, and runtime label May 15, 2020
@rjmholt rjmholt added this to the 7.0.x-Servicing-Consider milestone May 15, 2020
iSazonov
iSazonov reviewed May 15, 2020
View reviewed changes
@iSazonov iSazonov added CL-Engine Indicates that a PR should be marked as an engine change in the Change Log and removed WG-Engine core PowerShell engine, interpreter, and runtime labels May 15, 2020
@iSazonov iSazonov merged commit 3dfd95a into PowerShell:master May 16, 2020
@ghost
Copy link
ghost commented Jun 11, 2020

🎉v7.0.2 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Jun 11, 2020
Closed
@ghost
Copy link
ghost commented Jun 25, 2020

🎉v7.1.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

silijon pushed a commit to SkyKick/PowerShell that referenced this pull request Jul 2, 2020
@rjmholt @TravisEz13
Ensure null-coalescing LHS is evaluated only once (PowerShell#12667)
92a737b 
# Conflicts:
#	src/System.Management.Automation/engine/parser/Compiler.cs
@jshealth jshealth mentioned this pull request Jul 14, 2020
Closed
@rjmholt rjmholt deleted the fix-null-coalescing-evaluation branch July 30, 2020 04:33
@TravisEz13 TravisEz13 mentioned this pull request Oct 12, 2022
Merged
22 tasks
@SeeminglyScience SeeminglyScience mentioned this pull request Jan 12, 2025
Open
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes

@iSazonov iSazonov iSazonov approved these changes

@adityapatwardhan adityapatwardhan Awaiting requested review from adityapatwardhan

+1 more reviewer

@vexx32 vexx32 vexx32 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@iSazonov iSazonov

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Milestone

7.0.2

Development

Successfully merging this pull request may close these issues.

The null-coalescing operator evaluates the left-hand operand twice.

5 participants

@rjmholt @daxian-dbw @iSazonov @vexx32 @TravisEz13
3361
0