Update CachedReflectionInfo class with parameter types#9029
Update CachedReflectionInfo class with parameter types#9029daxian-dbw merged 23 commits intoPowerShell:masterfrom
Conversation
e68012f to
817d44d
Compare
c3eec64 to
d995964
Compare
76e17c9 to
0197507
Compare
f8b76dd to
22472fa
Compare
b332436 to
3f910c2
Compare
|
@iSazonov I replied to your comment in the issue #9016 (comment). The update should focus on operations with public members of public types. I don't see a need to do the same for internal types. And furthermore, maybe we can ignore such operations on public interfaces too. |
|
@daxian-dbw Thanks! I already have done. It was a lot of work. One additional thought. While I was working on it, it turned out that these changes are very sensitive, while the diagnosis is very scanty. If someone changes an internal api, then pwsh can fall with a non-informative error and it will be very difficult to find the cause of the problem. The change doesn't add more information in error message but reduces the likelihood of conflicts and as a result simplify diagnostic. |
|
@iSazonov I appreciate the effort you put behind this PR, thanks! I'm not too worried about the internal types. Say you are adding an overload to one of the methods used here, then you definitely need to understand what the existing method is used for. Therefore, you will see how it's used in I'm mainly worried about the .NET public types, because we have no control on changes to them in future version of .NET Core. |
Yes, but some our namespace is so large that we have the same problem as with public. So you request is to leave only public (without interfaces)? Additional: looking how difficult to make signatures for reflection I am thinking about request to enhance .Net Core like: static readonly MethodInfo stringIndex = typeof(System.String).GetMethod("public int IndexOf(char value, StringComparison comparisonType)")
`` |
|
@daxian-dbw I found only one public type. |
There was a problem hiding this comment.
Thank you @iSazonov for making so much effort on this!
Sorry that I didn't make my intention clear enough in the issue, which caused you to spend some effort in vain :(
|
@daxian-dbw It was a useful mindful exercise. :-) Besides, I learned that refletion is sometimes very confusing. static readonly MethodInfo stringIndex = typeof(System.String).GetMethod("public int IndexOf(char value, StringComparison comparisonType)")or public delegate int IndexOfDelegate(char value, StringComparison comparisonType)
static readonly MethodInfo stringIndex = typeof(System.String).GetMethod(IndexOfDelegate) |
|
I don't think they will accept these proposals. The reflection APIs have been there for a long time, and I double they will do any enhancement on them now.
This means the string has to be parsed to the real signature, meaning that the compiler-ish thing needs to kick in ... that's a lot work for not much value.
This means you have to define a delegate for the reflection operation, it's almost same as constructing the |
PR Summary
Related #9016
Use GetMethod overload which allow to specify types of parameters.
PR Context
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.[feature]to your commit messages if the change is significant or affects feature tests