From 73015314c840d4ac4bca9908b63211e054c62631 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 30 Oct 2018 11:47:24 -0700 Subject: [PATCH] Invert the binding order of InitializerExpressions and CreationExpressions (#30805) * Invert the order of binding for InitializerExpressions and CreationExpressions: - Push intializer binding into the respective creation expression bindings so that initializer binding takes place *after* the creation, ensuring any referenced out vars have already been inferred, preventing infinite loops - Add tests to show behavior --- .../Portable/Binder/Binder_Expressions.cs | 85 ++++++++++++------- .../Test/Semantic/Semantics/OutVarTests.cs | 47 ++++++++++ 2 files changed, 102 insertions(+), 30 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs index 5ceeb5ad159..ad591055159 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs @@ -3703,36 +3703,29 @@ protected BoundExpression BindObjectCreationExpression(ObjectCreationExpressionS { var typeWithAnnotations = BindType(node.Type, diagnostics); var type = typeWithAnnotations.TypeSymbol; + var originalType = type; if (typeWithAnnotations.IsAnnotated && !type.IsNullableType()) { diagnostics.Add(ErrorCode.ERR_AnnotationDisallowedInObjectCreation, node.Location, type); } - BoundObjectInitializerExpressionBase boundInitializerOpt = node.Initializer == null ? - null : - BindInitializerExpression( - syntax: node.Initializer, - type: type, - typeSyntax: node.Type, - diagnostics: diagnostics); - switch (type.TypeKind) { case TypeKind.Struct: case TypeKind.Class: case TypeKind.Enum: case TypeKind.Error: - return BindClassCreationExpression(node, (NamedTypeSymbol)type, GetName(node.Type), boundInitializerOpt, diagnostics); + return BindClassCreationExpression(node, (NamedTypeSymbol)type, GetName(node.Type), diagnostics, originalType); case TypeKind.Delegate: return BindDelegateCreationExpression(node, (NamedTypeSymbol)type, diagnostics); case TypeKind.Interface: - return BindInterfaceCreationExpression(node, (NamedTypeSymbol)type, boundInitializerOpt, diagnostics); + return BindInterfaceCreationExpression(node, (NamedTypeSymbol)type, diagnostics); case TypeKind.TypeParameter: - return BindTypeParameterCreationExpression(node, (TypeParameterSymbol)type, boundInitializerOpt, diagnostics); + return BindTypeParameterCreationExpression(node, (TypeParameterSymbol)type, diagnostics); case TypeKind.Submission: // script class is synthesized and should not be used as a type of a new expression: @@ -3930,7 +3923,7 @@ private BoundExpression BindDelegateCreationExpression(ObjectCreationExpressionS } } - private BoundExpression BindClassCreationExpression(ObjectCreationExpressionSyntax node, NamedTypeSymbol type, string typeName, BoundObjectInitializerExpressionBase boundInitializerOpt, DiagnosticBag diagnostics) + private BoundExpression BindClassCreationExpression(ObjectCreationExpressionSyntax node, NamedTypeSymbol type, string typeName, DiagnosticBag diagnostics, TypeSymbol initializerType = null) { // Get the bound arguments and the argument names. AnalyzedArguments analyzedArguments = AnalyzedArguments.GetInstance(); @@ -3944,15 +3937,15 @@ private BoundExpression BindClassCreationExpression(ObjectCreationExpressionSynt if (type.IsStatic) { diagnostics.Add(ErrorCode.ERR_InstantiatingStaticClass, node.Location, type); - return MakeBadExpressionForObjectCreation(node, type, boundInitializerOpt, analyzedArguments); + return MakeBadExpressionForObjectCreation(node, type, analyzedArguments, diagnostics); } else if (node.Type.Kind() == SyntaxKind.TupleType) { diagnostics.Add(ErrorCode.ERR_NewWithTupleTypeSyntax, node.Type.GetLocation()); - return MakeBadExpressionForObjectCreation(node, type, boundInitializerOpt, analyzedArguments); + return MakeBadExpressionForObjectCreation(node, type, analyzedArguments, diagnostics); } - return BindClassCreationExpression(node, typeName, node.Type, type, analyzedArguments, diagnostics, boundInitializerOpt); + return BindClassCreationExpression(node, typeName, node.Type, type, analyzedArguments, diagnostics, node.Initializer, initializerType); } finally { @@ -3960,13 +3953,17 @@ private BoundExpression BindClassCreationExpression(ObjectCreationExpressionSynt } } - private BoundExpression MakeBadExpressionForObjectCreation(ObjectCreationExpressionSyntax node, TypeSymbol type, BoundExpression boundInitializerOpt, AnalyzedArguments analyzedArguments) + private BoundExpression MakeBadExpressionForObjectCreation(ObjectCreationExpressionSyntax node, TypeSymbol type, AnalyzedArguments analyzedArguments, DiagnosticBag diagnostics) { var children = ArrayBuilder.GetInstance(); children.AddRange(BuildArgumentsForErrorRecovery(analyzedArguments)); - if (boundInitializerOpt != null) + if (node.Initializer != null) { - children.Add(boundInitializerOpt); + var boundInitializer = BindInitializerExpression(syntax: node.Initializer, + type: type, + typeSyntax: node.Type, + diagnostics: diagnostics); + children.Add(boundInitializer); } return new BoundBadExpression(node, LookupResultKind.NotCreatable, ImmutableArray.Create(type), children.ToImmutableAndFree(), type); @@ -4711,7 +4708,8 @@ private bool IsConstructorAccessible(MethodSymbol constructor, ref HashSet useSiteDiagnostics = null; + BoundObjectInitializerExpressionBase boundInitializerOpt = null; // If we have a dynamic argument then do overload resolution to see if there are one or more // applicable candidates. If there are, then this is a dynamic object creation; we'll work out @@ -4744,6 +4743,7 @@ private bool IsConstructorAccessible(MethodSymbol constructor, ref HashSet.GetInstance(); childNodes.AddRange(BuildArgumentsForErrorRecovery(analyzedArguments, candidateConstructors)); - if (boundInitializerOpt != null) + if (initializerSyntaxOpt != null) { - childNodes.Add(boundInitializerOpt); + childNodes.Add(boundInitializerOpt ?? makeBoundInitializerOpt()); } return new BoundBadExpression(node, resultKind, symbols.ToImmutableAndFree(), childNodes.ToImmutableAndFree(), type); + + BoundObjectInitializerExpressionBase makeBoundInitializerOpt() + { + if (initializerSyntaxOpt != null) + { + return BindInitializerExpression(syntax: initializerSyntaxOpt, + type: initializerTypeOpt ?? type, + typeSyntax: typeNode, + diagnostics: diagnostics); + } + return null; + } } - private BoundExpression BindInterfaceCreationExpression(ObjectCreationExpressionSyntax node, NamedTypeSymbol type, BoundObjectInitializerExpressionBase boundInitializerOpt, DiagnosticBag diagnostics) + private BoundExpression BindInterfaceCreationExpression(ObjectCreationExpressionSyntax node, NamedTypeSymbol type, DiagnosticBag diagnostics) { Debug.Assert((object)type != null); @@ -4889,7 +4902,7 @@ private BoundExpression BindInterfaceCreationExpression(ObjectCreationExpression NamedTypeSymbol coClassType = type.ComImportCoClass; if ((object)coClassType != null) { - return BindComImportCoClassCreationExpression(node, type, coClassType, boundInitializerOpt, diagnostics); + return BindComImportCoClassCreationExpression(node, type, coClassType, diagnostics); } } @@ -4909,7 +4922,7 @@ private BoundExpression BindBadInterfaceCreationExpression(ObjectCreationExpress return result; } - private BoundExpression BindComImportCoClassCreationExpression(ObjectCreationExpressionSyntax node, NamedTypeSymbol interfaceType, NamedTypeSymbol coClassType, BoundObjectInitializerExpressionBase boundInitializerOpt, DiagnosticBag diagnostics) + private BoundExpression BindComImportCoClassCreationExpression(ObjectCreationExpressionSyntax node, NamedTypeSymbol interfaceType, NamedTypeSymbol coClassType, DiagnosticBag diagnostics) { Debug.Assert((object)interfaceType != null); Debug.Assert(interfaceType.IsInterfaceType()); @@ -4944,10 +4957,10 @@ private BoundExpression BindComImportCoClassCreationExpression(ObjectCreationExp // NoPIA support if (interfaceType.ContainingAssembly.IsLinked) { - return BindNoPiaObjectCreationExpression(node, interfaceType, coClassType, boundInitializerOpt, diagnostics); + return BindNoPiaObjectCreationExpression(node, interfaceType, coClassType, diagnostics); } - var classCreation = BindClassCreationExpression(node, coClassType, coClassType.Name, boundInitializerOpt, diagnostics); + var classCreation = BindClassCreationExpression(node, coClassType, coClassType.Name, diagnostics, interfaceType); HashSet useSiteDiagnostics = null; Conversion conversion = this.Conversions.ClassifyConversionFromExpression(classCreation, interfaceType, ref useSiteDiagnostics, forCast: true); diagnostics.Add(node, useSiteDiagnostics); @@ -4985,7 +4998,6 @@ private BoundExpression BindComImportCoClassCreationExpression(ObjectCreationExp ObjectCreationExpressionSyntax node, NamedTypeSymbol interfaceType, NamedTypeSymbol coClassType, - BoundObjectInitializerExpressionBase boundInitializerOpt, DiagnosticBag diagnostics) { string guidString; @@ -4996,6 +5008,12 @@ private BoundExpression BindComImportCoClassCreationExpression(ObjectCreationExp guidString = System.Guid.Empty.ToString("D"); } + var boundInitializerOpt = node.Initializer == null ? null : + BindInitializerExpression(syntax: node.Initializer, + type: interfaceType, + typeSyntax: node.Type, + diagnostics: diagnostics); + var creation = new BoundNoPiaObjectCreationExpression(node, guidString, boundInitializerOpt, interfaceType); // Get the bound arguments and the argument names, it is an error if any are present. @@ -5020,7 +5038,7 @@ private BoundExpression BindComImportCoClassCreationExpression(ObjectCreationExp return creation; } - private BoundExpression BindTypeParameterCreationExpression(ObjectCreationExpressionSyntax node, TypeParameterSymbol typeParameter, BoundObjectInitializerExpressionBase boundInitializerOpt, DiagnosticBag diagnostics) + private BoundExpression BindTypeParameterCreationExpression(ObjectCreationExpressionSyntax node, TypeParameterSymbol typeParameter, DiagnosticBag diagnostics) { AnalyzedArguments analyzedArguments = AnalyzedArguments.GetInstance(); BindArgumentsAndNames(node.ArgumentList, diagnostics, analyzedArguments); @@ -5039,10 +5057,17 @@ private BoundExpression BindTypeParameterCreationExpression(ObjectCreationExpres } else { + var boundInitializerOpt = node.Initializer == null ? + null : + BindInitializerExpression( + syntax: node.Initializer, + type: typeParameter, + typeSyntax: node.Type, + diagnostics: diagnostics); return new BoundNewT(node, boundInitializerOpt, typeParameter); } - return MakeBadExpressionForObjectCreation(node, typeParameter, boundInitializerOpt, analyzedArguments); + return MakeBadExpressionForObjectCreation(node, typeParameter, analyzedArguments, diagnostics); } finally { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs index f9d5c265fa9..940c0b9554f 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OutVarTests.cs @@ -34493,6 +34493,53 @@ public class C "); Assert.Null(model.GetOperation(node3).Parent); } + + [Fact] + public void OutVarInConstructorUsedInObjectInitializer() + { + var source = +@" +public class C +{ + public int Number { get; set; } + + public C(out int n) + { + n = 1; + } + + public static void Main() + { + C c = new C(out var i) { Number = i }; + System.Console.WriteLine(c.Number); + } +} +"; + CompileAndVerify(source, expectedOutput: @"1"); + } + + [Fact] + public void OutVarInConstructorUsedInCollectionInitializer() + { + var source = +@" +public class C : System.Collections.Generic.List +{ + public C(out int n) + { + n = 1; + } + + public static void Main() + { + C c = new C(out var i) { i, i, i }; + System.Console.WriteLine(c[0]); + } +} +"; + CompileAndVerify(source, expectedOutput: @"1"); + } + } internal static class OutVarTestsExtensions -- GitLab