Skip to content

Use more effective evaluation order in LanguagePrimitives.IsNullLike() - #10781

Merged
Ilya (iSazonov) merged 1 commit into
PowerShell:masterfrom
vexx32:IsNullLike
Oct 16, 2019
Merged

Ilya (iSazonov) merged 1 commit into
PowerShell:masterfrom
vexx32:IsNullLike

Conversation

@vexx32

@vexx32 Rain Sallow (/u/ta11ow) (vexx32) commented Oct 12, 2019

Copy link
Copy Markdown
Collaborator

PR Summary

Refactor IsNullLike() to evaluate the most commonly-expected null-like values first.

New order of evaluation:

  1. IsNull(value):
    1. value == null
    2. value == AutomationNull.Value
  2. value == DBNull.Value
  3. value == AutomationNull.Value

PR Context

IsNullLike() is used in binders like -eq (e.g., $null -eq $value) and it makes the most sense to evaluate in order of the most common null/null-like values first so that the code path short-circuits in the most common situations.

PR Checklist

@iSazonov

Ilya (iSazonov) commented Oct 12, 2019

Copy link
Copy Markdown
Collaborator

It was in #9794 (milestone 7.0-Preview2) as a breaking change without PowerShell Committee approve. So add the labels.

@iSazonov Ilya (iSazonov) added Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Oct 12, 2019
@iSazonov Ilya (iSazonov) added this to the 7.0.0-preview.5 milestone Oct 12, 2019
@vexx32

Rain Sallow (/u/ta11ow) (vexx32) commented Oct 13, 2019

Copy link
Copy Markdown
Collaborator Author

Ilya (@iSazonov) the committee approved the change in the original issue (see #9561 (comment)) 🙂

@iSazonov

Copy link
Copy Markdown
Collaborator

My concern about order.

@daxian-dbw

Copy link
Copy Markdown
Member

Why is changing the order a breaking change? It doesn't change the behavior of this method, does it?

@vexx32

Copy link
Copy Markdown
Collaborator Author

Nope, it still checks the same things, just in a different order. 🙂

@iSazonov Ilya (iSazonov) added CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log and removed Breaking-Change breaking change that may affect users CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Oct 15, 2019
@iSazonov

Copy link
Copy Markdown
Collaborator

I see the logic is more simple than I thought.

@iSazonov Ilya (iSazonov) self-assigned this Oct 16, 2019
@iSazonov
Ilya (iSazonov) merged commit 25286eb into PowerShell:master Oct 16, 2019
@iSazonov Ilya (iSazonov) changed the title LanguagePrimitives.IsNullLike() - use more effective evaluation order Use more effective evaluation order in LanguagePrimitives.IsNullLike() Oct 16, 2019
@ghost

Copy link
Copy Markdown

🎉v7.0.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

Dongbo Wang (daxian-dbw) added a commit to daxian-dbw/PowerShell that referenced this pull request Jan 14, 2020
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
PowerShell#10781)

Refactor IsNullLike() to evaluate the most commonly-expected null-like values first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants