-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
Add type inference support for generic methods with type parameters #16951
Add type inference support for generic methods with type parameters #16951
Conversation
Apparently the changes that got merged today made by code incompatible. I guess I should have tested it on the latest master before making this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Ilya <darpa@yandex.ru>
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
…artinGC94/PowerShell into TypeInferenceForGenericMethods
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Ilya <darpa@yandex.ru>
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the long delay in review. Looks good to me overall with 2 comments.
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
@MartinGC94 I refactored your change a little bit to avoid calling |
src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
Outdated
Show resolved
Hide resolved
/rebase |
Started rebase: https://github.com/PowerShell/PowerShell/actions/runs/2387740157
|
@MartinGC94 The PR needs to be rebased to get rid of a test failure, but we are not able to force push to your branch. Can you please rebase the |
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
🎉 Handy links: |
PR Summary
Fixes #16934
Now you can tab complete based on the expected output from a method that is invoked with type parameters such as
[array]::Empty[int]().<Tab>
PR Context
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).