Support target-typed new #25196
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,6 +95,11 @@ protected BoundExpression CreateConversion( | |
| return CreateAnonymousFunctionConversion(syntax, source, conversion, isCast, destination, diagnostics); | ||
| } | ||
|
|
||
| if (conversion.IsNew) | ||
| { | ||
| return CreateImplicitNewConversion(syntax, source, conversion, isCast, destination, diagnostics); | ||
| } | ||
|
|
||
| if (conversion.IsStackAlloc) | ||
| { | ||
| return CreateStackAllocConversion(syntax, source, conversion, isCast, destination, diagnostics); | ||
|
|
@@ -128,6 +133,98 @@ protected BoundExpression CreateConversion( | |
| { WasCompilerGenerated = wasCompilerGenerated }; | ||
| } | ||
|
|
||
| private BoundExpression CreateImplicitNewConversion(SyntaxNode syntax, BoundExpression source, Conversion conversion, bool isCast, TypeSymbol destination, DiagnosticBag diagnostics) | ||
| { | ||
| var node = (ObjectCreationExpressionSyntax)source.Syntax; | ||
| var arguments = ((UnboundObjectCreationExpression)source).AnalyzedArguments; | ||
|
|
||
| TypeSymbol type = destination.StrippedType(); | ||
| BoundObjectInitializerExpressionBase boundInitializerOpt = node.Initializer != null | ||
| ? BindInitializerExpression(syntax: node.Initializer, type: type, typeSyntax: syntax, diagnostics) | ||
| : null; | ||
|
|
||
| if (ReportBadTargetType(syntax, type, diagnostics)) | ||
| { | ||
| return MakeBadExpressionForObjectCreation(node, destination, boundInitializerOpt, arguments); | ||
| } | ||
|
|
||
| BoundExpression result; | ||
| switch (type.TypeKind) | ||
|
There was a problem hiding this comment.
|
||
| { | ||
| case TypeKind.Struct: | ||
| case TypeKind.Class: | ||
| result = BindClassCreationExpression( | ||
| node, | ||
| typeName: type.Name, | ||
| typeNode: node, | ||
| type: (NamedTypeSymbol)type, | ||
| arguments, | ||
| diagnostics, | ||
| boundInitializerOpt, | ||
| forTargetTypedNew: true); | ||
| break; | ||
|
|
||
| case TypeKind.TypeParameter: | ||
| result = BindTypeParameterCreationExpression( | ||
| node, | ||
| typeParameter: (TypeParameterSymbol)type, | ||
| arguments, | ||
| boundInitializerOpt, | ||
| diagnostics); | ||
| break; | ||
|
|
||
| default: | ||
| throw ExceptionUtilities.UnexpectedValue(type.Kind); | ||
| } | ||
|
|
||
| if (destination.IsNullableType()) | ||
|
There was a problem hiding this comment. May be good to add a comment on the nullable case to clarify why this is needed: |
||
| { | ||
| result = new BoundConversion( | ||
| syntax, | ||
| result, | ||
| conversion: new Conversion(ConversionKind.ImplicitNullable, Conversion.IdentityUnderlying), | ||
| @checked: false, | ||
| explicitCastInCode: isCast, | ||
| constantValueOpt: null, // A "target-typed new" would never produce a constant. | ||
| type: destination); | ||
| } | ||
|
There was a problem hiding this comment. It can't be compiler generated and explicit in code at the same time. #Resolved There was a problem hiding this comment. what values are expected for I guess the latter should be false. |
||
|
|
||
| return result; | ||
| } | ||
|
|
||
| private bool ReportBadTargetType(SyntaxNode syntax, TypeSymbol type, DiagnosticBag diagnostics) | ||
| { | ||
| switch (type.TypeKind) | ||
| { | ||
| case TypeKind.Array: | ||
| case TypeKind.Enum: | ||
| case TypeKind.Delegate: | ||
| case TypeKind.Interface: | ||
| Error(diagnostics, ErrorCode.ERR_BadTargetTypeForNew, syntax, type); | ||
| return true; | ||
|
|
||
| case TypeKind.Pointer: | ||
| Error(diagnostics, ErrorCode.ERR_UnsafeTypeInObjectCreation, syntax, type); | ||
| return true; | ||
|
|
||
| case TypeKind.Dynamic: | ||
| Error(diagnostics, ErrorCode.ERR_NoConstructors, syntax, type); | ||
| return true; | ||
|
|
||
| case TypeKind.Struct when type.IsTupleType: | ||
| Error(diagnostics, ErrorCode.ERR_NewWithTupleTypeSyntax, syntax, type); | ||
| return true; | ||
|
|
||
| case TypeKind.Struct: | ||
| case TypeKind.Class: | ||
| case TypeKind.TypeParameter: | ||
| return false; | ||
|
|
||
| default: | ||
| return true; | ||
|
There was a problem hiding this comment.
Is the default case reachable? If not, let's throw an UnexpectedValue exception. #Closed There was a problem hiding this comment. Yes, it's reachable, for example There was a problem hiding this comment. Thanks for the clarification. I think we should make the In reply to: 206795071 [](ancestors = 206795071) |
||
| } | ||
| } | ||
|
|
||
| protected BoundExpression CreateUserDefinedConversion(SyntaxNode syntax, BoundExpression source, Conversion conversion, bool isCast, TypeSymbol destination, DiagnosticBag diagnostics) | ||
| { | ||
| if (!conversion.IsValid) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1292,7 +1292,12 @@ | |
| <!-- BinderOpt is added as a temporary solution for IOperation implementation and should probably be removed in the future --> | ||
| <Field Name="BinderOpt" Type="Binder" Null="allow"/> | ||
| </Node> | ||
|
|
||
| <Node Name="UnboundObjectCreationExpression" Base="BoundExpression"> | ||
|
There was a problem hiding this comment.
I assume this node cannot survive initial binding. If that's correct, please add a comment and add something like the following to public override BoundNode VisitDeconstructionVariablePendingInference(DeconstructionVariablePendingInference node)
{
// DeconstructionVariablePendingInference nodes are only used within initial binding, but don't survive past that stage
throw ExceptionUtilities.Unreachable;
}
``` #ClosedThere was a problem hiding this comment. Since the object initializer is logically part of the (un)bound object creation expression, it should be captured here. Probably as an optional There was a problem hiding this comment. Done. Please review the follow-up PR at #29167 after this goes in. thanks. |
||
| <!-- Type is not significant for this node type; always null --> | ||
| <Field Name="Type" Type="TypeSymbol" Override="true" Null="always"/> | ||
| <Field Name="AnalyzedArguments" Type="AnalyzedArguments"/> | ||
|
There was a problem hiding this comment.
I don't think we should hold onto a pooled object in the bound tree. It's unclear who's responsible for freeing it... #Closed There was a problem hiding this comment. So you're suggesting we postpone binding arguments (potentially until we create the conversion)? In that case, we should be responsible for binding the argument list in error cases when there's no destination type to convert to e.g. There was a problem hiding this comment. Some alternatives for discussion:
I'd suggest we go with (1) for now, with a PROTOTYPE marker. In reply to: 206795081 [](ancestors = 206795081) There was a problem hiding this comment. As yet another alternative, I think we could bind the arg list in CreateImplicitNewConversion and another time in some other phase (likely DiagnosticsPass?) when we encounter a UnboundObjectCreationExpression. This way, we only bind it once, for the error cases or otherwise. |
||
| </Node> | ||
|
|
||
| <!-- | ||
| Tuple literals can exist in two forms - literal and converted literal. | ||
|
|
||
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.
As I mentioned in BoundNodes.xml, we should not require a relationship between an
UnboundObjectCreationExpressionand its syntax node. Please store the initializer (or anything else needed) in theUnboundObjectCreationExpression.