8000 Enable nullable: System.Management.Automation.IDispatch by powercode · Pull Request #14185 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Enable nullable: System.Management.Automation.IDispatch #14185

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 26, 2021
Merged

Enable nullable: System.Management.Automation.IDispatch #14185

merged 3 commits into from
May 26, 2021

Conversation

powercode
Copy link
Collaborator

Tracking issue: #12631.

@ghost ghost assigned anmenaga Nov 20, 2020
@ghost ghost added the Review - Needed The PR is being reviewed label Nov 28, 2020
@ghost
Copy link
ghost commented Nov 28, 2020

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@rjmholt
Copy link
Collaborator
rjmholt commented Dec 11, 2020

Don't know about this one -- COM isn't my area

@ghost ghost removed the Review - Needed The PR is being reviewed label Dec 11, 2020
@iSazonov
Copy link
Collaborator
iSazonov commented Dec 11, 2020 •
edited by unfurl-links bot
Loading

@daxian-dbw Could you please look? There is a difference with .Net Runtime
https://github.com/dotnet/runtime/blob/1d9e50cb4735df46d3de0cee5791e97295eaf588/src/libraries/Common/src/System/Runtime/InteropServices/IDispatch.cs#L25-L35

GitHub
.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps. - dotnet/runtime

@powercode
Copy link
Collaborator Author
powercode commented Dec 14, 2020 •
edited by unfurl-links bot
Loading

The relevant docs are here:
https://docs.microsoft.com/en-us/windows/win32/api/oaidl/nf-oaidl-idispatch-gettypeinfo
https://docs.microsoft.com/en-us/windows/win32/api/oaidl/nf-oaidl-idispatch-invoke

Retrieves the type information for an object, which can then be used to get the type information for an interface.
Provides access to properties and methods exposed by an object.

powercode
powercode commented Dec 14, 2020
View reviewed changes
src/System.Management.Automation/engine/COM/ComDispatch.cs
@@ -34,7 +36,7 @@ internal interface IDispatch
int lcid,
COM.INVOKEKIND wFlags,
[In, Out][MarshalAs(UnmanagedType.LPArray)] COM.DISPPARAMS[] paramArray,
out object pVarResult,
out object? pVarResult,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

VARIANT *pVarResult,

Pointer to the location where the result is to be stored, or NULL if the caller expects no result. This argument is ignored if DISPATCH_PROPERTYPUT or DISPATCH_PROPERTYPUTREF is specified.

powercode
powercode commented Dec 14, 2020
View reviewed changes
src/System.Management.Automation/engine/COM/ComDispatch.cs
@@ -34,7 +36,7 @@ internal interface IDispatch
int lcid,
COM.INVOKEKIND wFlags,
[In, Out][MarshalAs(UnmanagedType.LPArray)] COM.DISPPARAMS[] paramArray,
out object pVarResult,
out object? pVarResult,
out ComInvoker.EXCEPINFO pExcepInfo,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

EXCEPINFO* pExcepInfo

Pointer to a structure that contains exception information. This structure should be filled in if DISP_E_EXCEPTION is returned. Can be NULL.

This can be null, but as we use a struct for ComInvoker.EXCEPTINFO, it shouldn't be marked as ?

@SteveL-MSFT SteveL-MSFT added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Dec 15, 2020
@ghost ghost added the Review - Needed The PR is being reviewed label Dec 23, 2020
@ghost
Copy link
ghost commented Dec 23, 2020

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@powercode
Copy link
Collaborator Author

ppTInfo can for sure be null.

@iSazonov iSazono 8000 v merged commit 6361141 into PowerShell:master May 26, 2021
@ghost ghost removed the Review - Needed The PR is being reviewed label May 26, 2021
@iSazonov iSazonov assigned iSazonov and unassigned anmenaga May 26, 2021
@iSazonov iSazonov added this to the 7.2.0-preview.6 milestone May 26, 2021
@TravisEz13 TravisEz13 removed this from the 7.2.0-preview.6 milestone May 27, 2021
@powercode powercode deleted the nullable/IDispatch branch June 24, 2021 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iSazonov iSazonov iSazonov approved these changes

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw

@rjmholt rjmholt Awaiting requested review from rjmholt

Assignees

@iSazonov iSazonov

Labels
CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@powercode @rjmholt @iSazonov @TravisEz13 @SteveL-MSFT @anmenaga
0