From dffb86b0f48dad64a5f128ae8890d3c27e050d44 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 8 Jul 2016 10:39:15 -0700 Subject: [PATCH 1/5] Addressing some PROTOTYPE markers --- .../CSharp/Portable/CSharpCodeAnalysis.csproj | 3 ++ ...writer_DeconstructionAssignmentOperator.cs | 1 - .../CSharp/Portable/Parser/LanguageParser.cs | 1 - .../Syntax/EventFieldDeclarationSyntax.cs | 11 ++++++ .../Portable/Syntax/FieldDeclarationSyntax.cs | 11 ++++++ .../Portable/Syntax/FixedStatementSyntax.cs | 11 ++++++ .../Syntax/LocalDeclarationStatementSyntax.cs | 7 ++-- .../CSharp/Portable/Syntax/SyntaxFactory.cs | 36 ------------------- .../Emit/CodeGen/CodeGenDeconstructTests.cs | 6 ++-- 9 files changed, 45 insertions(+), 42 deletions(-) create mode 100644 src/Compilers/CSharp/Portable/Syntax/EventFieldDeclarationSyntax.cs create mode 100644 src/Compilers/CSharp/Portable/Syntax/FieldDeclarationSyntax.cs create mode 100644 src/Compilers/CSharp/Portable/Syntax/FixedStatementSyntax.cs diff --git a/src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj b/src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj index b34067dcb816d..6376750aac608 100644 --- a/src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj +++ b/src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj @@ -761,7 +761,10 @@ + + + diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs index fb9421eb9589c..6036458ef80d7 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs @@ -73,7 +73,6 @@ private ImmutableArray LeftHandSideSideEffects(ImmutableArray /// Check ahead for a deconstruction declaration. This requires at least one good-looking variable and the presence of an equals sign. /// Doesn't move the cursor. - /// PROTOTYPE(tuples) Can this be done without allocations? /// private bool IsPossibleDeconstructionDeclaration() { diff --git a/src/Compilers/CSharp/Portable/Syntax/EventFieldDeclarationSyntax.cs b/src/Compilers/CSharp/Portable/Syntax/EventFieldDeclarationSyntax.cs new file mode 100644 index 0000000000000..1ee410b3d2ba1 --- /dev/null +++ b/src/Compilers/CSharp/Portable/Syntax/EventFieldDeclarationSyntax.cs @@ -0,0 +1,11 @@ + +namespace Microsoft.CodeAnalysis.CSharp.Syntax +{ + public sealed partial class EventFieldDeclarationSyntax : BaseFieldDeclarationSyntax + { + public EventFieldDeclarationSyntax AddDeclarationVariables(params VariableDeclaratorSyntax[] items) + { + return this.WithDeclaration(this.Declaration.WithVariables(this.Declaration.Variables.AddRange(items))); + } + } +} \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/Syntax/FieldDeclarationSyntax.cs b/src/Compilers/CSharp/Portable/Syntax/FieldDeclarationSyntax.cs new file mode 100644 index 0000000000000..9b8af69c415b8 --- /dev/null +++ b/src/Compilers/CSharp/Portable/Syntax/FieldDeclarationSyntax.cs @@ -0,0 +1,11 @@ + +namespace Microsoft.CodeAnalysis.CSharp.Syntax +{ + public sealed partial class FieldDeclarationSyntax : BaseFieldDeclarationSyntax + { + public FieldDeclarationSyntax AddDeclarationVariables(params VariableDeclaratorSyntax[] items) + { + return this.WithDeclaration(this.Declaration.WithVariables(this.Declaration.Variables.AddRange(items))); + } + } +} \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/Syntax/FixedStatementSyntax.cs b/src/Compilers/CSharp/Portable/Syntax/FixedStatementSyntax.cs new file mode 100644 index 0000000000000..d7c586ecaf304 --- /dev/null +++ b/src/Compilers/CSharp/Portable/Syntax/FixedStatementSyntax.cs @@ -0,0 +1,11 @@ + +namespace Microsoft.CodeAnalysis.CSharp.Syntax +{ + public sealed partial class FixedStatementSyntax : StatementSyntax + { + public FixedStatementSyntax AddDeclarationVariables(params VariableDeclaratorSyntax[] items) + { + return this.WithDeclaration(this.Declaration.WithVariables(this.Declaration.Variables.AddRange(items))); + } + } +} \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/Syntax/LocalDeclarationStatementSyntax.cs b/src/Compilers/CSharp/Portable/Syntax/LocalDeclarationStatementSyntax.cs index b95a1185d938f..6fcfa03a6014e 100644 --- a/src/Compilers/CSharp/Portable/Syntax/LocalDeclarationStatementSyntax.cs +++ b/src/Compilers/CSharp/Portable/Syntax/LocalDeclarationStatementSyntax.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.CSharp.Syntax { @@ -12,6 +10,11 @@ public LocalDeclarationStatementSyntax Update(SyntaxTokenList modifiers, Variabl { return Update(modifiers, this.RefKeyword, declaration, semicolonToken); } + + public LocalDeclarationStatementSyntax AddDeclarationVariables(params VariableDeclaratorSyntax[] items) + { + return this.WithDeclaration(this.Declaration.WithVariables(this.Declaration.Variables.AddRange(items))); + } } } diff --git a/src/Compilers/CSharp/Portable/Syntax/SyntaxFactory.cs b/src/Compilers/CSharp/Portable/Syntax/SyntaxFactory.cs index 19f958e1dab94..09967718e180d 100644 --- a/src/Compilers/CSharp/Portable/Syntax/SyntaxFactory.cs +++ b/src/Compilers/CSharp/Portable/Syntax/SyntaxFactory.cs @@ -2650,39 +2650,3 @@ internal partial class ContextAwareSyntax #endif } } - -// PROTOTYPE(tuples) Move this to a better place -namespace Microsoft.CodeAnalysis.CSharp.Syntax -{ - public sealed partial class EventFieldDeclarationSyntax : BaseFieldDeclarationSyntax - { - public EventFieldDeclarationSyntax AddDeclarationVariables(params VariableDeclaratorSyntax[] items) - { - return this.WithDeclaration(this.Declaration.WithVariables(this.Declaration.Variables.AddRange(items))); - } - } - - public sealed partial class FieldDeclarationSyntax : BaseFieldDeclarationSyntax - { - public FieldDeclarationSyntax AddDeclarationVariables(params VariableDeclaratorSyntax[] items) - { - return this.WithDeclaration(this.Declaration.WithVariables(this.Declaration.Variables.AddRange(items))); - } - } - - public sealed partial class FixedStatementSyntax : StatementSyntax - { - public FixedStatementSyntax AddDeclarationVariables(params VariableDeclaratorSyntax[] items) - { - return this.WithDeclaration(this.Declaration.WithVariables(this.Declaration.Variables.AddRange(items))); - } - } - - public sealed partial class LocalDeclarationStatementSyntax : StatementSyntax - { - public LocalDeclarationStatementSyntax AddDeclarationVariables(params VariableDeclaratorSyntax[] items) - { - return this.WithDeclaration(this.Declaration.WithVariables(this.Declaration.Variables.AddRange(items))); - } - } -} \ No newline at end of file diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index f2c3aaee9a80f..737a6668d1f3c 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -2484,7 +2484,8 @@ public void Deconstruct(out int a, out string b) comp.VerifyDiagnostics(); } - [Fact(Skip = "PROTOTYPE(tuples)")] + [Fact(Skip = "https://github.com/dotnet/roslyn/issues/12400")] + [WorkItem(12400, "https://github.com/dotnet/roslyn/issues/12400")] public void AssignWithPostfixOperator() { string source = @" @@ -2513,7 +2514,8 @@ public void Deconstruct(out int a, out string b) } } "; - // PROTOTYPE(tuples) we expect "2 hello" instead, which means the evaluation order is wrong + // https://github.com/dotnet/roslyn/issues/12400 + // we expect "2 hello" instead, which means the evaluation order is wrong var comp = CompileAndVerify(source, expectedOutput: "1 hello"); comp.VerifyDiagnostics(); } From 011d81daea3ab304c48ae701dfb5df06305f248e Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 8 Jul 2016 17:19:08 -0700 Subject: [PATCH 2/5] One more PROTOTYPE marker --- .../CSharp/Test/Syntax/Parsing/DeconstructionTest.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/DeconstructionTest.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/DeconstructionTest.cs index 2de7267266e0c..2c71f9f6cde22 100644 --- a/src/Compilers/CSharp/Test/Syntax/Parsing/DeconstructionTest.cs +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/DeconstructionTest.cs @@ -5,6 +5,7 @@ using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities; using Xunit; +using Roslyn.Test.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Parsing { @@ -1728,7 +1729,8 @@ public void InvalidStatement() Assert.True(statement.HasErrors); } - [Fact(Skip = "PROTOTYPE(tuples)")] + [Fact(Skip = "https://github.com/dotnet/roslyn/issues/12402")] + [WorkItem(12402, "https://github.com/dotnet/roslyn/issues/12402")] public void ConfusedForWithDeconstruction() { var text = "for ((int x, var (y, z)) in foo) { }"; From 21515cd4a714f817c8bcdb67d05f15de6fc638d5 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Fri, 8 Jul 2016 17:31:52 -0700 Subject: [PATCH 3/5] Addressing remaining PROTOTYPE markers --- .../Portable/Binder/Binder_Deconstruct.cs | 46 ++++--------------- .../Emit/CodeGen/CodeGenDeconstructTests.cs | 17 ++++--- 2 files changed, 19 insertions(+), 44 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs index a1e88aa02da52..8c1e86681f4a2 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs @@ -56,23 +56,14 @@ private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(CShar if ((object)boundRHS.Type == null) { - if (boundRHS.Kind == BoundKind.TupleLiteral) - { - // tuple literal without type such as `(null, null)`, let's fix it up by peeking at the LHS - TypeSymbol lhsAsTuple = MakeTupleTypeFromDeconstructionLHS(checkedVariables, diagnostics, Compilation); - boundRHS = GenerateConversionForAssignment(lhsAsTuple, boundRHS, diagnostics); - } - else - { - // expression without type such as `null` - Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, right); - FailRemainingInferences(checkedVariables, diagnostics); - - return new BoundDeconstructionAssignmentOperator( - node, FlattenDeconstructVariables(checkedVariables), boundRHS, - ImmutableArray.Empty, ImmutableArray.Empty, - voidType, hasErrors: true); - } + // expression without natural type such as `null` or `(null, 1)` + Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, right); + FailRemainingInferences(checkedVariables, diagnostics); + + return new BoundDeconstructionAssignmentOperator( + node, FlattenDeconstructVariables(checkedVariables), boundRHS, + ImmutableArray.Empty, ImmutableArray.Empty, + voidType, hasErrors: true); } var deconstructionSteps = ArrayBuilder.GetInstance(1); @@ -281,27 +272,6 @@ public DeconstructionVariable(ArrayBuilder variables, CS return !hasErrors; } - /// - /// For cases where the RHS of a deconstruction-assignment has no type (TupleLiteral), we squint and look at the LHS as a tuple type to give the RHS a type. - /// - private static TypeSymbol MakeTupleTypeFromDeconstructionLHS(ArrayBuilder topLevelCheckedVariables, DiagnosticBag diagnostics, CSharpCompilation compilation) - { - var typesBuilder = ArrayBuilder.GetInstance(topLevelCheckedVariables.Count); - foreach (var variable in topLevelCheckedVariables) - { - if (variable.HasNestedVariables) - { - typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(variable.NestedVariables, diagnostics, compilation)); - } - else - { - typesBuilder.Add(variable.Single.Type); - } - } - - return TupleTypeSymbol.Create(locationOpt: null, elementTypes: typesBuilder.ToImmutableAndFree(), elementLocations: default(ImmutableArray), elementNames: default(ImmutableArray), compilation: compilation, diagnostics: diagnostics); - } - /// /// Returns a list of variables, where some may be nested variables (BoundDeconstructionVariables). /// Checks that all the variables are assignable to. diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index 737a6668d1f3c..c89c23d483d00 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -2938,7 +2938,7 @@ static void Main() ); } - [Fact(Skip = "PROTOTYPE(tuples)")] + [Fact] public void TypelessDeclaration() { string source = @" @@ -2950,13 +2950,15 @@ static void Main() } } "; - // crash var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); comp.VerifyDiagnostics( + // (6,24): error CS8210: Deconstruct assignment requires an expression with a type on the right-hand-side. + // var (x1, x2) = (1, null); + Diagnostic(ErrorCode.ERR_DeconstructRequiresExpression, "(1, null)").WithLocation(6, 24) ); } - [Fact(Skip = "PROTOTYPE(tuples)")] + [Fact] public void InferTypeOfTypelessDeclaration() { string source = @" @@ -2969,9 +2971,12 @@ static void Main() } } "; - // crash - var comp = CompileAndVerify(source, expectedOutput: "1 2 "); - comp.VerifyDiagnostics(); + var comp = CreateCompilationWithMscorlib(source, references: new[] { ValueTupleRef, SystemRuntimeFacadeRef }); + comp.VerifyDiagnostics( + // (6,37): error CS8210: Deconstruct assignment requires an expression with a type on the right-hand-side. + // (var (x1, x2), string x3) = ((1, 2), null); + Diagnostic(ErrorCode.ERR_DeconstructRequiresExpression, "((1, 2), null)").WithLocation(6, 37) + ); } [Fact] From 9f16b63cae0eb1b98d3f3e04f957d6f83d20efbe Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Sat, 9 Jul 2016 11:26:46 -0700 Subject: [PATCH 4/5] Keep inferrence behavior for d-assignments --- .../Portable/Binder/Binder_Deconstruct.cs | 52 +++++++++++++++---- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs index 8c1e86681f4a2..d712bfc05c9bc 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs @@ -21,7 +21,7 @@ private BoundExpression BindDeconstructionAssignment(AssignmentExpressionSyntax var left = (TupleExpressionSyntax)node.Left; ArrayBuilder checkedVariables = BindDeconstructionAssignmentVariables(left.Arguments, left, diagnostics); - var result = BindDeconstructionAssignment(node, node.Right, checkedVariables, diagnostics); + var result = BindDeconstructionAssignment(node, node.Right, checkedVariables, diagnostics, isDeclaration: false); FreeDeconstructionVariables(checkedVariables); return result; @@ -47,7 +47,7 @@ private static void FreeDeconstructionVariables(ArrayBuilder - private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(CSharpSyntaxNode node, ExpressionSyntax right, ArrayBuilder checkedVariables, DiagnosticBag diagnostics, BoundDeconstructValuePlaceholder rhsPlaceholder = null) + private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(CSharpSyntaxNode node, ExpressionSyntax right, ArrayBuilder checkedVariables, DiagnosticBag diagnostics, bool isDeclaration, BoundDeconstructValuePlaceholder rhsPlaceholder = null) { TypeSymbol voidType = GetSpecialType(SpecialType.System_Void, diagnostics, node); @@ -56,14 +56,23 @@ private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(CShar if ((object)boundRHS.Type == null) { - // expression without natural type such as `null` or `(null, 1)` - Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, right); - FailRemainingInferences(checkedVariables, diagnostics); - - return new BoundDeconstructionAssignmentOperator( - node, FlattenDeconstructVariables(checkedVariables), boundRHS, - ImmutableArray.Empty, ImmutableArray.Empty, - voidType, hasErrors: true); + if (boundRHS.Kind == BoundKind.TupleLiteral && !isDeclaration) + { + // tuple literal without type such as `(null, null)`, let's fix it up by peeking at the LHS + TypeSymbol lhsAsTuple = MakeTupleTypeFromDeconstructionLHS(checkedVariables, diagnostics, Compilation); + boundRHS = GenerateConversionForAssignment(lhsAsTuple, boundRHS, diagnostics); + } + else + { + // expression without type such as `null` + Error(diagnostics, ErrorCode.ERR_DeconstructRequiresExpression, right); + FailRemainingInferences(checkedVariables, diagnostics); + + return new BoundDeconstructionAssignmentOperator( + node, FlattenDeconstructVariables(checkedVariables), boundRHS, + ImmutableArray.Empty, ImmutableArray.Empty, + voidType, hasErrors: true); + } } var deconstructionSteps = ArrayBuilder.GetInstance(1); @@ -272,6 +281,27 @@ public DeconstructionVariable(ArrayBuilder variables, CS return !hasErrors; } + /// + /// For cases where the RHS of a deconstruction-assignment has no type (TupleLiteral), we squint and look at the LHS as a tuple type to give the RHS a type. + /// + private static TypeSymbol MakeTupleTypeFromDeconstructionLHS(ArrayBuilder topLevelCheckedVariables, DiagnosticBag diagnostics, CSharpCompilation compilation) + { + var typesBuilder = ArrayBuilder.GetInstance(topLevelCheckedVariables.Count); + foreach (var variable in topLevelCheckedVariables) + { + if (variable.HasNestedVariables) + { + typesBuilder.Add(MakeTupleTypeFromDeconstructionLHS(variable.NestedVariables, diagnostics, compilation)); + } + else + { + typesBuilder.Add(variable.Single.Type); + } + } + + return TupleTypeSymbol.Create(locationOpt: null, elementTypes: typesBuilder.ToImmutableAndFree(), elementLocations: default(ImmutableArray), elementNames: default(ImmutableArray), compilation: compilation, diagnostics: diagnostics); + } + /// /// Returns a list of variables, where some may be nested variables (BoundDeconstructionVariables). /// Checks that all the variables are assignable to. @@ -466,7 +496,7 @@ internal BoundDeconstructionAssignmentOperator BindDeconstructionDeclaration(CSh { ArrayBuilder locals = BindDeconstructionDeclarationLocals(declaration, declaration.Type, diagnostics); - var result = BindDeconstructionAssignment(node, right, locals, diagnostics, rightPlaceholder); + var result = BindDeconstructionAssignment(node, right, locals, diagnostics, isDeclaration: true, rhsPlaceholder: rightPlaceholder); FreeDeconstructionVariables(locals); return result; From 183322ee1f5b5c786fe383bd43170388aefd33ba Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Sun, 10 Jul 2016 01:50:18 -0700 Subject: [PATCH 5/5] Add test for error right --- .../Emit/CodeGen/CodeGenDeconstructTests.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index c89c23d483d00..572c5985aec57 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -1284,6 +1284,28 @@ static void Main() ); } + [Fact] + public void ErrorRight() + { + string source = @" +class C +{ + static void Main() + { + int x; + (x, x) = undeclared; + } +} +"; + + var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.Regular.WithRefsFeature()); + comp.VerifyDiagnostics( + // (7,18): error CS0103: The name 'undeclared' does not exist in the current context + // (x, x) = undeclared; + Diagnostic(ErrorCode.ERR_NameNotInContext, "undeclared").WithArguments("undeclared").WithLocation(7, 18) + ); + } + [Fact] public void VoidRight() {