From 929ce8f23663dd23925205fcadaa98fecb8303e9 Mon Sep 17 00:00:00 2001 From: Charles Stoner Date: Sun, 5 Apr 2020 23:05:34 -0700 Subject: [PATCH 1/3] Reduce stack frame size of CheckConstraints --- .../OverloadResolution/OverloadResolution.cs | 1 - .../Portable/FlowAnalysis/NullableWalker.cs | 3 +- .../Portable/Symbols/ConstraintsHelper.cs | 132 ++++++++++-------- .../Symbols/ReducedExtensionMethodSymbol.cs | 2 +- .../Symbols/Tuples/TupleTypeSymbol.cs | 2 +- .../CSharp/Test/Emit/Emit/EndToEndTests.cs | 45 ++++++ 6 files changed, 122 insertions(+), 63 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index 01acb0243deb9..688b4aa3222b2 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -386,7 +386,6 @@ private bool FailsConstraintChecks(MethodSymbol method, out ArrayBuilder element.Locations.FirstOrDefault() ?? location, node.Syntax.Location); - tupleOpt.CheckConstraints(_conversions, includeNullability: true, typeSyntax: node.Syntax, locations, currentCompilation: compilation, diagnosticsOpt: null, nullabilityDiagnosticsOpt: Diagnostics); + tupleOpt.CheckConstraints(_conversions, typeSyntax: node.Syntax, locations, currentCompilation: compilation, diagnosticsOpt: null, nullabilityDiagnosticsOpt: Diagnostics); } SetResultType(node, TypeWithState.Create(tupleOpt, NullableFlowState.NotNull)); diff --git a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs index 7836d86ee0a17..ddcbd0d812edf 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs @@ -6,12 +6,9 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; -using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; -using Microsoft.CodeAnalysis.Collections; namespace Microsoft.CodeAnalysis.CSharp.Symbols { @@ -438,7 +435,6 @@ private static bool CheckConstraintsSingleType(TypeSymbol type, CheckConstraints public static void CheckConstraints( this NamedTypeSymbol tuple, ConversionsBase conversions, - bool includeNullability, SyntaxNode typeSyntax, ImmutableArray elementLocations, CSharpCompilation currentCompilation, @@ -465,7 +461,13 @@ public static void CheckConstraints( foreach (var underlyingTuple in underlyingTupleTypeChain) { ArrayBuilder useSiteDiagnosticsBuilder = null; - CheckTypeConstraints(underlyingTuple, conversions, includeNullability, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: nullabilityDiagnosticsBuilder, ref useSiteDiagnosticsBuilder); + CheckTypeConstraints( + underlyingTuple, + conversions, + currentCompilation, + diagnosticsBuilder, + nullabilityDiagnosticsBuilderOpt: (nullabilityDiagnosticsOpt is null) ? null : nullabilityDiagnosticsBuilder, + ref useSiteDiagnosticsBuilder); if (useSiteDiagnosticsBuilder != null) { @@ -497,7 +499,6 @@ void populateDiagnosticsAndClear(ArrayBuilder build builder.Clear(); } - } underlyingTupleTypeChain.Free(); @@ -524,7 +525,7 @@ public static bool CheckConstraintsForNamedType( var diagnosticsBuilder = ArrayBuilder.GetInstance(); ArrayBuilder useSiteDiagnosticsBuilder = null; - var result = !typeSyntax.HasErrors && CheckTypeConstraints(type, conversions, includeNullability, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: diagnosticsBuilder, ref useSiteDiagnosticsBuilder); + var result = !typeSyntax.HasErrors && CheckTypeConstraints(type, conversions, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: includeNullability ? diagnosticsBuilder : null, ref useSiteDiagnosticsBuilder); if (useSiteDiagnosticsBuilder != null) { @@ -566,7 +567,7 @@ public static bool CheckConstraints( var diagnosticsBuilder = ArrayBuilder.GetInstance(); ArrayBuilder useSiteDiagnosticsBuilder = null; - var result = CheckTypeConstraints(type, conversions, includeNullability, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: diagnosticsBuilder, ref useSiteDiagnosticsBuilder); + var result = CheckTypeConstraints(type, conversions, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: includeNullability ? diagnosticsBuilder : null, ref useSiteDiagnosticsBuilder); if (useSiteDiagnosticsBuilder != null) { @@ -654,7 +655,7 @@ public static bool CheckConstraints( var diagnosticsBuilder = ArrayBuilder.GetInstance(); ArrayBuilder useSiteDiagnosticsBuilder = null; - var result = CheckMethodConstraints(method, conversions, includeNullability: false, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: null, ref useSiteDiagnosticsBuilder); + var result = CheckMethodConstraints(method, conversions, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: null, ref useSiteDiagnosticsBuilder); if (useSiteDiagnosticsBuilder != null) { @@ -685,7 +686,7 @@ public static bool CheckConstraints( var diagnosticsBuilder = ArrayBuilder.GetInstance(); ArrayBuilder useSiteDiagnosticsBuilder = null; - var result = CheckMethodConstraints(method, conversions, includeNullability: false, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: null, ref useSiteDiagnosticsBuilder); + var result = CheckMethodConstraints(method, conversions, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: null, ref useSiteDiagnosticsBuilder); if (useSiteDiagnosticsBuilder != null) { @@ -701,10 +702,9 @@ public static bool CheckConstraints( return result; } - public static bool CheckTypeConstraints( + private static bool CheckTypeConstraints( NamedTypeSymbol type, ConversionsBase conversions, - bool includeNullability, CSharpCompilation currentCompilation, ArrayBuilder diagnosticsBuilder, ArrayBuilder nullabilityDiagnosticsBuilderOpt, @@ -713,7 +713,6 @@ public static bool CheckTypeConstraints( return CheckConstraints( type, conversions, - includeNullability, type.TypeSubstitution, type.OriginalDefinition.TypeParameters, type.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics, @@ -726,7 +725,6 @@ public static bool CheckTypeConstraints( public static bool CheckMethodConstraints( MethodSymbol method, ConversionsBase conversions, - bool includeNullability, CSharpCompilation currentCompilation, ArrayBuilder diagnosticsBuilder, ArrayBuilder nullabilityDiagnosticsBuilderOpt, @@ -736,7 +734,6 @@ public static bool CheckMethodConstraints( return CheckConstraints( method, conversions, - includeNullability, method.TypeSubstitution, ((MethodSymbol)method.OriginalDefinition).TypeParameters, method.TypeArgumentsWithAnnotations, @@ -766,7 +763,6 @@ public static bool CheckMethodConstraints( public static bool CheckConstraints( this Symbol containingSymbol, ConversionsBase conversions, - bool includeNullability, TypeMap substitution, ImmutableArray typeParameters, ImmutableArray typeArguments, @@ -779,7 +775,7 @@ public static bool CheckConstraints( { Debug.Assert(typeParameters.Length == typeArguments.Length); Debug.Assert(typeParameters.Length > 0); - Debug.Assert(!conversions.IncludeNullability || includeNullability); + Debug.Assert(!conversions.IncludeNullability || (nullabilityDiagnosticsBuilderOpt != null)); int n = typeParameters.Length; bool succeeded = true; @@ -794,7 +790,7 @@ public static bool CheckConstraints( var typeArgument = typeArguments[i]; var typeParameter = typeParameters[i]; - if (!CheckConstraints(containingSymbol, conversions, includeNullability, substitution, typeParameter, typeArgument, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt, ref useSiteDiagnosticsBuilder, + if (!CheckConstraints(containingSymbol, conversions, substitution, typeParameter, typeArgument, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt, ref useSiteDiagnosticsBuilder, ignoreTypeConstraintsDependentOnTypeParametersOpt)) { succeeded = false; @@ -808,7 +804,6 @@ public static bool CheckConstraints( private static bool CheckConstraints( Symbol containingSymbol, ConversionsBase conversions, - bool includeNullability, TypeMap substitution, TypeParameterSymbol typeParameter, TypeWithAnnotations typeArgument, @@ -842,12 +837,6 @@ private static bool CheckConstraints( return false; } - if (includeNullability && typeParameter.HasNotNullConstraint && typeArgument.GetValueNullableAnnotation().IsAnnotated() && !typeArgument.Type.IsNonNullableValueType()) - { - var diagnostic = new CSDiagnosticInfo(ErrorCode.WRN_NullabilityMismatchInTypeParameterNotNullConstraint, containingSymbol.ConstructedFrom(), typeParameter, typeArgument); - nullabilityDiagnosticsBuilderOpt.Add(new TypeParameterDiagnosticInfo(typeParameter, diagnostic)); - } - if (typeParameter.HasReferenceTypeConstraint) { if (!typeArgument.Type.IsReferenceType) @@ -856,16 +845,10 @@ private static bool CheckConstraints( diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new CSDiagnosticInfo(ErrorCode.ERR_RefConstraintNotSatisfied, containingSymbol.ConstructedFrom(), typeParameter, typeArgument.Type))); return false; } - - if (includeNullability && nullabilityDiagnosticsBuilderOpt != null && - typeParameter.ReferenceTypeConstraintIsNullable == false && - typeArgument.GetValueNullableAnnotation().IsAnnotated()) - { - var diagnostic = new CSDiagnosticInfo(ErrorCode.WRN_NullabilityMismatchInTypeParameterReferenceTypeConstraint, containingSymbol.ConstructedFrom(), typeParameter, typeArgument); - nullabilityDiagnosticsBuilderOpt.Add(new TypeParameterDiagnosticInfo(typeParameter, diagnostic)); - } } + checkNullability(containingSymbol, typeParameter, typeArgument, nullabilityDiagnosticsBuilderOpt); + if (typeParameter.HasUnmanagedTypeConstraint) { var managedKind = typeArgument.Type.ManagedKind; @@ -886,8 +869,7 @@ private static bool CheckConstraints( var csDiagnosticInfo = MessageID.IDS_FeatureUnmanagedConstructedTypes.GetFeatureAvailabilityDiagnosticInfo(currentCompilation); if (csDiagnosticInfo != null) { - var typeParameterDiagnosticInfo = new TypeParameterDiagnosticInfo(typeParameter, csDiagnosticInfo); - diagnosticsBuilder.Add(typeParameterDiagnosticInfo); + diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, csDiagnosticInfo)); return false; } } @@ -909,25 +891,76 @@ private static bool CheckConstraints( var constraintTypes = ArrayBuilder.GetInstance(); HashSet useSiteDiagnostics = null; substitution.SubstituteConstraintTypesDistinctWithoutModifiers(typeParameter, typeParameter.ConstraintTypesWithDefinitionUseSiteDiagnostics(ref useSiteDiagnostics), constraintTypes, - ignoreTypeConstraintsDependentOnTypeParametersOpt); - + ignoreTypeConstraintsDependentOnTypeParametersOpt); bool hasError = false; foreach (var constraintType in constraintTypes) + { + checkConstraintType(containingSymbol, conversions, typeParameter, typeArgument, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt, ref useSiteDiagnostics, constraintType, ref hasError); + } + constraintTypes.Free(); + + if (AppendUseSiteDiagnostics(useSiteDiagnostics, typeParameter, ref useSiteDiagnosticsBuilder)) + { + hasError = true; + } + + // Check the constructor constraint. + if (typeParameter.HasConstructorConstraint && !SatisfiesConstructorConstraint(typeArgument.Type)) + { + // "'{2}' must be a non-abstract type with a public parameterless constructor in order to use it as parameter '{1}' in the generic type or method '{0}'" + diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new CSDiagnosticInfo(ErrorCode.ERR_NewConstraintNotSatisfied, containingSymbol.ConstructedFrom(), typeParameter, typeArgument.Type))); + return false; + } + + return !hasError; + + static void checkNullability( + Symbol containingSymbol, + TypeParameterSymbol typeParameter, + TypeWithAnnotations typeArgument, + ArrayBuilder nullabilityDiagnosticsBuilderOpt) + { + if (nullabilityDiagnosticsBuilderOpt != null) + { + if (typeParameter.HasNotNullConstraint && typeArgument.GetValueNullableAnnotation().IsAnnotated() && !typeArgument.Type.IsNonNullableValueType()) + { + nullabilityDiagnosticsBuilderOpt.Add(new TypeParameterDiagnosticInfo(typeParameter, new CSDiagnosticInfo(ErrorCode.WRN_NullabilityMismatchInTypeParameterNotNullConstraint, containingSymbol.ConstructedFrom(), typeParameter, typeArgument))); + } + + if (typeParameter.HasReferenceTypeConstraint && + typeParameter.ReferenceTypeConstraintIsNullable == false && + typeArgument.GetValueNullableAnnotation().IsAnnotated()) + { + nullabilityDiagnosticsBuilderOpt.Add(new TypeParameterDiagnosticInfo(typeParameter, new CSDiagnosticInfo(ErrorCode.WRN_NullabilityMismatchInTypeParameterReferenceTypeConstraint, containingSymbol.ConstructedFrom(), typeParameter, typeArgument))); + } + } + } + + static void checkConstraintType( + Symbol containingSymbol, + ConversionsBase conversions, + TypeParameterSymbol typeParameter, + TypeWithAnnotations typeArgument, + CSharpCompilation currentCompilation, + ArrayBuilder diagnosticsBuilder, + ArrayBuilder nullabilityDiagnosticsBuilderOpt, + ref HashSet useSiteDiagnostics, + TypeWithAnnotations constraintType, + ref bool hasError) { if (SatisfiesConstraintType(conversions.WithNullability(false), typeArgument, constraintType, ref useSiteDiagnostics)) { - if (includeNullability && nullabilityDiagnosticsBuilderOpt != null) + if (nullabilityDiagnosticsBuilderOpt != null) { if (!SatisfiesConstraintType(conversions.WithNullability(true), typeArgument, constraintType, ref useSiteDiagnostics) || (typeArgument.GetValueNullableAnnotation().IsAnnotated() && !typeArgument.Type.IsNonNullableValueType() && TypeParameterSymbol.IsNotNullableFromConstraintType(constraintType, out _) == true)) { - var diagnostic = new CSDiagnosticInfo(ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint, containingSymbol.ConstructedFrom(), constraintType, typeParameter, typeArgument); - nullabilityDiagnosticsBuilderOpt.Add(new TypeParameterDiagnosticInfo(typeParameter, diagnostic)); + nullabilityDiagnosticsBuilderOpt.Add(new TypeParameterDiagnosticInfo(typeParameter, new CSDiagnosticInfo(ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint, containingSymbol.ConstructedFrom(), constraintType, typeParameter, typeArgument))); } } - continue; + return; } ErrorCode errorCode; @@ -952,23 +985,6 @@ private static bool CheckConstraints( diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new CSDiagnosticInfo(errorCode, containingSymbol.ConstructedFrom(), distinguisher.First, typeParameter, distinguisher.Second))); hasError = true; } - - if (AppendUseSiteDiagnostics(useSiteDiagnostics, typeParameter, ref useSiteDiagnosticsBuilder)) - { - hasError = true; - } - - constraintTypes.Free(); - - // Check the constructor constraint. - if (typeParameter.HasConstructorConstraint && !SatisfiesConstructorConstraint(typeArgument.Type)) - { - // "'{2}' must be a non-abstract type with a public parameterless constructor in order to use it as parameter '{1}' in the generic type or method '{0}'" - diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new CSDiagnosticInfo(ErrorCode.ERR_NewConstraintNotSatisfied, containingSymbol.ConstructedFrom(), typeParameter, typeArgument.Type))); - return false; - } - - return !hasError; } private static bool AppendUseSiteDiagnostics( diff --git a/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs index e03fff8b7f42c..26cda8c205dd2 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs @@ -198,7 +198,7 @@ private static MethodSymbol InferExtensionMethodTypeArguments(MethodSymbol metho var diagnosticsBuilder = ArrayBuilder.GetInstance(); var substitution = new TypeMap(typeParams, typeArgsForConstraintsCheck); ArrayBuilder useSiteDiagnosticsBuilder = null; - var success = method.CheckConstraints(conversions, includeNullability: false, substitution, typeParams, typeArgsForConstraintsCheck, compilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: null, ref useSiteDiagnosticsBuilder, + var success = method.CheckConstraints(conversions, substitution, typeParams, typeArgsForConstraintsCheck, compilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: null, ref useSiteDiagnosticsBuilder, ignoreTypeConstraintsDependentOnTypeParametersOpt: notInferredTypeParameters.Count > 0 ? notInferredTypeParameters : null); diagnosticsBuilder.Free(); notInferredTypeParameters.Free(); diff --git a/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs index 3893ead42ee0c..0b5fabab566a3 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs @@ -67,7 +67,7 @@ internal static NamedTypeSymbol CreateTuple( var constructedType = CreateTuple(underlyingType, elementNames, errorPositions, elementLocations, locations); if (shouldCheckConstraints && diagnostics != null) { - constructedType.CheckConstraints(compilation.Conversions, includeNullability, syntax, elementLocations, compilation, diagnostics, diagnostics); + constructedType.CheckConstraints(compilation.Conversions, syntax, elementLocations, compilation, diagnosticsOpt: diagnostics, nullabilityDiagnosticsOpt: includeNullability ? diagnostics : null); } return constructedType; diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs index 210c629ff67a9..bb7f8c3fbb9b6 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs @@ -8,6 +8,8 @@ using Xunit; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; +using Microsoft.CodeAnalysis.CSharp.Symbols; +using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Emit { @@ -267,5 +269,48 @@ static void Main() }); } } + + [WorkItem(42361, "https://github.com/dotnet/roslyn/issues/42361")] + [ConditionalFact(typeof(WindowsOnly))] + public void Constraints() + { + int n = (ExecutionConditionUtil.Architecture, ExecutionConditionUtil.Configuration) switch + { + (ExecutionArchitecture.x86, ExecutionConfiguration.Debug) => 420, + (ExecutionArchitecture.x86, ExecutionConfiguration.Release) => 1080, + (ExecutionArchitecture.x64, ExecutionConfiguration.Debug) => 280, + (ExecutionArchitecture.x64, ExecutionConfiguration.Release) => 760, + _ => throw new Exception($"Unexpected configuration {ExecutionConditionUtil.Architecture} {ExecutionConditionUtil.Configuration}") + }; + + RunTest(n, runTest); + + static void runTest(int n) + { + // class C0 where T : C1 { } + // class C1 where T : C2 { } + // ... + // class CN where T : C0 { } + var sourceBuilder = new StringBuilder(); + var diagnosticsBuilder = ArrayBuilder.GetInstance(); + for (int i = 0; i <= n; i++) + { + int next = (i == n) ? 0 : i + 1; + sourceBuilder.AppendLine($"class C{i} where T : C{next} {{ }}"); + diagnosticsBuilder.Add(Diagnostic(ErrorCode.ERR_GenericConstraintNotSatisfiedRefType, "T").WithArguments($"C{i}", $"C{next}", "T", "T")); + } + var source = sourceBuilder.ToString(); + var diagnostics = diagnosticsBuilder.ToArrayAndFree(); + + RunInThread(() => + { + var comp = CreateCompilation(source); + var type = comp.GetMember("C0"); + var typeParameter = type.TypeParameters[0]; + Assert.True(typeParameter.IsReferenceType); + comp.VerifyDiagnostics(diagnostics); + }); + } + } } } From a4ab2270ee6bf899953c62fcdd1c30be251b4f60 Mon Sep 17 00:00:00 2001 From: Charles Stoner Date: Mon, 6 Apr 2020 15:11:18 -0700 Subject: [PATCH 2/3] Bundle arguments in CheckConstraintsArgs --- .../CSharp/Portable/Binder/Binder_Symbols.cs | 2 +- .../OverloadResolutionResult.cs | 2 +- .../Portable/Symbols/ConstraintsHelper.cs | 77 +++++++++---------- .../Source/SourceTypeParameterSymbol.cs | 9 +-- .../CSharp/Test/Emit/Emit/EndToEndTests.cs | 6 +- 5 files changed, 43 insertions(+), 53 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs index dccb6fa09cd11..6c55dbbd000a7 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs @@ -495,7 +495,7 @@ NamespaceOrTypeOrAliasSymbolWithAnnotations bindNullable() ReportUseSiteDiagnostics(constructedType.Type.OriginalDefinition, diagnostics, syntax); var type = (NamedTypeSymbol)constructedType.Type; var location = syntax.Location; - type.CheckConstraints(this.Compilation, this.Conversions, includeNullability: true, location, diagnostics); + type.CheckConstraints(new CheckConstraintsArgs(this.Compilation, this.Conversions, includeNullability: true, location, diagnostics)); } else if (constructedType.Type.IsTypeParameterDisallowingAnnotation()) { diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs index 70fa131b36040..62f2659b3ded6 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs @@ -981,7 +981,7 @@ private bool HadConstructedParameterFailedConstraintCheck( // formal parameter type. TypeSymbol formalParameterType = method.GetParameterType(result.Result.BadParameter); - formalParameterType.CheckAllConstraints((CSharpCompilation)compilation, conversions, includeNullability: false, location, diagnostics); + formalParameterType.CheckAllConstraints(new CheckConstraintsArgs((CSharpCompilation)compilation, conversions, includeNullability: false, location, diagnostics)); return true; } diff --git a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs index ddcbd0d812edf..9283ab8eedbd9 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs @@ -30,6 +30,29 @@ public TypeParameterDiagnosticInfo(TypeParameterSymbol typeParameter, Diagnostic } } + internal readonly struct CheckConstraintsArgs + { + public readonly CSharpCompilation CurrentCompilation; + public readonly ConversionsBase Conversions; + public readonly bool IncludeNullability; + public readonly Location Location; + public readonly DiagnosticBag Diagnostics; + + public CheckConstraintsArgs(CSharpCompilation currentCompilation, ConversionsBase conversions, Location location, DiagnosticBag diagnostics) : + this(currentCompilation, conversions, currentCompilation.IsFeatureEnabled(MessageID.IDS_FeatureNullableReferenceTypes), location, diagnostics) + { + } + + public CheckConstraintsArgs(CSharpCompilation currentCompilation, ConversionsBase conversions, bool includeNullability, Location location, DiagnosticBag diagnostics) + { + this.CurrentCompilation = currentCompilation; + this.Conversions = conversions; + this.IncludeNullability = includeNullability; + this.Location = location; + this.Diagnostics = diagnostics; + } + } + /// /// Helper methods for generic type parameter constraints. There are two sets of methods: one /// set for resolving constraint "bounds" (that is, determining the effective base type, interface set, @@ -370,7 +393,7 @@ public static void CheckAllConstraints( DiagnosticBag diagnostics) { bool includeNullability = compilation.IsFeatureEnabled(MessageID.IDS_FeatureNullableReferenceTypes); - CheckAllConstraints(type, compilation, conversions, includeNullability, location, diagnostics); + type.CheckAllConstraints(new CheckConstraintsArgs(compilation, conversions, includeNullability, location, diagnostics)); } public static bool CheckAllConstraints( @@ -382,48 +405,24 @@ public static bool CheckAllConstraints( // Nullability checks can only add warnings here so skip them for this check as we are only // concerned with errors. - CheckAllConstraints(type, compilation, conversions, includeNullability: false, NoLocation.Singleton, diagnostics); + type.CheckAllConstraints(new CheckConstraintsArgs(compilation, conversions, includeNullability: false, NoLocation.Singleton, diagnostics)); bool ok = !diagnostics.HasAnyErrors(); diagnostics.Free(); return ok; } - public static void CheckAllConstraints( - this TypeSymbol type, - CSharpCompilation compilation, - ConversionsBase conversions, - bool includeNullability, - Location location, - DiagnosticBag diagnostics) + public static void CheckAllConstraints(this TypeSymbol type, CheckConstraintsArgs args) { - type.VisitType(s_checkConstraintsSingleTypeFunc, new CheckConstraintsArgs(compilation, conversions, includeNullability, location, diagnostics)); - } - - private readonly struct CheckConstraintsArgs - { - public readonly CSharpCompilation CurrentCompilation; - public readonly ConversionsBase Conversions; - public readonly bool IncludeNullability; - public readonly Location Location; - public readonly DiagnosticBag Diagnostics; - - public CheckConstraintsArgs(CSharpCompilation currentCompilation, ConversionsBase conversions, bool includeNullability, Location location, DiagnosticBag diagnostics) - { - this.CurrentCompilation = currentCompilation; - this.Conversions = conversions; - this.IncludeNullability = includeNullability; - this.Location = location; - this.Diagnostics = diagnostics; - } + type.VisitType(s_checkConstraintsSingleTypeFunc, args); } private static readonly Func s_checkConstraintsSingleTypeFunc = (type, arg, unused) => CheckConstraintsSingleType(type, arg); - private static bool CheckConstraintsSingleType(TypeSymbol type, CheckConstraintsArgs args) + private static bool CheckConstraintsSingleType(TypeSymbol type, in CheckConstraintsArgs args) { if (type.Kind == SymbolKind.NamedType) { - ((NamedTypeSymbol)type).CheckConstraints(args.CurrentCompilation, args.Conversions, args.IncludeNullability, args.Location, args.Diagnostics); + ((NamedTypeSymbol)type).CheckConstraints(args); } else if (type.Kind == SymbolKind.PointerType) { @@ -550,15 +549,9 @@ public static bool CheckConstraintsForNamedType( return result; } - public static bool CheckConstraints( - this NamedTypeSymbol type, - CSharpCompilation currentCompilation, - ConversionsBase conversions, - bool includeNullability, - Location location, - DiagnosticBag diagnostics) + public static bool CheckConstraints(this NamedTypeSymbol type, in CheckConstraintsArgs args) { - Debug.Assert(currentCompilation is object); + Debug.Assert(args.CurrentCompilation is object); if (!RequiresChecking(type)) { @@ -567,7 +560,7 @@ public static bool CheckConstraints( var diagnosticsBuilder = ArrayBuilder.GetInstance(); ArrayBuilder useSiteDiagnosticsBuilder = null; - var result = CheckTypeConstraints(type, conversions, currentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: includeNullability ? diagnosticsBuilder : null, ref useSiteDiagnosticsBuilder); + var result = CheckTypeConstraints(type, args.Conversions, args.CurrentCompilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: args.IncludeNullability ? diagnosticsBuilder : null, ref useSiteDiagnosticsBuilder); if (useSiteDiagnosticsBuilder != null) { @@ -576,7 +569,7 @@ public static bool CheckConstraints( foreach (var pair in diagnosticsBuilder) { - diagnostics.Add(new CSDiagnostic(pair.DiagnosticInfo, location)); + args.Diagnostics.Add(new CSDiagnostic(pair.DiagnosticInfo, args.Location)); } diagnosticsBuilder.Free(); @@ -584,10 +577,10 @@ public static bool CheckConstraints( // we only check for distinct interfaces when the type is not from source, as we // trust that types that are from source have already been checked by the compiler // to prevent this from happening in the first place. - if (!(currentCompilation != null && type.IsFromCompilation(currentCompilation)) && HasDuplicateInterfaces(type, null)) + if (!(args.CurrentCompilation != null && type.IsFromCompilation(args.CurrentCompilation)) && HasDuplicateInterfaces(type, null)) { result = false; - diagnostics.Add(ErrorCode.ERR_BogusType, location, type); + args.Diagnostics.Add(ErrorCode.ERR_BogusType, args.Location, type); } return result; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs index 2ddc0d1ef179d..9f9d06c4c8c42 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs @@ -256,18 +256,15 @@ private void CheckConstraintTypeConstraints(DiagnosticBag diagnostics) return; } - var corLibrary = this.ContainingAssembly.CorLibrary; - var conversions = new TypeConversions(corLibrary); - var location = _locations[0]; - + var args = new CheckConstraintsArgs(DeclaringCompilation, new TypeConversions(ContainingAssembly.CorLibrary), _locations[0], diagnostics); foreach (var constraintType in constraintTypes) { HashSet useSiteDiagnostics = null; constraintType.Type.AddUseSiteDiagnostics(ref useSiteDiagnostics); - if (!diagnostics.Add(location, useSiteDiagnostics)) + if (!diagnostics.Add(args.Location, useSiteDiagnostics)) { - constraintType.Type.CheckAllConstraints(DeclaringCompilation, conversions, location, diagnostics); + constraintType.Type.CheckAllConstraints(args); } } } diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs index bb7f8c3fbb9b6..fdc74456af56c 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs @@ -277,9 +277,9 @@ public void Constraints() int n = (ExecutionConditionUtil.Architecture, ExecutionConditionUtil.Configuration) switch { (ExecutionArchitecture.x86, ExecutionConfiguration.Debug) => 420, - (ExecutionArchitecture.x86, ExecutionConfiguration.Release) => 1080, - (ExecutionArchitecture.x64, ExecutionConfiguration.Debug) => 280, - (ExecutionArchitecture.x64, ExecutionConfiguration.Release) => 760, + (ExecutionArchitecture.x86, ExecutionConfiguration.Release) => 1100, + (ExecutionArchitecture.x64, ExecutionConfiguration.Debug) => 200, + (ExecutionArchitecture.x64, ExecutionConfiguration.Release) => 520, _ => throw new Exception($"Unexpected configuration {ExecutionConditionUtil.Architecture} {ExecutionConditionUtil.Configuration}") }; From 6dc4bff625de817392338c06680c2e4666c92734 Mon Sep 17 00:00:00 2001 From: Charles Stoner Date: Tue, 7 Apr 2020 14:05:58 -0700 Subject: [PATCH 3/3] PR feedback --- .../CSharp/Portable/Binder/Binder_Symbols.cs | 2 +- .../OverloadResolutionResult.cs | 2 +- .../Portable/Symbols/ConstraintsHelper.cs | 46 +++++++++---------- .../Source/SourceTypeParameterSymbol.cs | 2 +- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs index 6c55dbbd000a7..dc4f6da4d5032 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs @@ -495,7 +495,7 @@ NamespaceOrTypeOrAliasSymbolWithAnnotations bindNullable() ReportUseSiteDiagnostics(constructedType.Type.OriginalDefinition, diagnostics, syntax); var type = (NamedTypeSymbol)constructedType.Type; var location = syntax.Location; - type.CheckConstraints(new CheckConstraintsArgs(this.Compilation, this.Conversions, includeNullability: true, location, diagnostics)); + type.CheckConstraints(new ConstraintsHelper.CheckConstraintsArgs(this.Compilation, this.Conversions, includeNullability: true, location, diagnostics)); } else if (constructedType.Type.IsTypeParameterDisallowingAnnotation()) { diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs index 62f2659b3ded6..baf816abad830 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs @@ -981,7 +981,7 @@ private bool HadConstructedParameterFailedConstraintCheck( // formal parameter type. TypeSymbol formalParameterType = method.GetParameterType(result.Result.BadParameter); - formalParameterType.CheckAllConstraints(new CheckConstraintsArgs((CSharpCompilation)compilation, conversions, includeNullability: false, location, diagnostics)); + formalParameterType.CheckAllConstraints(new ConstraintsHelper.CheckConstraintsArgs((CSharpCompilation)compilation, conversions, includeNullability: false, location, diagnostics)); return true; } diff --git a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs index 9283ab8eedbd9..0f10b4e93ec49 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs @@ -30,29 +30,6 @@ public TypeParameterDiagnosticInfo(TypeParameterSymbol typeParameter, Diagnostic } } - internal readonly struct CheckConstraintsArgs - { - public readonly CSharpCompilation CurrentCompilation; - public readonly ConversionsBase Conversions; - public readonly bool IncludeNullability; - public readonly Location Location; - public readonly DiagnosticBag Diagnostics; - - public CheckConstraintsArgs(CSharpCompilation currentCompilation, ConversionsBase conversions, Location location, DiagnosticBag diagnostics) : - this(currentCompilation, conversions, currentCompilation.IsFeatureEnabled(MessageID.IDS_FeatureNullableReferenceTypes), location, diagnostics) - { - } - - public CheckConstraintsArgs(CSharpCompilation currentCompilation, ConversionsBase conversions, bool includeNullability, Location location, DiagnosticBag diagnostics) - { - this.CurrentCompilation = currentCompilation; - this.Conversions = conversions; - this.IncludeNullability = includeNullability; - this.Location = location; - this.Diagnostics = diagnostics; - } - } - /// /// Helper methods for generic type parameter constraints. There are two sets of methods: one /// set for resolving constraint "bounds" (that is, determining the effective base type, interface set, @@ -416,6 +393,29 @@ public static void CheckAllConstraints(this TypeSymbol type, CheckConstraintsArg type.VisitType(s_checkConstraintsSingleTypeFunc, args); } + internal readonly struct CheckConstraintsArgs + { + public readonly CSharpCompilation CurrentCompilation; + public readonly ConversionsBase Conversions; + public readonly bool IncludeNullability; + public readonly Location Location; + public readonly DiagnosticBag Diagnostics; + + public CheckConstraintsArgs(CSharpCompilation currentCompilation, ConversionsBase conversions, Location location, DiagnosticBag diagnostics) : + this(currentCompilation, conversions, currentCompilation.IsFeatureEnabled(MessageID.IDS_FeatureNullableReferenceTypes), location, diagnostics) + { + } + + public CheckConstraintsArgs(CSharpCompilation currentCompilation, ConversionsBase conversions, bool includeNullability, Location location, DiagnosticBag diagnostics) + { + this.CurrentCompilation = currentCompilation; + this.Conversions = conversions; + this.IncludeNullability = includeNullability; + this.Location = location; + this.Diagnostics = diagnostics; + } + } + private static readonly Func s_checkConstraintsSingleTypeFunc = (type, arg, unused) => CheckConstraintsSingleType(type, arg); private static bool CheckConstraintsSingleType(TypeSymbol type, in CheckConstraintsArgs args) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs index 9f9d06c4c8c42..3448387711ec4 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs @@ -256,7 +256,7 @@ private void CheckConstraintTypeConstraints(DiagnosticBag diagnostics) return; } - var args = new CheckConstraintsArgs(DeclaringCompilation, new TypeConversions(ContainingAssembly.CorLibrary), _locations[0], diagnostics); + var args = new ConstraintsHelper.CheckConstraintsArgs(DeclaringCompilation, new TypeConversions(ContainingAssembly.CorLibrary), _locations[0], diagnostics); foreach (var constraintType in constraintTypes) { HashSet useSiteDiagnostics = null;