X Tutup
The Wayback Machine - https://web.archive.org/web/20220814024727/https://github.com/PowerShell/PowerShell/pull/17795
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use null propagation operator for windows build #17795

Merged
merged 7 commits into from Jul 29, 2022

Conversation

fflaten
Copy link
Contributor

@fflaten fflaten commented Jul 28, 2022

PR Summary

Replace if not null-checks with null propagation operator to solve IDE0031 with .NET Preview 7.
Additional findings when compiled against win7-x64 (in devcontainer).

PR Context

Related to #17769

PR Checklist

@fflaten
Copy link
Contributor Author

fflaten commented Jul 28, 2022

Don't have a Windows-machine to build on atm. but hopefully I caught most of them.

@iSazonov Any tips regarding 31408eb ?
When building in devcontainer with -Runtime win7-x64 and changing them to securityDescHandle?.Value.Free(); it failed with:

/workspaces/PowerShell/src/System.Management.Automation/engine/remoting/common/RunspaceConnectionInfo.cs(2850,32): error CS1061: 'GCHandle' does not contain a definition for 'Value' and no accessible extension method 'Value' accepting a first argument of type 'GCHandle' could be found (are you missing a using directive or an assembly reference?) [/workspaces/PowerShell/src/System.Management.Automation/System.Management.Automation.csproj]
/workspaces/PowerShell/src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs(507,32): error CS1061: 'GCHandle' does not contain a definition for 'Value' and no accessible extension method 'Value' accepting a first argument of type 'GCHandle' could be found (are you missing a using directive or an assembly reference?) [/workspaces/PowerShell/src/System.Management.Automation/System.Management.Automation.csproj]

Update: Never mind. ?. removes nullable from the type.

src/Microsoft.WSMan.Management/WsManHelper.cs Outdated Show resolved Hide resolved
src/Microsoft.WSMan.Management/WsManHelper.cs Outdated Show resolved Hide resolved
@msftbot msftbot bot added the Waiting on Author label Jul 28, 2022
@msftbot msftbot bot removed the Waiting on Author label Jul 28, 2022
Copy link
Collaborator

@PaulHigin PaulHigin left a comment

LGTM

{
securityDescHandle.Value.Free();
}
securityDescHandle?.Free();
Copy link
Collaborator

@iSazonov iSazonov Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It generates more efficient code. I wonder but it is great!
Compere:
before
after

@PaulHigin PaulHigin merged commit 5f4e100 into PowerShell:master Jul 29, 2022
39 checks passed
@fflaten fflaten deleted the fix-nullprop-win branch Jul 29, 2022
@iSazonov iSazonov added the CL-CodeCleanup label Jul 31, 2022
@fflaten fflaten mentioned this pull request Aug 1, 2022
22 tasks
@msftbot
Copy link

msftbot bot commented Aug 11, 2022

🎉v7.3.0-preview.7 has been released which incorporates this pull request.🎉

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-CodeCleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants
X Tutup