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

Fix incorrect optimization of Activator.CreateInstance #50228

Merged
merged 1 commit into from
Mar 25, 2021

Conversation

MichalStrehovsky
Copy link
Member

@MichalStrehovsky MichalStrehovsky commented Mar 25, 2021

We should not return "default" if there's a private parameterless constructor. GetDefaultConstructor only returns the default constructor in the C# sense (public parameterless ctor on a non-abstract class).

We should not return "default" if there's a private parameterless constructor. GetDefaultConstructor only calls the default constructor in the C# sense (public parameterless ctor on a non-abstract class).
@VSadov
Copy link
Member

VSadov commented Mar 25, 2021

Curious - what will happen if there is a parameterless constructor, but inaccessible. Will it throw an exception?

@MichalStrehovsky
Copy link
Member Author

Yup, MissingMethodException.

@jkotas jkotas merged commit 85e06bf into main Mar 25, 2021
@jkotas jkotas deleted the MichalStrehovsky-patch-1 branch March 25, 2021 14:41
@VSadov
Copy link
Member

VSadov commented Mar 25, 2021

I think C# is still undecided what to do in this case. - Return default for compat reasons or make it a compile error.
It seems better if behaviors match. https://github.com/dotnet/csharplang/blob/main/proposals/parameterless-struct-constructors.md#object-creation

Behavior of Activator may tip the scales towards making this an error.
CC:@cston

@MichalStrehovsky
Copy link
Member Author

AFAIK C# is tipping towards disallowing non-public parameterless constructors so this would avoid the problem.

I see it kind of as a runtime bug because the spec decided that any struct (regardless of its constructors) is going to satisfy the new constraint but we then went ahead and blocked it inconsistently for the only API where the constraint makes sense (Activator).

@ghost ghost locked as resolved and limited conversation to collaborators Apr 24, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants