Enable C# 9.0 features by iSazonov · Pull Request #13090 · PowerShell/PowerShell

iSazonov · Jul 3, 2020

PR Summary

Enable C# 9.0 features. Mainly to get is not for patterns like if (!(obj is null)) -> if(obj is not null)

PR Context

PR Checklist

daxian-dbw · Jul 7, 2020

I have a concern that a language feature in preview may not make to the official release, so what if we start to use a syntax in code and that feature is pulled out?

One feature I want is new is not. It is unbelievable that it will be removed.
Other new C# features are very specific and are not of interest at the moment. We will reject its till release in code reviews.
We already did this when Span appeared.

I think it's a valid concern, but the risk is low, especially if we're judicious about which features we use, as @iSazonov says. We're already taking .NET preview versions, and I'd say the risk of needing to roll those back is much greater.

As long as we can for a non-preview version of PowerShell ensure that we pin to a non-preview version of the C# language then I'd be happy with this change.

But it needs extra rigour to ensure that we don ship a 7.0.3 / 7.1.0 that uses preview language versions.

I expect most of that rigour is already in place, but still needed to be called out, just in case

Well we don't synchronise to C# language version releases like .NET releases, so we are forced to retain preview language versions for stable releases. When release time comes, pinning to a stable version would mean downgrading the version and changing all the code relying on the preview features we've been using to that point, which is bad for several reasons.

But I don't think it's uncommon to build a stable product on a preview language version, and unlike .NET preview releases, the language features all compile down to the same things. Using a preview C# version will affect the development, but is unlikely to affect the product.

I think the real risk is that a preview feature is taken out of the language because it doesn't meet some bar, at which point we'll need to go and change all the code that uses that feature in order to build again.

I'll add that it makes backporting harder theoretically, but so would just incrementing the language version to another stable version.

From what I can gather, this is actually a moot discussion because C# 9 should stabilise with .NET 5. The only risk that remains then is that something we depend on in preview does not progress to stable. However, the risk here is identical to relying on a preview .NET API -- we would need to revert to using the old implementation if it happened.

Thanks for the clarification @rjmholt that makes a lot of sense :-)

I've followed up with the .NET team on this to understand if our assumptions here can be relied on. I think until that's the case, it's fair for us to hold this PR

Ok I’ve confirmed that C# 9 is planned to stabilise with .NET 5 — so our only remaining concern is the tolerance of removed preview features

Current plan is only modernizing our code with is not null feature. No plans exist to use other new features in near month or two before C# 9.0 will be stabilized.

rjmholt · Jul 9, 2020

Looks like there's a test failure on Unix:

Describing Validate start of console host
Assemblies that are loaded but not expected: System.Reflection.Emit.dll
    [-] No new assemblies are loaded 20ms
      Expected exactly $null, but got @{InputObject=System.Reflection.Emit.dll; SideIndicator==>}.
      119:         $diffs | Should -BeExactly $null
      at <ScriptBlock>, /Users/runner/work/1/s/test/powershell/Host/Startup.Tests.ps1: line 119

I've rerun the tests, but they may fail again

iSazonov · Jul 9, 2020

Rebased... maybe resolve CI failures.

TravisEz13 · Jul 9, 2020


    <TargetFramework>net5.0</TargetFramework>
-    <LangVersion>8.0</LangVersion>
-    <PublishReadyToRun>true</PublishReadyToRun>


This should not be removed

Opps... Fixed.

rjmholt · Jul 10, 2020

@iSazonov please resolve conflicts (I know they're because I merged the other PR...)

rjmholt · Jul 10, 2020

@TravisEz13 please re-review

Review has been addressed and is now outdated

iSazonov · Jul 22, 2020

@xtqqczze If you want you could cleanup code (I will review) with var == null -> var is null, var != null -> var is not null (excluding UI.Internal and WinRM code).

xtqqczze · Jul 23, 2020

@iSazonov Yes I can cleanup code. I will open seperate PR for var == null -> var is null as this is not C# 9 change.

ghost · Aug 17, 2020

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

Handy links:

Release Notes

iSazonov added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Jul 3, 2020

iSazonov requested review from TravisEz13, adityapatwardhan, anmenaga, daxian-dbw and rjmholt July 3, 2020 13:23

iSazonov assigned rjmholt Jul 3, 2020

rjmholt approved these changes Jul 3, 2020

View reviewed changes

daxian-dbw approved these changes Jul 7, 2020

View reviewed changes

daxian-dbw reviewed Jul 7, 2020

View reviewed changes

TravisEz13 previously requested changes Jul 8, 2020

View reviewed changes

Comment thread PowerShell.Common.props Outdated

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Jul 8, 2020

iSazonov changed the title ~~Disable R2R for debug build and enable C# 9.0 features~~ Enable C# 9.0 features Jul 9, 2020

rjmholt mentioned this pull request Jul 9, 2020

Remove rcedit dependency, move daily ico dependency to props file #13123

Merged

14 tasks

iSazonov added 2 commits July 9, 2020 22:33

Disable R2R for debug build

93eda9c

Revert change for R2R

334d363

iSazonov force-pushed the speedup-debug-compile branch from c9c152a to 334d363 Compare July 9, 2020 17:34

TravisEz13 reviewed Jul 9, 2020

View reviewed changes

Fix typo.

586cf11

Merge branch 'master' into speedup-debug-compile

3d6f21b

rjmholt requested a review from TravisEz13 July 10, 2020 17:28

rjmholt merged commit e88e8bd into PowerShell:master Jul 21, 2020

iSazonov deleted the speedup-debug-compile branch July 22, 2020 04:15

iSazonov added this to the 7.1.0-preview.7 milestone Jul 22, 2020

This was referenced Jul 25, 2020

Use is null syntax #13277

Merged

Use is not null syntax #13286

Closed

TravisEz13 modified the milestones: 7.1.0-preview.7, 7.1.0-preview.6 Aug 5, 2020

Search code, repositories, users, issues, pull requests...

Conversation

iSazonov commented Jul 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjmholt Jul 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rjmholt commented Jul 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Jul 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjmholt commented Jul 10, 2020

Uh oh!

rjmholt commented Jul 10, 2020

Uh oh!

iSazonov commented Jul 22, 2020

Uh oh!

xtqqczze commented Jul 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Aug 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

iSazonov commented Jul 3, 2020 •

edited

Loading

rjmholt Jul 9, 2020 •

edited

Loading

rjmholt commented Jul 9, 2020 •

edited

Loading

xtqqczze commented Jul 23, 2020 •

edited

Loading