Skip to content

Conversation

@DoctorKrolic
Copy link
Contributor

Closes #3495

This is my first contribution to this project, so to found places in code I needed I used full text search on the solution. Besides the place I changed there is also this piece:

case Call c when c.Method.FullNameIs("System.Activator", "CreateInstance") && c.Method.TypeArguments.Count == 1:
IType type = c.Method.TypeArguments[0];
initObjRR = new InitializedObjectResolveResult(type);
expr = new ObjectCreateExpression(ConvertType(type))
.WithILInstruction(c)
.WithRR(new TypeResolveResult(type));
break;

I was never able to hit this code path when I debugged added test case. Does this logic also need to be changed and if so, what test should I add to cover it?

@dgrunwald
Copy link
Member

I think that extra logic you found is used when dealing with new T { 1, 2, 3 } (where T : IList<int>, new()).
In that case you can't avoid the transformation to new T syntax; you could only disable collection initializer syntax.

@DoctorKrolic
Copy link
Contributor Author

In that case you can't avoid the transformation to new T syntax

Why not? If both collection initializer syntax and new T are disabled, we can get this:
Before:

static T Test<T>()
    where T : IList<int>, new()
{
    return new T()
    {
        1, 2, 3
    };
}

After:

static T Test<T>()
    where T : IList<int>, new()
{
    T t = Activator.CreateInstance<T>();
    t.Add(1);
    t.Add(2);
    t.Add(3);
    return t;
}

@siegfriedpammer
Copy link
Member

I am not a fan of the setting name GenericTypeInstantiation...

Copilot says:

"GenericTypeInstantiation" is the act of creating a constructed type from a generic type definition by supplying actual type arguments (e.g., turning List<T> into List<string>).

System.Reflection.Metadata uses the term to refer to construction of generic type instances, see ISignatureTypeProvider.GetGenericInstantiation. It's also used in that way throughout ILSpy's source code.

We should probably use something like UseObjectCreationOfGenericTypeParameter but that is not really succinct.

Does this logic also need to be changed and if so, what test should I add to cover it?

It's sufficient to add the check here:

case Call c when c.Method.FullNameIs("System.Activator", "CreateInstance") && c.Method.TypeArguments.Count == 1:
instType = c.Method.TypeArguments[0];
blockKind = BlockKind.ObjectInitializer;
break;

just like we already do for with initializers:

case CallInstruction ci when context.Settings.WithExpressions && IsRecordCloneMethodCall(ci):
instType = ci.Method.DeclaringType;
blockKind = BlockKind.WithInitializer;
initInst = ci.Arguments.Single();
break;

Then, if the setting is disabled, the initializer transform won't be applied in this specific case only.

@siegfriedpammer
Copy link
Member

Upon reading the code referenced above again, I think this should be implemented by adding if (!context.Settings.YourOptionName) { return; } in the respective case section, to make it easier to read.

@siegfriedpammer siegfriedpammer merged commit 7d0451a into icsharpcode:master Jul 12, 2025
5 checks passed
@siegfriedpammer
Copy link
Member

Thank you for your contribution!

@DoctorKrolic DoctorKrolic deleted the no-new-T branch July 13, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an option to not transform Activator.CreateInstance<T>() to new T()

3 participants