Fix Activator.CreateInstance case #2146
Conversation
When no ctor parameters are passed, and non-public binding flags are used, we only need to require parameterless constructors.
|
Test case? |
|
Yup, I just wanted to prioritize figuring out the other warnings. |
| [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor | DynamicallyAccessedMemberTypes.NonPublicConstructors), | ||
| KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] Type type) | ||
| { | ||
| Activator.CreateInstance (type, nonPublic: true); |
There was a problem hiding this comment.
How does this test work? I assume we're trying to assert that the given type has its non-public constructors preserved, but how are we confirming that we're also not marking the other public constructors?
Wasn't the problem in the PR that we were marking extra things and getting additional warnings?
There was a problem hiding this comment.
Not really - the problem is that PublicParameterlessConstructor | NonPublicConstructors was an annotation which was not considered sufficient for calling CreateInstance(type, true). Which it should be.
There was a problem hiding this comment.
Wasn't the problem in the PR that we were marking extra things and getting additional warnings?
I may have given you the wrong impression when we chatted - this additional warning was because we tried to require (not mark) extra things on the input (which was an abstract DAMT value). But the extra requirements also would have resulted in extra things being marked for concrete input types (that just wasn't the failure mode we happened to hit).
* Fix Activator.CreateInstance case When no ctor parameters are passed, and non-public binding flags are used, we only need to require parameterless constructors. * Add test Commit migrated from dotnet/linker@2174dc1
When no ctor parameters are passed, and non-public binding flags are
used, we only need to require parameterless public constructors. Fixes one of the issues in dotnet/runtime#55636.