From c52f6e468d384e38e2d1a23a77e426d674315462 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 13 Nov 2020 17:26:30 -0800 Subject: [PATCH 01/14] Port: Ensure that attribute arguments are bound to their natural type if they do not have one. (cherry picked from commit 4af2613853062fe8abaf5b5e1b5aa1723a6c1d7a) --- .../Portable/Binder/Binder_Attributes.cs | 13 ++- .../CSharp/Test/Emit/CodeGen/PatternTests.cs | 84 +++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs index 05ed9d38f94c6..ef74411978e44 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs @@ -180,11 +180,20 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a } var constructorArguments = analyzedArguments.ConstructorArguments; - ImmutableArray boundConstructorArguments = constructorArguments.Arguments.ToImmutableAndFree(); + + // If the arguments didn't have a natural type, they're not constants, and so would already have diagnostics from the rest of binding. + var ignoredDiagnostics = new DiagnosticBag(); + ImmutableArray boundConstructorArguments = constructorArguments.Arguments.SelectAsArray( + (arg, thisAndDiagnostics) => thisAndDiagnostics.@this.BindToNaturalType(arg, thisAndDiagnostics.ignoredDiagnostics), + (@this: this, ignoredDiagnostics)); ImmutableArray boundConstructorArgumentNamesOpt = constructorArguments.GetNames(); - ImmutableArray boundNamedArguments = analyzedArguments.NamedArguments; + ImmutableArray boundNamedArguments = analyzedArguments.NamedArguments.SelectAsArray( + (namedArg, thisAndDiagnostics) => thisAndDiagnostics.@this.BindToNaturalType(namedArg, thisAndDiagnostics.ignoredDiagnostics), + (@this: this, ignoredDiagnostics)); constructorArguments.Free(); + Debug.Assert(ignoredDiagnostics.Count == 0 || diagnostics.HasAnyResolvedErrors()); + return new BoundAttribute(node, attributeConstructor, boundConstructorArguments, boundConstructorArgumentNamesOpt, argsToParamsOpt, expanded, boundNamedArguments, resultKind, attributeType, hasErrors: resultKind != LookupResultKind.Viable); } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs index 8b1f8f242b7a5..ea2c494b56e9a 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs @@ -4,6 +4,8 @@ #nullable disable +using System.Linq; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities; using Roslyn.Test.Utilities; @@ -4871,6 +4873,88 @@ public class B // [My(1 switch { 1 => 1, _ => string.Empty })] Diagnostic(ErrorCode.ERR_BadArgType, "1 switch { 1 => 1, _ => string.Empty }").WithArguments("1", "", "int").WithLocation(11, 9) ); + + var tree = compilation.SyntaxTrees[0]; + var model = compilation.GetSemanticModel(tree); + + var switchExpressions = tree.GetRoot().DescendantNodes().OfType().ToArray(); + + VerifyOperationTreeForNode(compilation, model, switchExpressions[0], @" +ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: System.Int32, IsInvalid) (Syntax: '1 switch { ... 1, _ => 2 }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '1 => 1') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsInvalid) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '_ => 2') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null, IsInvalid) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2, IsInvalid) (Syntax: '2') +"); + + VerifyOperationTreeForNode(compilation, model, switchExpressions[1], @" +ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: System.Int32, IsInvalid) (Syntax: '1 switch { ... > new B() }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '1 => new A()') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsInvalid) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Value: + IConversionOperation (TryCast: False, Unchecked) (OperatorMethod: System.Int32 A.op_Implicit(A a)) (OperationKind.Conversion, Type: System.Int32, IsInvalid, IsImplicit) (Syntax: 'new A()') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: True) (MethodSymbol: System.Int32 A.op_Implicit(A a)) + Operand: + IObjectCreationOperation (Constructor: A..ctor()) (OperationKind.ObjectCreation, Type: A, IsInvalid) (Syntax: 'new A()') + Arguments(0) + Initializer: + null + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '_ => new B()') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null, IsInvalid) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + IConversionOperation (TryCast: False, Unchecked) (OperatorMethod: System.Int32 B.op_Implicit(B b)) (OperationKind.Conversion, Type: System.Int32, IsInvalid, IsImplicit) (Syntax: 'new B()') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: True) (MethodSymbol: System.Int32 B.op_Implicit(B b)) + Operand: + IObjectCreationOperation (Constructor: B..ctor()) (OperationKind.ObjectCreation, Type: B, IsInvalid) (Syntax: 'new B()') + Arguments(0) + Initializer: + null +"); + + VerifyOperationTreeForNode(compilation, model, switchExpressions[2], @" +ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: ?, IsInvalid) (Syntax: '1 switch { ... ing.Empty }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '1 => 1') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsInvalid) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Value: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsInvalid, IsImplicit) (Syntax: '1') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '_ => string.Empty') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null, IsInvalid) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsInvalid, IsImplicit) (Syntax: 'string.Empty') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + IFieldReferenceOperation: System.String System.String.Empty (Static) (OperationKind.FieldReference, Type: System.String, IsInvalid) (Syntax: 'string.Empty') + Instance Receiver: + null +"); } [Fact] From 1674c583680b60729ead770fd6db18467b633405 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 4 Feb 2021 17:22:40 -0800 Subject: [PATCH 02/14] Fixup: Simplify logic, ensure that builders aren't leaked and we aren't allocating arrays we don't need to. --- .../Portable/Binder/Binder_Attributes.cs | 40 +++++++++---------- .../Collections/ArrayBuilderExtensions.cs | 18 +++++++++ 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs index ef74411978e44..16faff3c1b34e 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs @@ -169,6 +169,8 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a } diagnostics.Add(node, useSiteInfo); + var constructorArguments = analyzedArguments.ConstructorArguments; + ImmutableArray boundConstructorArguments; if (attributeConstructor is object) { ReportDiagnosticsIfObsolete(diagnostics, attributeConstructor, node, hasBaseReceiver: false); @@ -177,22 +179,22 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a { Error(diagnostics, ErrorCode.ERR_AttributeCtorInParameter, node, attributeConstructor.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat)); } - } - var constructorArguments = analyzedArguments.ConstructorArguments; + boundConstructorArguments = BuildArgumentsForErrorRecovery(constructorArguments, ImmutableArray.Create(attributeConstructor)); + constructorArguments.Arguments.Free(); + } + else + { + boundConstructorArguments = constructorArguments.Arguments.SelectAsArrayInPlaceAndFree( + (arg, @this) => @this.BindToTypeForErrorRecovery(arg), this); + } - // If the arguments didn't have a natural type, they're not constants, and so would already have diagnostics from the rest of binding. - var ignoredDiagnostics = new DiagnosticBag(); - ImmutableArray boundConstructorArguments = constructorArguments.Arguments.SelectAsArray( - (arg, thisAndDiagnostics) => thisAndDiagnostics.@this.BindToNaturalType(arg, thisAndDiagnostics.ignoredDiagnostics), - (@this: this, ignoredDiagnostics)); ImmutableArray boundConstructorArgumentNamesOpt = constructorArguments.GetNames(); - ImmutableArray boundNamedArguments = analyzedArguments.NamedArguments.SelectAsArray( - (namedArg, thisAndDiagnostics) => thisAndDiagnostics.@this.BindToNaturalType(namedArg, thisAndDiagnostics.ignoredDiagnostics), - (@this: this, ignoredDiagnostics)); - constructorArguments.Free(); + ImmutableArray boundNamedArguments = + analyzedArguments.NamedArguments?.SelectAsArrayInPlaceAndFree((namedArg, @this) => @this.BindToTypeForErrorRecovery(namedArg), this) + ?? ImmutableArray.Empty; - Debug.Assert(ignoredDiagnostics.Count == 0 || diagnostics.HasAnyResolvedErrors()); + constructorArguments.Free(); return new BoundAttribute(node, attributeConstructor, boundConstructorArguments, boundConstructorArgumentNamesOpt, argsToParamsOpt, expanded, boundNamedArguments, resultKind, attributeType, hasErrors: resultKind != LookupResultKind.Viable); @@ -310,11 +312,10 @@ private AnalyzedAttributeArguments BindAttributeArguments( BindingDiagnosticBag diagnostics) { var boundConstructorArguments = AnalyzedArguments.GetInstance(); - var boundNamedArguments = ImmutableArray.Empty; + ArrayBuilder? boundNamedArgumentsBuilder = null; if (attributeArgumentList != null) { - ArrayBuilder? boundNamedArgumentsBuilder = null; HashSet? boundNamedArgumentsSet = null; // Only report the first "non-trailing named args required C# 7.2" error, @@ -365,14 +366,9 @@ private AnalyzedAttributeArguments BindAttributeArguments( boundNamedArgumentsSet.Add(argumentName); } } - - if (boundNamedArgumentsBuilder != null) - { - boundNamedArguments = boundNamedArgumentsBuilder.ToImmutableAndFree(); - } } - return new AnalyzedAttributeArguments(boundConstructorArguments, boundNamedArguments); + return new AnalyzedAttributeArguments(boundConstructorArguments, boundNamedArgumentsBuilder); } private BoundExpression BindNamedAttributeArgument(AttributeArgumentSyntax namedArgument, NamedTypeSymbol attributeType, BindingDiagnosticBag diagnostics) @@ -1257,9 +1253,9 @@ private static TypedConstant CreateTypedConstant(BoundExpression node, TypedCons private struct AnalyzedAttributeArguments { internal readonly AnalyzedArguments ConstructorArguments; - internal readonly ImmutableArray NamedArguments; + internal readonly ArrayBuilder? NamedArguments; - internal AnalyzedAttributeArguments(AnalyzedArguments constructorArguments, ImmutableArray namedArguments) + internal AnalyzedAttributeArguments(AnalyzedArguments constructorArguments, ArrayBuilder? namedArguments) { this.ConstructorArguments = constructorArguments; this.NamedArguments = namedArguments; diff --git a/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs b/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs index 029c738ac7ddd..1caae98fc9de4 100644 --- a/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs +++ b/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs @@ -137,6 +137,24 @@ public static ImmutableArray SelectAsArray(this A } } + /// + /// Maps an array builder to immutable array, applying the select function to the array builder in place, and freeing the underlying + /// array builder. + /// + /// The sequence to map + /// The mapping delegate + /// The extra input used by mapping delegate + /// If the items's length is 0, this will return an empty immutable array. + public static ImmutableArray SelectAsArrayInPlaceAndFree(this ArrayBuilder items, Func map, TArg arg) + { + for (int i = 0; i < items.Count; i++) + { + items[i] = map(items[i], arg); + } + + return items.ToImmutableAndFree(); + } + public static void AddOptional(this ArrayBuilder builder, T? item) where T : class { From 30a20db9cca7a243b2ff7ce57489ee2b748cfbd2 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 13 Nov 2020 17:35:21 -0800 Subject: [PATCH 03/14] Port: Skip tests not compatible with IOperation (cherry picked from commit d4ab21ff7c23511a259d32564cc0307065994022) --- .../Symbol/Compilation/CompilationAPITests.cs | 2 +- .../Compilation/ReferenceManagerTests.cs | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Compilation/CompilationAPITests.cs b/src/Compilers/CSharp/Test/Symbol/Compilation/CompilationAPITests.cs index 8f462e286ab52..188a65c7fe99f 100644 --- a/src/Compilers/CSharp/Test/Symbol/Compilation/CompilationAPITests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Compilation/CompilationAPITests.cs @@ -2192,7 +2192,7 @@ protected override PortableExecutableReference WithPropertiesImpl(MetadataRefere } } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoUsedAssembliesValidation), typeof(NoIOperationValidation), Reason = "IOperation skip: Compilation changes over time, adds new errors")] public void MetadataConsistencyWhileEvolvingCompilation() { var md1 = AssemblyMetadata.CreateFromImage(CreateCompilation("public class C { }").EmitToArray()); diff --git a/src/Compilers/CSharp/Test/Symbol/Compilation/ReferenceManagerTests.cs b/src/Compilers/CSharp/Test/Symbol/Compilation/ReferenceManagerTests.cs index ff6b62174c9b0..18d41de5dea5c 100644 --- a/src/Compilers/CSharp/Test/Symbol/Compilation/ReferenceManagerTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Compilation/ReferenceManagerTests.cs @@ -2525,7 +2525,7 @@ public class P "mscorlib: global,Y,Y,Z"); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution1() { // c - a -> b @@ -2628,7 +2628,7 @@ public class C : A "B, Version=3.0.0.0: Y,X"); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_WeakIdentities1() { // c - a -> "b,v1,PKT=null" @@ -2664,7 +2664,7 @@ public void MissingAssemblyResolution_WeakIdentities1() "B, Version=1.0.0.0: "); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_WeakIdentities2() { // c - a -> "b,v1,PKT=null" @@ -2717,7 +2717,7 @@ public void MissingAssemblyResolution_None() resolver.VerifyResolutionAttempts(); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_ActualMissing() { // c - a -> d @@ -2740,7 +2740,7 @@ public void MissingAssemblyResolution_ActualMissing() /// /// Ignore assemblies returned by the resolver that don't match the reference identity. /// - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_MissingDueToResolutionMismatch() { // c - a -> b @@ -2765,7 +2765,7 @@ public void MissingAssemblyResolution_MissingDueToResolutionMismatch() "A -> B, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null"); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_Multiple() { // c - a -> d @@ -2790,7 +2790,7 @@ public void MissingAssemblyResolution_Multiple() "B -> D, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null"); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_Modules() { // c - a - d @@ -2994,7 +2994,7 @@ public void MissingAssemblyResolution_BindingToExplicitReference_BetterVersion() "E, Version=1.0.0.0"); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_BindingToImplicitReference1() { // c - a -> d -> "b,v2" @@ -3038,7 +3038,7 @@ public void MissingAssemblyResolution_BindingToImplicitReference1() "B, Version=1.0.0.0: "); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_BindingToImplicitReference2() { // c - a -> d -> "b,v2" @@ -3104,7 +3104,7 @@ public void MissingAssemblyResolution_BindingToImplicitReference2() "A -> B, Version=1.0.0.0, Culture=neutral, PublicKeyToken=ce65828c82a341f2"); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_BindingToImplicitReference3() { // c - a -> d -> "b,v2" @@ -3170,7 +3170,7 @@ public void MissingAssemblyResolution_BindingToImplicitReference3() "A -> B, Version=1.0.0.0, Culture=neutral, PublicKeyToken=ce65828c82a341f2"); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_Supersession_FxUnification() { var options = TestOptions.ReleaseDll.WithAssemblyIdentityComparer(DesktopAssemblyIdentityComparer.Default); @@ -3213,7 +3213,7 @@ public void MissingAssemblyResolution_Supersession_FxUnification() "System, Version=2.0.0.0: "); } - [ConditionalFact(typeof(NoUsedAssembliesValidation))] + [ConditionalFact(typeof(NoIOperationValidation), typeof(NoUsedAssembliesValidation), Reason = "IOperation adds extra assemblies")] public void MissingAssemblyResolution_Supersession_StrongNames() { var options = TestOptions.ReleaseDll.WithAssemblyIdentityComparer(DesktopAssemblyIdentityComparer.Default); From b729808fc2d836b10b3bd0ef1050d43e209653df Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 17 Nov 2020 12:15:22 -0800 Subject: [PATCH 04/14] Port: Add test and verification condition for vb indexed properties. (cherry picked from commit ff250a3b027902338cd40b09ccefe36b4397f21d) --- ...perationTests_IObjectCreationExpression.cs | 133 ++++++++++++++++++ .../Compilation/ControlFlowGraphVerifier.cs | 10 ++ 2 files changed, 143 insertions(+) diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IObjectCreationExpression.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IObjectCreationExpression.cs index d778c6e6dcb36..de45c423bb3af 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IObjectCreationExpression.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IObjectCreationExpression.cs @@ -15183,5 +15183,138 @@ static void F(A a) Predecessors: [B3] Statements (0)"); } + + [Fact] + public void IndexedPropertyWithDefaultArgumentInVB() + { + var source1 = +@"Imports System +Imports System.Collections.Generic +Imports System.Runtime.InteropServices + + + + + +Public Interface IA + Property P1(Optional index As Integer = 1) As Object + ReadOnly Property P2(Optional index As Integer = 2) As IA +End Interface +Public Class A + Implements IA + Property P1(Optional index As Integer = 1) As Object Implements IA.P1 + Get + Console.WriteLine(""P1({0}).get"", index) + Return Nothing + End Get + Set(value As Object) + Console.WriteLine(""P1({0}).set"", index) + End Set + End Property + ReadOnly Property P2(Optional index As Integer = 2) As IA Implements IA.P2 + Get + Console.WriteLine(""P2({0}).get"", index) + Return New A() + End Get + End Property +End Class"; + var reference1 = BasicCompilationUtils.CompileToMetadata(source1, verify: Verification.Passes); + var source2 = +@"class B +{ + static void M(IA a) + /**/{ + a = new IA() { P2 = { P1 = 5 } }; + }/**/ +}"; + + VerifyFlowGraphAndDiagnosticsForTest(source2, expectedDiagnostics: DiagnosticDescription.None, references: new[] { reference1 }, + expectedFlowGraph: @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] + Entering: {R1} +.locals {R1} +{ + CaptureIds: [0] [1] + Block[B1] - Block + Predecessors: [B0] + Statements (2) + IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: 'a') + Value: + IParameterReferenceOperation: a (OperationKind.ParameterReference, Type: IA) (Syntax: 'a') + IFlowCaptureOperation: 1 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: 'new IA() { ... P1 = 5 } }') + Value: + IObjectCreationOperation (Constructor: A..ctor()) (OperationKind.ObjectCreation, Type: IA) (Syntax: 'new IA() { ... P1 = 5 } }') + Arguments(0) + Initializer: + null + Next (Regular) Block[B2] + Entering: {R2} + .locals {R2} + { + CaptureIds: [2] + Block[B2] - Block + Predecessors: [B1] + Statements (1) + IFlowCaptureOperation: 2 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: 'P2') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2, IsImplicit) (Syntax: 'P2') + Next (Regular) Block[B3] + Entering: {R3} + .locals {R3} + { + CaptureIds: [3] + Block[B3] - Block + Predecessors: [B2] + Statements (2) + IFlowCaptureOperation: 3 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: 'P1') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsImplicit) (Syntax: 'P1') + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Object) (Syntax: 'P1 = 5') + Left: + IPropertyReferenceOperation: System.Object IA.P1[[System.Int32 index = 1]] { get; set; } (OperationKind.PropertyReference, Type: System.Object) (Syntax: 'P1') + Instance Receiver: + IPropertyReferenceOperation: IA IA.P2[[System.Int32 index = 2]] { get; } (OperationKind.PropertyReference, Type: IA) (Syntax: 'P2') + Instance Receiver: + IFlowCaptureReferenceOperation: 1 (OperationKind.FlowCaptureReference, Type: IA, IsImplicit) (Syntax: 'new IA() { ... P1 = 5 } }') + Arguments(1): + IArgumentOperation (ArgumentKind.DefaultValue, Matching Parameter: index) (OperationKind.Argument, Type: null, IsImplicit) (Syntax: 'P2') + IFlowCaptureReferenceOperation: 2 (OperationKind.FlowCaptureReference, Type: System.Int32, Constant: 2, IsImplicit) (Syntax: 'P2') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Arguments(1): + IArgumentOperation (ArgumentKind.DefaultValue, Matching Parameter: index) (OperationKind.Argument, Type: null, IsImplicit) (Syntax: 'P1') + IFlowCaptureReferenceOperation: 3 (OperationKind.FlowCaptureReference, Type: System.Int32, Constant: 1, IsImplicit) (Syntax: 'P1') + InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Right: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.Object, IsImplicit) (Syntax: '5') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + (Boxing) + Operand: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 5) (Syntax: '5') + Next (Regular) Block[B4] + Leaving: {R3} {R2} + } + } + Block[B4] - Block + Predecessors: [B3] + Statements (1) + IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null) (Syntax: 'a = new IA( ... P1 = 5 } };') + Expression: + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: IA) (Syntax: 'a = new IA( ... P1 = 5 } }') + Left: + IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: IA, IsImplicit) (Syntax: 'a') + Right: + IFlowCaptureReferenceOperation: 1 (OperationKind.FlowCaptureReference, Type: IA, IsImplicit) (Syntax: 'new IA() { ... P1 = 5 } }') + Next (Regular) Block[B5] + Leaving: {R1} +} +Block[B5] - Exit + Predecessors: [B4] + Statements (0) +"); + } } } diff --git a/src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs b/src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs index 813df800ed910..47156509599cc 100644 --- a/src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs +++ b/src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs @@ -1110,6 +1110,16 @@ bool isLongLivedCaptureReferenceSyntax(SyntaxNode captureReferenceSyntax) return true; } break; + + case CSharp.SyntaxKind.SimpleAssignmentExpression: + if (((CSharp.Syntax.AssignmentExpressionSyntax)syntax.Parent).Left == syntax + && syntax.Parent.Parent is InitializerExpressionSyntax { Parent: CSharp.Syntax.ObjectCreationExpressionSyntax }) + { + // Can occur when a property is actually an indexed property with a default argument accessed through + // COM interop, see IndexedPropertyWithDefaultArgumentInVB. + return true; + } + break; } } From b74afde4ec7dcf8b1cb45c43702f4bd5c38e74ae Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 4 Feb 2021 17:53:45 -0800 Subject: [PATCH 05/14] Fixup: Tighten assertion on nested member initializers. --- ...perationTests_IObjectCreationExpression.cs | 3 -- .../Compilation/ControlFlowGraphVerifier.cs | 29 ++++++++++++------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IObjectCreationExpression.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IObjectCreationExpression.cs index de45c423bb3af..fc94252b95daf 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IObjectCreationExpression.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IObjectCreationExpression.cs @@ -15204,16 +15204,13 @@ Public Class A Implements IA Property P1(Optional index As Integer = 1) As Object Implements IA.P1 Get - Console.WriteLine(""P1({0}).get"", index) Return Nothing End Get Set(value As Object) - Console.WriteLine(""P1({0}).set"", index) End Set End Property ReadOnly Property P2(Optional index As Integer = 2) As IA Implements IA.P2 Get - Console.WriteLine(""P2({0}).get"", index) Return New A() End Get End Property diff --git a/src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs b/src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs index 47156509599cc..71d1f4be45a2d 100644 --- a/src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs +++ b/src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs @@ -823,7 +823,8 @@ void assertCaptureReferences( ((isFirstOperandOfDynamicOrUserDefinedLogicalOperator(reference) || isIncrementedNullableForToLoopControlVariable(reference) || isConditionalAccessReceiver(reference) || - isCoalesceAssignmentTarget(reference)) && + isCoalesceAssignmentTarget(reference) || + isObjectInitializerInitializedObjectTarget(reference)) && block.EnclosingRegion.EnclosingRegion.CaptureIds.Contains(id)), $"Operation [{operationIndex}] in [{getBlockId(block)}] uses capture [{id.Value}] from another region. Should the regions be merged?", finalGraph); } @@ -875,6 +876,22 @@ bool isCoalesceAssignmentTarget(IFlowCaptureReferenceOperation reference) conditionalAccess.Left == referenceSyntax; } + bool isObjectInitializerInitializedObjectTarget(IFlowCaptureReferenceOperation reference) + { + if (reference.Language != LanguageNames.CSharp) + { + return false; + } + + CSharpSyntaxNode referenceSyntax = applyParenthesizedOrNullSuppressionIfAnyCS((CSharpSyntaxNode)reference.Syntax); + return referenceSyntax.Parent is CSharp.Syntax.AssignmentExpressionSyntax + { + RawKind: (int)CSharp.SyntaxKind.SimpleAssignmentExpression, + Parent: InitializerExpressionSyntax { Parent: CSharp.Syntax.ObjectCreationExpressionSyntax }, + Left: var left + } && left == referenceSyntax; + } + bool isFirstOperandOfDynamicOrUserDefinedLogicalOperator(IFlowCaptureReferenceOperation reference) { if (reference.Parent is IBinaryOperation binOp) @@ -1110,16 +1127,6 @@ bool isLongLivedCaptureReferenceSyntax(SyntaxNode captureReferenceSyntax) return true; } break; - - case CSharp.SyntaxKind.SimpleAssignmentExpression: - if (((CSharp.Syntax.AssignmentExpressionSyntax)syntax.Parent).Left == syntax - && syntax.Parent.Parent is InitializerExpressionSyntax { Parent: CSharp.Syntax.ObjectCreationExpressionSyntax }) - { - // Can occur when a property is actually an indexed property with a default argument accessed through - // COM interop, see IndexedPropertyWithDefaultArgumentInVB. - return true; - } - break; } } From ab6dcf461ef6dc06e62b94b92f1e0a201b8c923e Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 4 Feb 2021 18:26:17 -0800 Subject: [PATCH 06/14] Add test for error case involving using declaration in a switch case. --- .../IOperationTests_IUsingStatement.cs | 481 ++++++++++++++++++ .../Portable/Operations/OperationNodes.cs | 5 +- 2 files changed, 484 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IUsingStatement.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IUsingStatement.cs index 9875cf6aa07a0..77d4298dd2c41 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IUsingStatement.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IUsingStatement.cs @@ -8487,5 +8487,486 @@ void M() Statements (0) "); } + + [Fact] + public void UsingDeclaration_InsideSwitchCaseEmbeddedStatements() + { + var source = @" +using System; +class C1 : IDisposable +{ + public void Dispose() { } +} +class C2 +{ + public static void M(int x) + /**/{ + switch (x) + { + case 5: + using C1 o1 = new C1(); + break; + } + }/**/ +}"; + + var expectedDiagnostics = new DiagnosticDescription[] { + // file.cs(14,17): error CS8647: A using variable cannot be used directly within a switch section (consider using braces). + // using C1 o1 = new C1(); + Diagnostic(ErrorCode.ERR_UsingVarInSwitchCase, "using C1 o1 = new C1();").WithLocation(14, 17) + }; + + var expectedFlowGraph = @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] + Entering: {R1} +.locals {R1} +{ + CaptureIds: [0] + Block[B1] - Block + Predecessors: [B0] + Statements (1) + IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: 'x') + Value: + IParameterReferenceOperation: x (OperationKind.ParameterReference, Type: System.Int32) (Syntax: 'x') + Next (Regular) Block[B2] + Entering: {R2} + .locals {R2} + { + Locals: [C1 o1] + Block[B2] - Block + Predecessors: [B1] + Statements (0) + Jump if False (Regular) to Block[B8] + IBinaryOperation (BinaryOperatorKind.Equals) (OperationKind.Binary, Type: System.Boolean, IsImplicit) (Syntax: '5') + Left: + IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: System.Int32, IsImplicit) (Syntax: 'x') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 5) (Syntax: '5') + Leaving: {R2} {R1} + Next (Regular) Block[B3] + Block[B3] - Block + Predecessors: [B2] + Statements (1) + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Left: + ILocalReferenceOperation: o1 (IsDeclaration: True) (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Right: + IObjectCreationOperation (Constructor: C1..ctor()) (OperationKind.ObjectCreation, Type: C1, IsInvalid) (Syntax: 'new C1()') + Arguments(0) + Initializer: + null + Next (Regular) Block[B4] + Entering: {R3} {R4} + .try {R3, R4} + { + Block[B4] - Block + Predecessors: [B3] + Statements (0) + Next (Regular) Block[B8] + Finalizing: {R5} + Leaving: {R4} {R3} {R2} {R1} + } + .finally {R5} + { + Block[B5] - Block + Predecessors (0) + Statements (0) + Jump if True (Regular) to Block[B7] + IIsNullOperation (OperationKind.IsNull, Type: System.Boolean, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Operand: + ILocalReferenceOperation: o1 (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Next (Regular) Block[B6] + Block[B6] - Block + Predecessors: [B5] + Statements (1) + IInvocationOperation (virtual void System.IDisposable.Dispose()) (OperationKind.Invocation, Type: System.Void, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Instance Receiver: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.IDisposable, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: True, IsUserDefined: False) (MethodSymbol: null) + (ImplicitReference) + Operand: + ILocalReferenceOperation: o1 (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Arguments(0) + Next (Regular) Block[B7] + Block[B7] - Block + Predecessors: [B5] [B6] + Statements (0) + Next (StructuredExceptionHandling) Block[null] + } + } +} +Block[B8] - Exit + Predecessors: [B2] [B4] + Statements (0) +"; + + VerifyFlowGraphAndDiagnosticsForTest(source, expectedFlowGraph, expectedDiagnostics); + } + + [Fact] + public void UsingDeclaration_InsideIfEmbeddedStatement() + { + var source = @" +using System; +class C1 : IDisposable +{ + public void Dispose() { } +} +class C2 +{ + public static void M(bool b) + /**/{ + if (b) + using C1 o1 = new C1(); + }/**/ +}"; + + var expectedDiagnostics = new DiagnosticDescription[] { + // file.cs(12,13): error CS1023: Embedded statement cannot be a declaration or labeled statement + // using C1 o1 = new C1(); + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "using C1 o1 = new C1();").WithLocation(12, 13) + }; + + var expectedFlowGraph = @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] +Block[B1] - Block + Predecessors: [B0] + Statements (0) + Jump if False (Regular) to Block[B7] + IParameterReferenceOperation: b (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'b') + Next (Regular) Block[B2] + Entering: {R1} +.locals {R1} +{ + Locals: [C1 o1] + Block[B2] - Block + Predecessors: [B1] + Statements (1) + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Left: + ILocalReferenceOperation: o1 (IsDeclaration: True) (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Right: + IObjectCreationOperation (Constructor: C1..ctor()) (OperationKind.ObjectCreation, Type: C1, IsInvalid) (Syntax: 'new C1()') + Arguments(0) + Initializer: + null + Next (Regular) Block[B3] + Entering: {R2} {R3} + .try {R2, R3} + { + Block[B3] - Block + Predecessors: [B2] + Statements (0) + Next (Regular) Block[B7] + Finalizing: {R4} + Leaving: {R3} {R2} {R1} + } + .finally {R4} + { + Block[B4] - Block + Predecessors (0) + Statements (0) + Jump if True (Regular) to Block[B6] + IIsNullOperation (OperationKind.IsNull, Type: System.Boolean, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Operand: + ILocalReferenceOperation: o1 (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Next (Regular) Block[B5] + Block[B5] - Block + Predecessors: [B4] + Statements (1) + IInvocationOperation (virtual void System.IDisposable.Dispose()) (OperationKind.Invocation, Type: System.Void, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Instance Receiver: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.IDisposable, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: True, IsUserDefined: False) (MethodSymbol: null) + (ImplicitReference) + Operand: + ILocalReferenceOperation: o1 (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Arguments(0) + Next (Regular) Block[B6] + Block[B6] - Block + Predecessors: [B4] [B5] + Statements (0) + Next (StructuredExceptionHandling) Block[null] + } +} +Block[B7] - Exit + Predecessors: [B1] [B3] + Statements (0) +"; + + VerifyFlowGraphAndDiagnosticsForTest(source, expectedFlowGraph, expectedDiagnostics); + } + + [Fact] + public void UsingDeclaration_InsideForEmbeddedStatement() + { + var source = @" +using System; +using System.Collections; +class C1 : IDisposable +{ + public void Dispose() { } +} +class C2 +{ + public static void M() + /**/{ + for (;;) + using C1 o1 = new C1(); + }/**/ +}"; + + var expectedDiagnostics = new DiagnosticDescription[] { + // file.cs(13,13): error CS1023: Embedded statement cannot be a declaration or labeled statement + // using C1 o1 = new C1(); + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "using C1 o1 = new C1();").WithLocation(13, 13) + }; + + var expectedFlowGraph = @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] + Entering: {R1} +.locals {R1} +{ + Locals: [C1 o1] + Block[B1] - Block + Predecessors: [B0] [B6] + Statements (1) + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Left: + ILocalReferenceOperation: o1 (IsDeclaration: True) (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Right: + IObjectCreationOperation (Constructor: C1..ctor()) (OperationKind.ObjectCreation, Type: C1, IsInvalid) (Syntax: 'new C1()') + Arguments(0) + Initializer: + null + Next (Regular) Block[B2] + Entering: {R2} {R3} + .try {R2, R3} + { + Block[B2] - Block + Predecessors: [B1] + Statements (0) + Next (Regular) Block[B6] + Finalizing: {R4} + Leaving: {R3} {R2} {R1} + } + .finally {R4} + { + Block[B3] - Block + Predecessors (0) + Statements (0) + Jump if True (Regular) to Block[B5] + IIsNullOperation (OperationKind.IsNull, Type: System.Boolean, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Operand: + ILocalReferenceOperation: o1 (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Next (Regular) Block[B4] + Block[B4] - Block + Predecessors: [B3] + Statements (1) + IInvocationOperation (virtual void System.IDisposable.Dispose()) (OperationKind.Invocation, Type: System.Void, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Instance Receiver: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.IDisposable, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: True, IsUserDefined: False) (MethodSymbol: null) + (ImplicitReference) + Operand: + ILocalReferenceOperation: o1 (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Arguments(0) + Next (Regular) Block[B5] + Block[B5] - Block + Predecessors: [B3] [B4] + Statements (0) + Next (StructuredExceptionHandling) Block[null] + } +} +Block[B6] - Block + Predecessors: [B2] + Statements (0) + Next (Regular) Block[B1] + Entering: {R1} +Block[B7] - Exit [UnReachable] + Predecessors (0) + Statements (0) +"; + + VerifyFlowGraphAndDiagnosticsForTest(source, expectedFlowGraph, expectedDiagnostics); + } + + [Fact] + public void UsingDeclaration_InsideForEachEmbeddedStatement() + { + var source = @" +using System; +using System.Collections; +class C1 : IDisposable +{ + public void Dispose() { } +} +class C2 +{ + public static void M(IEnumerable e) + /**/{ + foreach (var o in e) + using C1 o1 = new C1(); + }/**/ +}"; + + var expectedDiagnostics = new DiagnosticDescription[] { + // file.cs(13,13): error CS1023: Embedded statement cannot be a declaration or labeled statement + // using C1 o1 = new C1(); + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "using C1 o1 = new C1();").WithLocation(13, 13) + }; + + var expectedFlowGraph = @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] + Entering: {R1} +.locals {R1} +{ + CaptureIds: [0] + Block[B1] - Block + Predecessors: [B0] + Statements (1) + IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: 'e') + Value: + IInvocationOperation (virtual System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()) (OperationKind.Invocation, Type: System.Collections.IEnumerator, IsImplicit) (Syntax: 'e') + Instance Receiver: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.Collections.IEnumerable, IsImplicit) (Syntax: 'e') + Conversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + (Identity) + Operand: + IParameterReferenceOperation: e (OperationKind.ParameterReference, Type: System.Collections.IEnumerable) (Syntax: 'e') + Arguments(0) + Next (Regular) Block[B2] + Entering: {R2} {R3} + .try {R2, R3} + { + Block[B2] - Block + Predecessors: [B1] [B5] + Statements (0) + Jump if False (Regular) to Block[B12] + IInvocationOperation (virtual System.Boolean System.Collections.IEnumerator.MoveNext()) (OperationKind.Invocation, Type: System.Boolean, IsImplicit) (Syntax: 'e') + Instance Receiver: + IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: System.Collections.IEnumerator, IsImplicit) (Syntax: 'e') + Arguments(0) + Finalizing: {R9} + Leaving: {R3} {R2} {R1} + Next (Regular) Block[B3] + Entering: {R4} + .locals {R4} + { + Locals: [System.Object o] + Block[B3] - Block + Predecessors: [B2] + Statements (1) + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: null, IsImplicit) (Syntax: 'var') + Left: + ILocalReferenceOperation: o (IsDeclaration: True) (OperationKind.LocalReference, Type: System.Object, IsImplicit) (Syntax: 'var') + Right: + IPropertyReferenceOperation: System.Object System.Collections.IEnumerator.Current { get; } (OperationKind.PropertyReference, Type: System.Object, IsImplicit) (Syntax: 'var') + Instance Receiver: + IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: System.Collections.IEnumerator, IsImplicit) (Syntax: 'e') + Next (Regular) Block[B4] + Entering: {R5} + .locals {R5} + { + Locals: [C1 o1] + Block[B4] - Block + Predecessors: [B3] + Statements (1) + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Left: + ILocalReferenceOperation: o1 (IsDeclaration: True) (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Right: + IObjectCreationOperation (Constructor: C1..ctor()) (OperationKind.ObjectCreation, Type: C1, IsInvalid) (Syntax: 'new C1()') + Arguments(0) + Initializer: + null + Next (Regular) Block[B5] + Entering: {R6} {R7} + .try {R6, R7} + { + Block[B5] - Block + Predecessors: [B4] + Statements (0) + Next (Regular) Block[B2] + Finalizing: {R8} + Leaving: {R7} {R6} {R5} {R4} + } + .finally {R8} + { + Block[B6] - Block + Predecessors (0) + Statements (0) + Jump if True (Regular) to Block[B8] + IIsNullOperation (OperationKind.IsNull, Type: System.Boolean, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Operand: + ILocalReferenceOperation: o1 (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Next (Regular) Block[B7] + Block[B7] - Block + Predecessors: [B6] + Statements (1) + IInvocationOperation (virtual void System.IDisposable.Dispose()) (OperationKind.Invocation, Type: System.Void, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Instance Receiver: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.IDisposable, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: True, IsUserDefined: False) (MethodSymbol: null) + (ImplicitReference) + Operand: + ILocalReferenceOperation: o1 (OperationKind.LocalReference, Type: C1, IsInvalid, IsImplicit) (Syntax: 'o1 = new C1()') + Arguments(0) + Next (Regular) Block[B8] + Block[B8] - Block + Predecessors: [B6] [B7] + Statements (0) + Next (StructuredExceptionHandling) Block[null] + } + } + } + } + .finally {R9} + { + CaptureIds: [1] + Block[B9] - Block + Predecessors (0) + Statements (1) + IFlowCaptureOperation: 1 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: 'e') + Value: + IConversionOperation (TryCast: True, Unchecked) (OperationKind.Conversion, Type: System.IDisposable, IsImplicit) (Syntax: 'e') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: True, IsUserDefined: False) (MethodSymbol: null) + (ExplicitReference) + Operand: + IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: System.Collections.IEnumerator, IsImplicit) (Syntax: 'e') + Jump if True (Regular) to Block[B11] + IIsNullOperation (OperationKind.IsNull, Type: System.Boolean, IsImplicit) (Syntax: 'e') + Operand: + IFlowCaptureReferenceOperation: 1 (OperationKind.FlowCaptureReference, Type: System.IDisposable, IsImplicit) (Syntax: 'e') + Next (Regular) Block[B10] + Block[B10] - Block + Predecessors: [B9] + Statements (1) + IInvocationOperation (virtual void System.IDisposable.Dispose()) (OperationKind.Invocation, Type: System.Void, IsImplicit) (Syntax: 'e') + Instance Receiver: + IFlowCaptureReferenceOperation: 1 (OperationKind.FlowCaptureReference, Type: System.IDisposable, IsImplicit) (Syntax: 'e') + Arguments(0) + Next (Regular) Block[B11] + Block[B11] - Block + Predecessors: [B9] [B10] + Statements (0) + Next (StructuredExceptionHandling) Block[null] + } +} +Block[B12] - Exit + Predecessors: [B2] + Statements (0) +"; + + VerifyFlowGraphAndDiagnosticsForTest(source, expectedFlowGraph, expectedDiagnostics); + } } } diff --git a/src/Compilers/Core/Portable/Operations/OperationNodes.cs b/src/Compilers/Core/Portable/Operations/OperationNodes.cs index 23a84af044677..dab2d1375d057 100644 --- a/src/Compilers/Core/Portable/Operations/OperationNodes.cs +++ b/src/Compilers/Core/Portable/Operations/OperationNodes.cs @@ -543,8 +543,9 @@ private BlockOperation(ImmutableArray statements, SemanticModel sema // Intentionally skipping SetParentOperation: this is used by CreateTemporaryBlock for the purposes of the // control flow factory, to temporarily create a new block for use in emulating the "block" a using variable // declaration introduces. These statements already have a parent node, and `SetParentOperation`'s verification - // would fail because that parent isn't this. - Debug.Assert(statements.All(s => s.Parent != this && s.Parent!.Kind == OperationKind.Block)); + // would fail because that parent isn't this. In error cases, the parent can also be a switch statement if + // the using declaration was used directly as an embedded statement in the case without a block. + Debug.Assert(statements.All(s => s.Parent != this && s.Parent!.Kind is OperationKind.Block or OperationKind.SwitchCase)); Operations = statements; Locals = ImmutableArray.Empty; } From df1c84a5bb5f59d3c596b3299fa536608453fccd Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 12 Feb 2021 17:30:10 -0800 Subject: [PATCH 07/14] Add IOperation Test Hook helper --- .../Test/Core/Compilation/CompilationExtensions.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Compilers/Test/Core/Compilation/CompilationExtensions.cs b/src/Compilers/Test/Core/Compilation/CompilationExtensions.cs index dfb6bf08da6ce..ab6f856893d93 100644 --- a/src/Compilers/Test/Core/Compilation/CompilationExtensions.cs +++ b/src/Compilers/Test/Core/Compilation/CompilationExtensions.cs @@ -3,6 +3,8 @@ // See the LICENSE file in the project root for more information. #nullable disable +// Uncomment to enable the IOperation test hook on all test runs. Do not commit this uncommented. +//#define ROSLYN_TEST_IOPERATION using System; using System.Collections.Generic; @@ -30,7 +32,13 @@ namespace Microsoft.CodeAnalysis.Test.Utilities { public static class CompilationExtensions { - internal static bool EnableVerifyIOperation { get; } = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("ROSLYN_TEST_IOPERATION")); + internal static bool EnableVerifyIOperation { get; } = +#if ROSLYN_TEST_IOPERATION + true; +#else + !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("ROSLYN_TEST_IOPERATION")); +#endif + internal static bool EnableVerifyUsedAssemblies { get; } = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("ROSLYN_TEST_USEDASSEMBLIES")); internal static ImmutableArray EmitToArray( From e53af812775dcce5ba2c77aeb3dfa36c8a97ccd1 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 12 Feb 2021 17:30:47 -0800 Subject: [PATCH 08/14] Adjust conditional access children for bad code. --- ...ationTests_IConditionalAccessExpression.cs | 90 +++++++++++++++++++ .../Operations/ControlFlowGraphBuilder.cs | 24 +++++ 2 files changed, 114 insertions(+) diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IConditionalAccessExpression.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IConditionalAccessExpression.cs index 957c6f4012e1d..9ec20eea45ccd 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IConditionalAccessExpression.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IConditionalAccessExpression.cs @@ -1563,5 +1563,95 @@ void M(C c) Statements (0) ", DiagnosticDescription.None); } + + [Fact] + public void InvalidConditionalAccess() + { + var code = @" +class C +{ + void M() + /**/{ + _ = 123?[1, 2]; + }/**/ +} +"; + + var expectedDiagnostics = new[] + { + // file.cs(6,16): error CS0023: Operator '?' cannot be applied to operand of type 'int' + // _ = 123?[1, 2]; + Diagnostic(ErrorCode.ERR_BadUnaryOp, "?").WithArguments("?", "int").WithLocation(6, 16) + }; + + VerifyFlowGraphAndDiagnosticsForTest(code, @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] + Entering: {R1} {R2} +.locals {R1} +{ + CaptureIds: [0] + .locals {R2} + { + CaptureIds: [1] [2] [3] + Block[B1] - Block + Predecessors: [B0] + Statements (3) + IFlowCaptureOperation: 1 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: '1') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + IFlowCaptureOperation: 2 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: '2') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2) (Syntax: '2') + IFlowCaptureOperation: 3 (OperationKind.FlowCapture, Type: null, IsInvalid, IsImplicit) (Syntax: '123') + Value: + IInvalidOperation (OperationKind.Invalid, Type: ?, IsInvalid, IsImplicit) (Syntax: '123') + Children(1): + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 123, IsInvalid) (Syntax: '123') + Jump if True (Regular) to Block[B3] + IIsNullOperation (OperationKind.IsNull, Type: System.Boolean, IsInvalid, IsImplicit) (Syntax: '123') + Operand: + IFlowCaptureReferenceOperation: 3 (OperationKind.FlowCaptureReference, Type: ?, IsInvalid, IsImplicit) (Syntax: '123') + Leaving: {R2} + Next (Regular) Block[B2] + Block[B2] - Block + Predecessors: [B1] + Statements (1) + IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsInvalid, IsImplicit) (Syntax: '[1, 2]') + Value: + IInvalidOperation (OperationKind.Invalid, Type: ?, IsInvalid) (Syntax: '[1, 2]') + Children(3): + IFlowCaptureReferenceOperation: 1 (OperationKind.FlowCaptureReference, Type: System.Int32, Constant: 1, IsImplicit) (Syntax: '1') + IFlowCaptureReferenceOperation: 2 (OperationKind.FlowCaptureReference, Type: System.Int32, Constant: 2, IsImplicit) (Syntax: '2') + IFlowCaptureReferenceOperation: 3 (OperationKind.FlowCaptureReference, Type: ?, IsInvalid, IsImplicit) (Syntax: '123') + Next (Regular) Block[B4] + Leaving: {R2} + } + Block[B3] - Block + Predecessors: [B1] + Statements (1) + IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsInvalid, IsImplicit) (Syntax: '123') + Value: + IDefaultValueOperation (OperationKind.DefaultValue, Type: ?, Constant: null, IsInvalid, IsImplicit) (Syntax: '123') + Next (Regular) Block[B4] + Block[B4] - Block + Predecessors: [B2] [B3] + Statements (1) + IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null, IsInvalid) (Syntax: '_ = 123?[1, 2];') + Expression: + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: ?, IsInvalid) (Syntax: '_ = 123?[1, 2]') + Left: + IDiscardOperation (Symbol: ? _) (OperationKind.Discard, Type: ?) (Syntax: '_') + Right: + IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: ?, IsInvalid, IsImplicit) (Syntax: '123?[1, 2]') + Next (Regular) Block[B5] + Leaving: {R1} +} +Block[B5] - Exit + Predecessors: [B4] + Statements (0) +", expectedDiagnostics); + } } } diff --git a/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs b/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs index 314a3491cf714..1c994e3d1790f 100644 --- a/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs +++ b/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs @@ -3301,6 +3301,11 @@ void resetConditionalAccessTracker() static bool isConditionalAccessInstancePresentInChildren(IOperation operation) { + if (operation is InvalidOperation invalidOperation) + { + return checkInvalidChildren(invalidOperation); + } + // The conditional access should always be first leaf node in the subtree when performing a depth-first search. Visit the first child recursively // until we either reach the bottom, or find the conditional access. Operation currentOperation = (Operation)operation; @@ -3310,12 +3315,31 @@ static bool isConditionalAccessInstancePresentInChildren(IOperation operation) { return true; } + else if (operation is InvalidOperation invalidChild) + { + return checkInvalidChildren(invalidChild); + } currentOperation = (Operation)enumerator.Current; } return false; } + + static bool checkInvalidChildren(InvalidOperation operation) + { + // Invalid operations can have children ordering that doesn't put the conditional access instance first. For these cases, + // use a recursive check + foreach (var child in operation.ChildOperations) + { + if (child is IConditionalAccessInstanceOperation || isConditionalAccessInstancePresentInChildren(child)) + { + return true; + } + } + + return false; + } } public override IOperation VisitConditionalAccessInstance(IConditionalAccessInstanceOperation operation, int? captureIdForResult) From 2d4a4b563b9b45ce75266ebfc829494598bc6d87 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 16 Feb 2021 12:00:14 -0800 Subject: [PATCH 09/14] Remove incorrect assert. --- .../CSharp/Portable/Symbols/TypeWithAnnotations.cs | 2 -- .../Test/Semantic/Semantics/NullableReferenceTypesTests.cs | 6 ++++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs b/src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs index d615fae84c766..fd2c313e1b886 100644 --- a/src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs +++ b/src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs @@ -942,8 +942,6 @@ private TypeSymbol GetResolvedType() { if ((object)_resolved == null) { - Debug.Assert(_underlying.IsSafeToResolve()); - TryForceResolve(asValueType: _underlying.Type.IsValueType); } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index bd15620ed9f2e..e7dcc8b1a5108 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -13437,6 +13437,12 @@ class B : IA } "; var compilation = CreateCompilation(new[] { source }, options: WithNullableEnable()); + + var tree = compilation.SyntaxTrees[0]; + var model = compilation.GetSemanticModel(tree); + var returnStatement = tree.GetRoot().DescendantNodes().OfType().Skip(1).Single(); + AssertEx.Equal("S?[]", model.GetTypeInfo(returnStatement.Expression).Type.ToTestDisplayString()); + compilation.VerifyDiagnostics( // (17,6): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. // S?[] IA.M2() where S : class From 2cbe6de5379738058c78a06996991a622a4699f9 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 19 Feb 2021 10:53:11 -0800 Subject: [PATCH 10/14] Attribute binding changes: * Simplify named argument binding by making all named args specifically BoundAssignmentOperators. They already were, this carries the knowledge through the rest of the code. * Add tests for invalid attribute binding scenarios. * Only bind constructor params for error recovery in bad cases. * Use the correct binder when binding for error recover to avoid runaway binding. --- .../Portable/Binder/Binder_Attributes.cs | 63 ++-- .../CSharp/Portable/BoundTree/BoundNodes.xml | 2 +- .../CSharp/Portable/BoundTree/Expression.cs | 2 +- .../Generated/BoundNodes.xml.Generated.cs | 10 +- .../CSharp/Test/Emit/CodeGen/PatternTests.cs | 296 ++++++++++++++++++ .../Core/Portable/Collections/StaticCast.cs | 3 - 6 files changed, 335 insertions(+), 41 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs index 16faff3c1b34e..c4c8f46f6d1db 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs @@ -180,19 +180,28 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a Error(diagnostics, ErrorCode.ERR_AttributeCtorInParameter, node, attributeConstructor.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat)); } - boundConstructorArguments = BuildArgumentsForErrorRecovery(constructorArguments, ImmutableArray.Create(attributeConstructor)); - constructorArguments.Arguments.Free(); + if (resultKind != LookupResultKind.Viable) + { + boundConstructorArguments = attributeArgumentBinder.BuildArgumentsForErrorRecovery(constructorArguments, ImmutableArray.Create(attributeConstructor)); + constructorArguments.Arguments.Free(); + } + else + { + boundConstructorArguments = constructorArguments.Arguments.ToImmutableAndFree(); + } } else { - boundConstructorArguments = constructorArguments.Arguments.SelectAsArrayInPlaceAndFree( - (arg, @this) => @this.BindToTypeForErrorRecovery(arg), this); + Debug.Assert(resultKind != LookupResultKind.Viable); + boundConstructorArguments = + constructorArguments.Arguments.SelectAsArrayInPlaceAndFree((arg, attributeArgumentBinder) => attributeArgumentBinder.BindToTypeForErrorRecovery(arg), attributeArgumentBinder); } + Debug.Assert(boundConstructorArguments.All(a => !a.NeedsToBeConverted())); + ImmutableArray boundConstructorArgumentNamesOpt = constructorArguments.GetNames(); - ImmutableArray boundNamedArguments = - analyzedArguments.NamedArguments?.SelectAsArrayInPlaceAndFree((namedArg, @this) => @this.BindToTypeForErrorRecovery(namedArg), this) - ?? ImmutableArray.Empty; + ImmutableArray boundNamedArguments = analyzedArguments.NamedArguments?.ToImmutableAndFree() ?? ImmutableArray.Empty; + Debug.Assert(boundNamedArguments.All(arg => !arg.Right.NeedsToBeConverted())); constructorArguments.Free(); @@ -312,7 +321,7 @@ private AnalyzedAttributeArguments BindAttributeArguments( BindingDiagnosticBag diagnostics) { var boundConstructorArguments = AnalyzedArguments.GetInstance(); - ArrayBuilder? boundNamedArgumentsBuilder = null; + ArrayBuilder? boundNamedArgumentsBuilder = null; if (attributeArgumentList != null) { @@ -352,7 +361,7 @@ private AnalyzedAttributeArguments BindAttributeArguments( string argumentName = argument.NameEquals.Name.Identifier.ValueText!; if (boundNamedArgumentsBuilder == null) { - boundNamedArgumentsBuilder = ArrayBuilder.GetInstance(); + boundNamedArgumentsBuilder = ArrayBuilder.GetInstance(); boundNamedArgumentsSet = new HashSet(); } else if (boundNamedArgumentsSet!.Contains(argumentName)) @@ -361,7 +370,7 @@ private AnalyzedAttributeArguments BindAttributeArguments( Error(diagnostics, ErrorCode.ERR_DuplicateNamedAttributeArgument, argument, argumentName); } - BoundExpression boundNamedArgument = BindNamedAttributeArgument(argument, attributeType, diagnostics); + BoundAssignmentOperator boundNamedArgument = BindNamedAttributeArgument(argument, attributeType, diagnostics); boundNamedArgumentsBuilder.Add(boundNamedArgument); boundNamedArgumentsSet.Add(argumentName); } @@ -371,7 +380,7 @@ private AnalyzedAttributeArguments BindAttributeArguments( return new AnalyzedAttributeArguments(boundConstructorArguments, boundNamedArgumentsBuilder); } - private BoundExpression BindNamedAttributeArgument(AttributeArgumentSyntax namedArgument, NamedTypeSymbol attributeType, BindingDiagnosticBag diagnostics) + private BoundAssignmentOperator BindNamedAttributeArgument(AttributeArgumentSyntax namedArgument, NamedTypeSymbol attributeType, BindingDiagnosticBag diagnostics) { bool wasError; LookupResultKind resultKind; @@ -987,7 +996,7 @@ public ImmutableArray VisitArguments(ImmutableArray> VisitNamedArguments(ImmutableArray arguments, BindingDiagnosticBag diagnostics, ref bool attrHasErrors) + public ImmutableArray> VisitNamedArguments(ImmutableArray arguments, BindingDiagnosticBag diagnostics, ref bool attrHasErrors) { ArrayBuilder>? builder = null; foreach (var argument in arguments) @@ -1013,28 +1022,20 @@ public ImmutableArray> VisitNamedArguments(I return builder.ToImmutableAndFree(); } - private KeyValuePair? VisitNamedArgument(BoundExpression argument, BindingDiagnosticBag diagnostics, ref bool attrHasErrors) + private KeyValuePair? VisitNamedArgument(BoundAssignmentOperator assignment, BindingDiagnosticBag diagnostics, ref bool attrHasErrors) { KeyValuePair? visitedArgument = null; - switch (argument.Kind) + switch (assignment.Left.Kind) { - case BoundKind.AssignmentOperator: - var assignment = (BoundAssignmentOperator)argument; - - switch (assignment.Left.Kind) - { - case BoundKind.FieldAccess: - var fa = (BoundFieldAccess)assignment.Left; - visitedArgument = new KeyValuePair(fa.FieldSymbol.Name, VisitExpression(assignment.Right, diagnostics, ref attrHasErrors, argument.HasAnyErrors)); - break; - - case BoundKind.PropertyAccess: - var pa = (BoundPropertyAccess)assignment.Left; - visitedArgument = new KeyValuePair(pa.PropertySymbol.Name, VisitExpression(assignment.Right, diagnostics, ref attrHasErrors, argument.HasAnyErrors)); - break; - } + case BoundKind.FieldAccess: + var fa = (BoundFieldAccess)assignment.Left; + visitedArgument = new KeyValuePair(fa.FieldSymbol.Name, VisitExpression(assignment.Right, diagnostics, ref attrHasErrors, assignment.HasAnyErrors)); + break; + case BoundKind.PropertyAccess: + var pa = (BoundPropertyAccess)assignment.Left; + visitedArgument = new KeyValuePair(pa.PropertySymbol.Name, VisitExpression(assignment.Right, diagnostics, ref attrHasErrors, assignment.HasAnyErrors)); break; } @@ -1253,9 +1254,9 @@ private static TypedConstant CreateTypedConstant(BoundExpression node, TypedCons private struct AnalyzedAttributeArguments { internal readonly AnalyzedArguments ConstructorArguments; - internal readonly ArrayBuilder? NamedArguments; + internal readonly ArrayBuilder? NamedArguments; - internal AnalyzedAttributeArguments(AnalyzedArguments constructorArguments, ArrayBuilder? namedArguments) + internal AnalyzedAttributeArguments(AnalyzedArguments constructorArguments, ArrayBuilder? namedArguments) { this.ConstructorArguments = constructorArguments; this.NamedArguments = namedArguments; diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index 0f9ece1fc4f05..40591d3932cc3 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -1615,7 +1615,7 @@ - + diff --git a/src/Compilers/CSharp/Portable/BoundTree/Expression.cs b/src/Compilers/CSharp/Portable/BoundTree/Expression.cs index 8f5be8db406c3..ab4a498be4c80 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/Expression.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/Expression.cs @@ -73,7 +73,7 @@ internal partial class BoundAnonymousObjectCreationExpression internal partial class BoundAttribute { - protected override ImmutableArray Children => StaticCast.From(this.ConstructorArguments.AddRange(this.NamedArguments)); + protected override ImmutableArray Children => StaticCast.From(this.ConstructorArguments.AddRange(StaticCast.From(this.NamedArguments))); } internal partial class BoundQueryClause diff --git a/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs b/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs index 1423b578476ef..680d8ad611fea 100644 --- a/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs +++ b/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs @@ -5756,7 +5756,7 @@ public BoundEventAssignmentOperator Update(EventSymbol @event, bool isAddition, internal sealed partial class BoundAttribute : BoundExpression { - public BoundAttribute(SyntaxNode syntax, MethodSymbol? constructor, ImmutableArray constructorArguments, ImmutableArray constructorArgumentNamesOpt, ImmutableArray constructorArgumentsToParamsOpt, bool constructorExpanded, ImmutableArray namedArguments, LookupResultKind resultKind, TypeSymbol type, bool hasErrors = false) + public BoundAttribute(SyntaxNode syntax, MethodSymbol? constructor, ImmutableArray constructorArguments, ImmutableArray constructorArgumentNamesOpt, ImmutableArray constructorArgumentsToParamsOpt, bool constructorExpanded, ImmutableArray namedArguments, LookupResultKind resultKind, TypeSymbol type, bool hasErrors = false) : base(BoundKind.Attribute, syntax, type, hasErrors || constructorArguments.HasErrors() || namedArguments.HasErrors()) { @@ -5786,14 +5786,14 @@ public BoundAttribute(SyntaxNode syntax, MethodSymbol? constructor, ImmutableArr public bool ConstructorExpanded { get; } - public ImmutableArray NamedArguments { get; } + public ImmutableArray NamedArguments { get; } private readonly LookupResultKind _ResultKind; public override LookupResultKind ResultKind { get { return _ResultKind; } } [DebuggerStepThrough] public override BoundNode? Accept(BoundTreeVisitor visitor) => visitor.VisitAttribute(this); - public BoundAttribute Update(MethodSymbol? constructor, ImmutableArray constructorArguments, ImmutableArray constructorArgumentNamesOpt, ImmutableArray constructorArgumentsToParamsOpt, bool constructorExpanded, ImmutableArray namedArguments, LookupResultKind resultKind, TypeSymbol type) + public BoundAttribute Update(MethodSymbol? constructor, ImmutableArray constructorArguments, ImmutableArray constructorArgumentNamesOpt, ImmutableArray constructorArgumentsToParamsOpt, bool constructorExpanded, ImmutableArray namedArguments, LookupResultKind resultKind, TypeSymbol type) { if (!Symbols.SymbolEqualityComparer.ConsiderEverything.Equals(constructor, this.Constructor) || constructorArguments != this.ConstructorArguments || constructorArgumentNamesOpt != this.ConstructorArgumentNamesOpt || constructorArgumentsToParamsOpt != this.ConstructorArgumentsToParamsOpt || constructorExpanded != this.ConstructorExpanded || namedArguments != this.NamedArguments || resultKind != this.ResultKind || !TypeSymbol.Equals(type, this.Type, TypeCompareKind.ConsiderEverything)) { @@ -10526,7 +10526,7 @@ internal abstract partial class BoundTreeRewriter : BoundTreeVisitor public override BoundNode? VisitAttribute(BoundAttribute node) { ImmutableArray constructorArguments = this.VisitList(node.ConstructorArguments); - ImmutableArray namedArguments = this.VisitList(node.NamedArguments); + ImmutableArray namedArguments = this.VisitList(node.NamedArguments); TypeSymbol? type = this.VisitType(node.Type); return node.Update(node.Constructor, constructorArguments, node.ConstructorArgumentNamesOpt, node.ConstructorArgumentsToParamsOpt, node.ConstructorExpanded, namedArguments, node.ResultKind, type); } @@ -12407,7 +12407,7 @@ public NullabilityRewriter(ImmutableDictionary constructorArguments = this.VisitList(node.ConstructorArguments); - ImmutableArray namedArguments = this.VisitList(node.NamedArguments); + ImmutableArray namedArguments = this.VisitList(node.NamedArguments); BoundAttribute updatedNode; if (_updatedNullabilities.TryGetValue(node, out (NullabilityInfo Info, TypeSymbol? Type) infoAndType)) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs index ea2c494b56e9a..5c97040f8c82a 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs @@ -4957,6 +4957,302 @@ public class B "); } + [Fact] + public void TargetTypedSwitch_Attribute_NamedArgument() + { + var source = @" +using System; +class Program +{ + [My(Value = 1 switch { 1 => 1, _ => 2 })] + public static void M1() { } + + [My(Value = 1 switch { 1 => new A(), _ => new B() })] + public static void M2() { } + + [My(Value = 1 switch { 1 => 1, _ => string.Empty })] + public static void M3() { } +} +public class MyAttribute : Attribute +{ + public MyAttribute() { } + public int Value { get; set; } +} +public class A +{ + public static implicit operator int(A a) => 4; +} +public class B +{ + public static implicit operator int(B b) => 2; +} +"; + var compilation = CreateCompilation(source); + compilation.VerifyDiagnostics( + // (5,17): error CS0182: An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type + // [My(Value = 1 switch { 1 => 1, _ => 2 })] + Diagnostic(ErrorCode.ERR_BadAttributeArgument, "1 switch { 1 => 1, _ => 2 }").WithLocation(5, 17), + // (8,17): error CS0182: An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type + // [My(Value = 1 switch { 1 => new A(), _ => new B() })] + Diagnostic(ErrorCode.ERR_BadAttributeArgument, "1 switch { 1 => new A(), _ => new B() }").WithLocation(8, 17), + // (11,41): error CS0029: Cannot implicitly convert type 'string' to 'int' + // [My(Value = 1 switch { 1 => 1, _ => string.Empty })] + Diagnostic(ErrorCode.ERR_NoImplicitConv, "string.Empty").WithArguments("string", "int").WithLocation(11, 41) + ); + + var tree = compilation.SyntaxTrees[0]; + var model = compilation.GetSemanticModel(tree); + + var attributeArguments = tree.GetRoot().DescendantNodes().OfType().ToArray(); + + VerifyOperationTreeForNode(compilation, model, attributeArguments[0], @" +ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Int32, IsInvalid) (Syntax: 'Value = 1 s ... 1, _ => 2 }') + Left: + IPropertyReferenceOperation: System.Int32 MyAttribute.Value { get; set; } (OperationKind.PropertyReference, Type: System.Int32) (Syntax: 'Value') + Instance Receiver: + null + Right: + ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: System.Int32, IsInvalid) (Syntax: '1 switch { ... 1, _ => 2 }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '1 => 1') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsInvalid) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '_ => 2') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null, IsInvalid) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2, IsInvalid) (Syntax: '2') +"); + + VerifyOperationTreeForNode(compilation, model, attributeArguments[1], @" +ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Int32, IsInvalid) (Syntax: 'Value = 1 s ... > new B() }') + Left: + IPropertyReferenceOperation: System.Int32 MyAttribute.Value { get; set; } (OperationKind.PropertyReference, Type: System.Int32) (Syntax: 'Value') + Instance Receiver: + null + Right: + ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: System.Int32, IsInvalid) (Syntax: '1 switch { ... > new B() }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '1 => new A()') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsInvalid) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid) (Syntax: '1') + Value: + IConversionOperation (TryCast: False, Unchecked) (OperatorMethod: System.Int32 A.op_Implicit(A a)) (OperationKind.Conversion, Type: System.Int32, IsInvalid, IsImplicit) (Syntax: 'new A()') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: True) (MethodSymbol: System.Int32 A.op_Implicit(A a)) + Operand: + IObjectCreationOperation (Constructor: A..ctor()) (OperationKind.ObjectCreation, Type: A, IsInvalid) (Syntax: 'new A()') + Arguments(0) + Initializer: + null + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '_ => new B()') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null, IsInvalid) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + IConversionOperation (TryCast: False, Unchecked) (OperatorMethod: System.Int32 B.op_Implicit(B b)) (OperationKind.Conversion, Type: System.Int32, IsInvalid, IsImplicit) (Syntax: 'new B()') + Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: True) (MethodSymbol: System.Int32 B.op_Implicit(B b)) + Operand: + IObjectCreationOperation (Constructor: B..ctor()) (OperationKind.ObjectCreation, Type: B, IsInvalid) (Syntax: 'new B()') + Arguments(0) + Initializer: + null +"); + + VerifyOperationTreeForNode(compilation, model, attributeArguments[2], @" +ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Int32, IsInvalid) (Syntax: 'Value = 1 s ... ing.Empty }') + Left: + IPropertyReferenceOperation: System.Int32 MyAttribute.Value { get; set; } (OperationKind.PropertyReference, Type: System.Int32) (Syntax: 'Value') + Instance Receiver: + null + Right: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.Int32, IsInvalid, IsImplicit) (Syntax: '1 switch { ... ing.Empty }') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: ?, IsInvalid) (Syntax: '1 switch { ... ing.Empty }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null) (Syntax: '1 => 1') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Value: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsImplicit) (Syntax: '1') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null, IsInvalid) (Syntax: '_ => string.Empty') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsInvalid, IsImplicit) (Syntax: 'string.Empty') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + IFieldReferenceOperation: System.String System.String.Empty (Static) (OperationKind.FieldReference, Type: System.String, IsInvalid) (Syntax: 'string.Empty') + Instance Receiver: + null +"); + } + + [Fact] + public void TargetTypedSwitch_Attribute_MissingNamedArgument() + { + var source = @" +using System; +class Program +{ + [My(Value = 1 switch { 1 => 1, _ => 2 })] + public static void M1() { } + + [My(Value = 1 switch { 1 => new A(), _ => new B() })] + public static void M2() { } + + [My(Value = 1 switch { 1 => 1, _ => string.Empty })] + public static void M3() { } +} +public class MyAttribute : Attribute +{ + public MyAttribute() { } +} +public class A +{ + public static implicit operator int(A a) => 4; +} +public class B +{ + public static implicit operator int(B b) => 2; +} +"; + var compilation = CreateCompilation(source); + compilation.VerifyDiagnostics( + // (5,9): error CS0246: The type or namespace name 'Value' could not be found (are you missing a using directive or an assembly reference?) + // [My(Value = 1 switch { 1 => 1, _ => 2 })] + Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "Value").WithArguments("Value").WithLocation(5, 9), + // (8,9): error CS0246: The type or namespace name 'Value' could not be found (are you missing a using directive or an assembly reference?) + // [My(Value = 1 switch { 1 => new A(), _ => new B() })] + Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "Value").WithArguments("Value").WithLocation(8, 9), + // (11,9): error CS0246: The type or namespace name 'Value' could not be found (are you missing a using directive or an assembly reference?) + // [My(Value = 1 switch { 1 => 1, _ => string.Empty })] + Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "Value").WithArguments("Value").WithLocation(11, 9) + ); + + var tree = compilation.SyntaxTrees[0]; + var model = compilation.GetSemanticModel(tree); + + var attributeArguments = tree.GetRoot().DescendantNodes().OfType().ToArray(); + + VerifyOperationTreeForNode(compilation, model, attributeArguments[0], @" +ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: ?, IsInvalid) (Syntax: 'Value = 1 s ... 1, _ => 2 }') + Left: + IInvalidOperation (OperationKind.Invalid, Type: ?, IsInvalid) (Syntax: 'Value') + Children(0) + Right: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsImplicit) (Syntax: '1 switch { ... 1, _ => 2 }') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: System.Int32) (Syntax: '1 switch { ... 1, _ => 2 }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null) (Syntax: '1 => 1') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null) (Syntax: '_ => 2') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2) (Syntax: '2') +"); + + VerifyOperationTreeForNode(compilation, model, attributeArguments[1], @" +ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: ?, IsInvalid) (Syntax: 'Value = 1 s ... > new B() }') + Left: + IInvalidOperation (OperationKind.Invalid, Type: ?, IsInvalid) (Syntax: 'Value') + Children(0) + Right: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsImplicit) (Syntax: '1 switch { ... > new B() }') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: ?) (Syntax: '1 switch { ... > new B() }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null) (Syntax: '1 => new A()') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Value: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsImplicit) (Syntax: 'new A()') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + IObjectCreationOperation (Constructor: A..ctor()) (OperationKind.ObjectCreation, Type: A) (Syntax: 'new A()') + Arguments(0) + Initializer: + null + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null) (Syntax: '_ => new B()') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsImplicit) (Syntax: 'new B()') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + IObjectCreationOperation (Constructor: B..ctor()) (OperationKind.ObjectCreation, Type: B) (Syntax: 'new B()') + Arguments(0) + Initializer: + null +"); + + VerifyOperationTreeForNode(compilation, model, attributeArguments[2], @" +ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: ?, IsInvalid) (Syntax: 'Value = 1 s ... ing.Empty }') + Left: + IInvalidOperation (OperationKind.Invalid, Type: ?, IsInvalid) (Syntax: 'Value') + Children(0) + Right: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsImplicit) (Syntax: '1 switch { ... ing.Empty }') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + ISwitchExpressionOperation (2 arms) (OperationKind.SwitchExpression, Type: ?) (Syntax: '1 switch { ... ing.Empty }') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Arms(2): + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null) (Syntax: '1 => 1') + Pattern: + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null) (Syntax: '1') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Value: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsImplicit) (Syntax: '1') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + ISwitchExpressionArmOperation (0 locals) (OperationKind.SwitchExpressionArm, Type: null) (Syntax: '_ => string.Empty') + Pattern: + IDiscardPatternOperation (OperationKind.DiscardPattern, Type: null) (Syntax: '_') (InputType: System.Int32, NarrowedType: System.Int32) + Value: + IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: ?, IsImplicit) (Syntax: 'string.Empty') + Conversion: CommonConversion (Exists: False, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null) + Operand: + IFieldReferenceOperation: System.String System.String.Empty (Static) (OperationKind.FieldReference, Type: System.String) (Syntax: 'string.Empty') + Instance Receiver: + null +"); + } + [Fact] public void TargetTypedSwitch_As() { diff --git a/src/Compilers/Core/Portable/Collections/StaticCast.cs b/src/Compilers/Core/Portable/Collections/StaticCast.cs index c051c2972ef7b..9a869e8796138 100644 --- a/src/Compilers/Core/Portable/Collections/StaticCast.cs +++ b/src/Compilers/Core/Portable/Collections/StaticCast.cs @@ -10,10 +10,7 @@ internal static class StaticCast { internal static ImmutableArray From(ImmutableArray from) where TDerived : class, T { - // Remove the pragma when we get a version with https://github.com/dotnet/runtime/issues/39799 fixed -#pragma warning disable CS8634 return ImmutableArray.CastUp(from); -#pragma warning restore CS8634 } } } From 49ed0a40b8cfa52023d78e736163e6fa3ff0bbc4 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 19 Feb 2021 11:03:52 -0800 Subject: [PATCH 11/14] Fix iteration bug in conditional access, add test to demonstrate. --- ...ationTests_IConditionalAccessExpression.cs | 96 ++++++++++++++++++- .../Operations/ControlFlowGraphBuilder.cs | 2 +- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IConditionalAccessExpression.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IConditionalAccessExpression.cs index 9ec20eea45ccd..dcf4952a800bb 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IConditionalAccessExpression.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IConditionalAccessExpression.cs @@ -1565,7 +1565,7 @@ void M(C c) } [Fact] - public void InvalidConditionalAccess() + public void InvalidConditionalAccess_01() { var code = @" class C @@ -1648,6 +1648,100 @@ void M() Next (Regular) Block[B5] Leaving: {R1} } +Block[B5] - Exit + Predecessors: [B4] + Statements (0) +", expectedDiagnostics); + } + + [Fact] + public void InvalidConditionalAccess_02() + { + var code = @" +class C +{ + void M() + /**/{ + _ = 123?[1, 2].ToString(); + }/**/ +} +"; + + var expectedDiagnostics = new[] + { + // file.cs(6,16): error CS0023: Operator '?' cannot be applied to operand of type 'int' + // _ = 123?[1, 2]; + Diagnostic(ErrorCode.ERR_BadUnaryOp, "?").WithArguments("?", "int").WithLocation(6, 16) + }; + + VerifyFlowGraphAndDiagnosticsForTest(code, @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] + Entering: {R1} {R2} +.locals {R1} +{ + CaptureIds: [0] + .locals {R2} + { + CaptureIds: [1] [2] [3] + Block[B1] - Block + Predecessors: [B0] + Statements (3) + IFlowCaptureOperation: 1 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: '1') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + IFlowCaptureOperation: 2 (OperationKind.FlowCapture, Type: null, IsImplicit) (Syntax: '2') + Value: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2) (Syntax: '2') + IFlowCaptureOperation: 3 (OperationKind.FlowCapture, Type: null, IsInvalid, IsImplicit) (Syntax: '123') + Value: + IInvalidOperation (OperationKind.Invalid, Type: ?, IsInvalid, IsImplicit) (Syntax: '123') + Children(1): + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 123, IsInvalid) (Syntax: '123') + Jump if True (Regular) to Block[B3] + IIsNullOperation (OperationKind.IsNull, Type: System.Boolean, IsInvalid, IsImplicit) (Syntax: '123') + Operand: + IFlowCaptureReferenceOperation: 3 (OperationKind.FlowCaptureReference, Type: ?, IsInvalid, IsImplicit) (Syntax: '123') + Leaving: {R2} + Next (Regular) Block[B2] + Block[B2] - Block + Predecessors: [B1] + Statements (1) + IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsInvalid, IsImplicit) (Syntax: '[1, 2].ToString()') + Value: + IInvalidOperation (OperationKind.Invalid, Type: ?, IsInvalid) (Syntax: '[1, 2].ToString()') + Children(1): + IOperation: (OperationKind.None, Type: null, IsInvalid) (Syntax: '[1, 2].ToString') + Children(1): + IInvalidOperation (OperationKind.Invalid, Type: ?, IsInvalid) (Syntax: '[1, 2]') + Children(3): + IFlowCaptureReferenceOperation: 1 (OperationKind.FlowCaptureReference, Type: System.Int32, Constant: 1, IsImplicit) (Syntax: '1') + IFlowCaptureReferenceOperation: 2 (OperationKind.FlowCaptureReference, Type: System.Int32, Constant: 2, IsImplicit) (Syntax: '2') + IFlowCaptureReferenceOperation: 3 (OperationKind.FlowCaptureReference, Type: ?, IsInvalid, IsImplicit) (Syntax: '123') + Next (Regular) Block[B4] + Leaving: {R2} + } + Block[B3] - Block + Predecessors: [B1] + Statements (1) + IFlowCaptureOperation: 0 (OperationKind.FlowCapture, Type: null, IsInvalid, IsImplicit) (Syntax: '123') + Value: + IDefaultValueOperation (OperationKind.DefaultValue, Type: ?, Constant: null, IsInvalid, IsImplicit) (Syntax: '123') + Next (Regular) Block[B4] + Block[B4] - Block + Predecessors: [B2] [B3] + Statements (1) + IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null, IsInvalid) (Syntax: '_ = 123?[1, ... ToString();') + Expression: + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: ?, IsInvalid) (Syntax: '_ = 123?[1, ... .ToString()') + Left: + IDiscardOperation (Symbol: ? _) (OperationKind.Discard, Type: ?) (Syntax: '_') + Right: + IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: ?, IsInvalid, IsImplicit) (Syntax: '123?[1, 2].ToString()') + Next (Regular) Block[B5] + Leaving: {R1} +} Block[B5] - Exit Predecessors: [B4] Statements (0) diff --git a/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs b/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs index 1c994e3d1790f..62885c64f7492 100644 --- a/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs +++ b/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs @@ -3315,7 +3315,7 @@ static bool isConditionalAccessInstancePresentInChildren(IOperation operation) { return true; } - else if (operation is InvalidOperation invalidChild) + else if (enumerator.Current is InvalidOperation invalidChild) { return checkInvalidChildren(invalidChild); } From 1955540b46186ae49db8421c7ed618f8431e8c18 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 24 Feb 2021 17:23:09 -0800 Subject: [PATCH 12/14] Simplify constructor argument binding code by moving error binding closer to overload resolution, remove unneeded frees. --- .../Portable/Binder/Binder_Attributes.cs | 43 ++++++++----------- .../Collections/ArrayBuilderExtensions.cs | 4 +- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs index c4c8f46f6d1db..25688d11a2831 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs @@ -155,6 +155,7 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a MethodSymbol? attributeConstructor = null; // Bind attributeType's constructor based on the bound constructor arguments + ImmutableArray boundConstructorArguments; if (!attributeTypeForBinding.IsErrorType()) { attributeConstructor = BindAttributeConstructor(node, @@ -165,12 +166,18 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a suppressErrors: attributeType.IsErrorType(), ref argsToParamsOpt, ref expanded, - ref useSiteInfo); + ref useSiteInfo, + out boundConstructorArguments); } + else + { + boundConstructorArguments = analyzedArguments.ConstructorArguments.Arguments.SelectAsArrayInPlace( + (arg, attributeArgumentBinder) => attributeArgumentBinder.BindToTypeForErrorRecovery(arg), + attributeArgumentBinder); + } + Debug.Assert(boundConstructorArguments.All(a => !a.NeedsToBeConverted())); diagnostics.Add(node, useSiteInfo); - var constructorArguments = analyzedArguments.ConstructorArguments; - ImmutableArray boundConstructorArguments; if (attributeConstructor is object) { ReportDiagnosticsIfObsolete(diagnostics, attributeConstructor, node, hasBaseReceiver: false); @@ -179,31 +186,13 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a { Error(diagnostics, ErrorCode.ERR_AttributeCtorInParameter, node, attributeConstructor.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat)); } - - if (resultKind != LookupResultKind.Viable) - { - boundConstructorArguments = attributeArgumentBinder.BuildArgumentsForErrorRecovery(constructorArguments, ImmutableArray.Create(attributeConstructor)); - constructorArguments.Arguments.Free(); - } - else - { - boundConstructorArguments = constructorArguments.Arguments.ToImmutableAndFree(); - } - } - else - { - Debug.Assert(resultKind != LookupResultKind.Viable); - boundConstructorArguments = - constructorArguments.Arguments.SelectAsArrayInPlaceAndFree((arg, attributeArgumentBinder) => attributeArgumentBinder.BindToTypeForErrorRecovery(arg), attributeArgumentBinder); } - Debug.Assert(boundConstructorArguments.All(a => !a.NeedsToBeConverted())); - - ImmutableArray boundConstructorArgumentNamesOpt = constructorArguments.GetNames(); + ImmutableArray boundConstructorArgumentNamesOpt = analyzedArguments.ConstructorArguments.GetNames(); ImmutableArray boundNamedArguments = analyzedArguments.NamedArguments?.ToImmutableAndFree() ?? ImmutableArray.Empty; Debug.Assert(boundNamedArguments.All(arg => !arg.Right.NeedsToBeConverted())); - constructorArguments.Free(); + analyzedArguments.ConstructorArguments.Free(); return new BoundAttribute(node, attributeConstructor, boundConstructorArguments, boundConstructorArgumentNamesOpt, argsToParamsOpt, expanded, boundNamedArguments, resultKind, attributeType, hasErrors: resultKind != LookupResultKind.Viable); @@ -551,7 +540,8 @@ protected MethodSymbol BindAttributeConstructor( bool suppressErrors, ref ImmutableArray argsToParamsOpt, ref bool expanded, - ref CompoundUseSiteInfo useSiteInfo) + ref CompoundUseSiteInfo useSiteInfo, + out ImmutableArray constructorArguments) { MemberResolutionResult memberResolutionResult; ImmutableArray candidateConstructors; @@ -570,6 +560,11 @@ protected MethodSymbol BindAttributeConstructor( memberResolutionResult.IsValid && !IsConstructorAccessible(memberResolutionResult.Member, ref useSiteInfo) ? LookupResultKind.Inaccessible : LookupResultKind.OverloadResolutionFailure); + constructorArguments = BuildArgumentsForErrorRecovery(boundConstructorArguments, candidateConstructors); + } + else + { + constructorArguments = boundConstructorArguments.Arguments.ToImmutable(); } argsToParamsOpt = memberResolutionResult.Result.ArgsToParamsOpt; expanded = memberResolutionResult.Result.Kind == MemberResolutionKind.ApplicableInExpandedForm; diff --git a/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs b/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs index 1caae98fc9de4..ab7f31de1ac52 100644 --- a/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs +++ b/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs @@ -145,14 +145,14 @@ public static ImmutableArray SelectAsArray(this A /// The mapping delegate /// The extra input used by mapping delegate /// If the items's length is 0, this will return an empty immutable array. - public static ImmutableArray SelectAsArrayInPlaceAndFree(this ArrayBuilder items, Func map, TArg arg) + public static ImmutableArray SelectAsArrayInPlace(this ArrayBuilder items, Func map, TArg arg) { for (int i = 0; i < items.Count; i++) { items[i] = map(items[i], arg); } - return items.ToImmutableAndFree(); + return items.ToImmutable(); } public static void AddOptional(this ArrayBuilder builder, T? item) From 76e0ccd279e1efb376d41bcab8414794a30670d1 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 24 Feb 2021 17:25:04 -0800 Subject: [PATCH 13/14] Remove SelectAsArrayInPlace as it no longer provides benefits. --- .../Portable/Binder/Binder_Attributes.cs | 2 +- .../Collections/ArrayBuilderExtensions.cs | 18 ------------------ 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs index 25688d11a2831..4f55ecdd650bc 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs @@ -171,7 +171,7 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a } else { - boundConstructorArguments = analyzedArguments.ConstructorArguments.Arguments.SelectAsArrayInPlace( + boundConstructorArguments = analyzedArguments.ConstructorArguments.Arguments.SelectAsArray( (arg, attributeArgumentBinder) => attributeArgumentBinder.BindToTypeForErrorRecovery(arg), attributeArgumentBinder); } diff --git a/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs b/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs index ab7f31de1ac52..029c738ac7ddd 100644 --- a/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs +++ b/src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs @@ -137,24 +137,6 @@ public static ImmutableArray SelectAsArray(this A } } - /// - /// Maps an array builder to immutable array, applying the select function to the array builder in place, and freeing the underlying - /// array builder. - /// - /// The sequence to map - /// The mapping delegate - /// The extra input used by mapping delegate - /// If the items's length is 0, this will return an empty immutable array. - public static ImmutableArray SelectAsArrayInPlace(this ArrayBuilder items, Func map, TArg arg) - { - for (int i = 0; i < items.Count; i++) - { - items[i] = map(items[i], arg); - } - - return items.ToImmutable(); - } - public static void AddOptional(this ArrayBuilder builder, T? item) where T : class { From c9fa8203f130032423189273ddbc238b4b9ea72c Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 25 Feb 2021 13:48:29 -0800 Subject: [PATCH 14/14] Minor formatting and comment change. --- src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs index 4f55ecdd650bc..d1586fcb68699 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs @@ -172,7 +172,7 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a else { boundConstructorArguments = analyzedArguments.ConstructorArguments.Arguments.SelectAsArray( - (arg, attributeArgumentBinder) => attributeArgumentBinder.BindToTypeForErrorRecovery(arg), + static (arg, attributeArgumentBinder) => attributeArgumentBinder.BindToTypeForErrorRecovery(arg), attributeArgumentBinder); } Debug.Assert(boundConstructorArguments.All(a => !a.NeedsToBeConverted())); @@ -302,7 +302,7 @@ protected bool IsAttributeConditionallyOmitted(NamedTypeSymbol attributeType, Sy } /// - /// The result of this method captures some AnalyzedArguments, which must be free'ed by the caller. + /// The caller is responsible for freeing and . /// private AnalyzedAttributeArguments BindAttributeArguments( AttributeArgumentListSyntax? attributeArgumentList,