Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid cycles while binding type parameter constraints #49261

Merged
2 commits merged into from
Nov 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,6 @@ internal bool AreNullableAnnotationsEnabled(SyntaxTree syntaxTree, int position)
internal bool AreNullableAnnotationsEnabled(SyntaxToken token)
{
RoslynDebug.Assert(token.SyntaxTree is object);
if ((Flags & BinderFlags.IgnoreNullableContext) != 0)
{
return false;
}
return AreNullableAnnotationsEnabled(token.SyntaxTree, token.SpanStart);
}

Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/BinderFlags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ internal enum BinderFlags : uint
InEEMethodBinder = 1 << 30,

/// <summary>
/// Assume '#nullable disable' context.
/// Skip binding type arguments (we use <see cref="Symbols.PlaceholderTypeArgumentSymbol"/> instead).
/// For example, currently used when type constraints are bound in some scenarios.
/// </summary>
IgnoreNullableContext = 1u << 31,
SuppressTypeArgumentBinding = 1u << 31,

// Groups

Expand Down
49 changes: 29 additions & 20 deletions src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
ImmutableArray<TypeParameterSymbol> typeParameters,
TypeParameterListSyntax typeParameterList,
SyntaxList<TypeParameterConstraintClauseSyntax> clauses,
bool canIgnoreNullableContext,
ref IReadOnlyDictionary<TypeParameterSymbol, bool> isValueTypeOverride,
DiagnosticBag diagnostics,
bool performOnlyCycleSafeValidation,
bool isForOverride = false)
{
Debug.Assert(this.Flags.Includes(BinderFlags.GenericConstraintsClause));
Expand Down Expand Up @@ -66,8 +65,7 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
Debug.Assert(ordinal >= 0);
Debug.Assert(ordinal < n);

(TypeParameterConstraintClause constraintClause, ArrayBuilder<TypeConstraintSyntax>? typeConstraintNodes) =
this.BindTypeParameterConstraints(typeParameterList.Parameters[ordinal], clause, isForOverride, canIgnoreNullableContext: canIgnoreNullableContext, diagnostics);
(TypeParameterConstraintClause constraintClause, ArrayBuilder<TypeConstraintSyntax>? typeConstraintNodes) = this.BindTypeParameterConstraints(typeParameterList.Parameters[ordinal], clause, isForOverride, diagnostics);
if (results[ordinal] == null)
{
results[ordinal] = constraintClause;
Expand Down Expand Up @@ -101,9 +99,7 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
}
}

TypeParameterConstraintClause.AdjustConstraintTypes(containingSymbol, typeParameters, results, ref isValueTypeOverride);

RemoveInvalidConstraints(typeParameters, results!, syntaxNodes, diagnostics);
RemoveInvalidConstraints(typeParameters, results!, syntaxNodes, performOnlyCycleSafeValidation, diagnostics);

foreach (var typeConstraintsSyntaxes in syntaxNodes)
{
Expand All @@ -118,8 +114,7 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
/// <summary>
/// Bind and return a single type parameter constraint clause along with syntax nodes corresponding to type constraints.
/// </summary>
private (TypeParameterConstraintClause, ArrayBuilder<TypeConstraintSyntax>?) BindTypeParameterConstraints(
TypeParameterSyntax typeParameterSyntax, TypeParameterConstraintClauseSyntax constraintClauseSyntax, bool isForOverride, bool canIgnoreNullableContext, DiagnosticBag diagnostics)
private (TypeParameterConstraintClause, ArrayBuilder<TypeConstraintSyntax>?) BindTypeParameterConstraints(TypeParameterSyntax typeParameterSyntax, TypeParameterConstraintClauseSyntax constraintClauseSyntax, bool isForOverride, DiagnosticBag diagnostics)
{
var constraints = TypeParameterConstraintKind.None;
ArrayBuilder<TypeWithAnnotations>? constraintTypes = null;
Expand Down Expand Up @@ -309,7 +304,7 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
Debug.Assert(!isForOverride ||
(constraints & (TypeParameterConstraintKind.ReferenceType | TypeParameterConstraintKind.ValueType)) != (TypeParameterConstraintKind.ReferenceType | TypeParameterConstraintKind.ValueType));

return (TypeParameterConstraintClause.Create(constraints, constraintTypes?.ToImmutableAndFree() ?? ImmutableArray<TypeWithAnnotations>.Empty, canIgnoreNullableContext), syntaxBuilder);
return (TypeParameterConstraintClause.Create(constraints, constraintTypes?.ToImmutableAndFree() ?? ImmutableArray<TypeWithAnnotations>.Empty), syntaxBuilder);

static void reportOverrideWithConstraints(ref bool reportedOverrideWithConstraints, TypeParameterConstraintSyntax syntax, DiagnosticBag diagnostics)
{
Expand Down Expand Up @@ -350,21 +345,23 @@ private static void RemoveInvalidConstraints(
ImmutableArray<TypeParameterSymbol> typeParameters,
ArrayBuilder<TypeParameterConstraintClause> constraintClauses,
ArrayBuilder<ArrayBuilder<TypeConstraintSyntax>?> syntaxNodes,
bool performOnlyCycleSafeValidation,
DiagnosticBag diagnostics)
{
Debug.Assert(typeParameters.Length > 0);
Debug.Assert(typeParameters.Length == constraintClauses.Count);
int n = typeParameters.Length;
for (int i = 0; i < n; i++)
{
constraintClauses[i] = RemoveInvalidConstraints(typeParameters[i], constraintClauses[i], syntaxNodes[i], diagnostics);
constraintClauses[i] = RemoveInvalidConstraints(typeParameters[i], constraintClauses[i], syntaxNodes[i], performOnlyCycleSafeValidation, diagnostics);
}
}

private static TypeParameterConstraintClause RemoveInvalidConstraints(
TypeParameterSymbol typeParameter,
TypeParameterConstraintClause constraintClause,
ArrayBuilder<TypeConstraintSyntax>? syntaxNodesOpt,
bool performOnlyCycleSafeValidation,
DiagnosticBag diagnostics)
{
if (syntaxNodesOpt != null)
Expand All @@ -383,16 +380,20 @@ private static TypeParameterConstraintClause RemoveInvalidConstraints(
// since, in general, it may be difficult to support all invalid types.
// In the future, we may want to include some invalid types
// though so the public binding API has the most information.
if (IsValidConstraint(typeParameter.Name, syntax, constraintType, constraintClause.Constraints, constraintTypeBuilder, diagnostics))
if (IsValidConstraint(typeParameter, syntax, constraintType, constraintClause.Constraints, constraintTypeBuilder, performOnlyCycleSafeValidation, diagnostics))
{
CheckConstraintTypeVisibility(containingSymbol, syntax.Location, constraintType, diagnostics);
if (!performOnlyCycleSafeValidation)
{
CheckConstraintTypeVisibility(containingSymbol, syntax.Location, constraintType, diagnostics);
}

constraintTypeBuilder.Add(constraintType);
}
}

if (constraintTypeBuilder.Count < n)
{
return TypeParameterConstraintClause.Create(constraintClause.Constraints, constraintTypeBuilder.ToImmutableAndFree(), constraintClause.IgnoresNullableContext);
return TypeParameterConstraintClause.Create(constraintClause.Constraints, constraintTypeBuilder.ToImmutableAndFree());
}

constraintTypeBuilder.Free();
Expand Down Expand Up @@ -421,26 +422,28 @@ private static void CheckConstraintTypeVisibility(
/// returns false and generates a diagnostic.
/// </summary>
private static bool IsValidConstraint(
string typeParameterName,
TypeParameterSymbol typeParameter,
TypeConstraintSyntax syntax,
TypeWithAnnotations type,
TypeParameterConstraintKind constraints,
ArrayBuilder<TypeWithAnnotations> constraintTypes,
bool performOnlyCycleSafeValidation,
DiagnosticBag diagnostics)
{
if (!isValidConstraintType(syntax, type, diagnostics))
if (!isValidConstraintType(typeParameter, syntax, type, performOnlyCycleSafeValidation, diagnostics))
{
return false;
}

if (constraintTypes.Contains(c => type.Equals(c, TypeCompareKind.AllIgnoreOptions)))
if (!performOnlyCycleSafeValidation && constraintTypes.Contains(c => type.Equals(c, TypeCompareKind.AllIgnoreOptions)))
{
// "Duplicate constraint '{0}' for type parameter '{1}'"
Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type.Type.SetUnknownNullabilityForReferenceTypes(), typeParameterName);
Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type.Type.SetUnknownNullabilityForReferenceTypes(), typeParameter.Name);
return false;
}

if (type.TypeKind == TypeKind.Class)
if (!type.DefaultType.IsTypeParameter() && // Doing an explicit check for type parameter on unresolved type to avoid cycles while calculating TypeKind. An unresolved type parameter cannot resolve to a class.
type.TypeKind == TypeKind.Class)
{
// If there is already a struct or class constraint (class constraint could be
// 'class' or explicit type), report an error and drop this class. If we don't
Expand Down Expand Up @@ -489,8 +492,14 @@ private static bool IsValidConstraint(

// Returns true if the type is a valid constraint type.
// Otherwise returns false and generates a diagnostic.
static bool isValidConstraintType(TypeConstraintSyntax syntax, TypeWithAnnotations typeWithAnnotations, DiagnosticBag diagnostics)
static bool isValidConstraintType(TypeParameterSymbol typeParameter, TypeConstraintSyntax syntax, TypeWithAnnotations typeWithAnnotations, bool performOnlyCycleSafeValidation, DiagnosticBag diagnostics)
{
if (typeWithAnnotations.NullableAnnotation == NullableAnnotation.Annotated && performOnlyCycleSafeValidation &&
typeWithAnnotations.DefaultType is TypeParameterSymbol typeParameterInConstraint && typeParameterInConstraint.ContainingSymbol == (object)typeParameter.ContainingSymbol)
{
return true;
}

TypeSymbol type = typeWithAnnotations.Type;

switch (type.SpecialType)
Expand Down
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,10 @@ private TypeWithAnnotations BindGenericSimpleNamespaceOrTypeOrAliasSymbol(
resultType = unconstructedType.AsUnboundGenericType();
}
}
else if ((Flags & BinderFlags.SuppressTypeArgumentBinding) != 0)
{
resultType = unconstructedType.Construct(PlaceholderTypeArgumentSymbol.CreateTypeArguments(unconstructedType.TypeParameters));
}
else
{
// It's not an unbound type expression, so we must have type arguments, and we have a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ private static bool IsNamedTypeAccessible(NamedTypeSymbol type, Symbol within, r
{
// type parameters are always accessible, so don't check those (so common it's
// worth optimizing this).
if (typeArg.DefaultType.TypeKind != TypeKind.TypeParameter && !IsSymbolAccessibleCore(typeArg.Type, within, null, out unused, compilation, ref useSiteDiagnostics, basesBeingResolved))
if (typeArg.Type.Kind != SymbolKind.TypeParameter && !IsSymbolAccessibleCore(typeArg.Type, within, null, out unused, compilation, ref useSiteDiagnostics, basesBeingResolved))
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,11 @@ public sealed override ImmutableArray<TypeParameterSymbol> TypeParameters
get { return _typeParameters; }
}

public sealed override ImmutableArray<TypeParameterConstraintClause> GetTypeParameterConstraintClauses(bool canIgnoreNullableContext)
=> ImmutableArray<TypeParameterConstraintClause>.Empty;
public sealed override ImmutableArray<ImmutableArray<TypeWithAnnotations>> GetTypeParameterConstraintTypes()
=> ImmutableArray<ImmutableArray<TypeWithAnnotations>>.Empty;

public sealed override ImmutableArray<TypeParameterConstraintKind> GetTypeParameterConstraintKinds()
=> ImmutableArray<TypeParameterConstraintKind>.Empty;

internal override int ParameterCount
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,8 @@ internal override ImmutableArray<NamedTypeSymbol> GetDeclaredInterfaces(ConsList

internal override bool IsRecord => false;

internal override bool Equals(TypeSymbol t2, TypeCompareKind comparison, IReadOnlyDictionary<TypeParameterSymbol, bool> isValueTypeOverrideOpt = null)
internal override bool Equals(TypeSymbol t2, TypeCompareKind comparison)
{
Debug.Assert(isValueTypeOverrideOpt == null);

if (ReferenceEquals(this, t2))
{
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ public override bool HasReferenceTypeConstraint
get { return false; }
}

public override bool IsReferenceTypeFromConstraintTypes
{
get { return false; }
}

internal override bool? ReferenceTypeConstraintIsNullable
{
get { return false; }
Expand All @@ -86,6 +91,11 @@ public override bool HasValueTypeConstraint
get { return false; }
}

public override bool IsValueTypeFromConstraintTypes
{
get { return false; }
}

public override bool HasUnmanagedTypeConstraint
{
get { return false; }
Expand All @@ -101,11 +111,11 @@ public override VarianceKind Variance
get { return VarianceKind.None; }
}

internal override void EnsureAllConstraintsAreResolved(bool canIgnoreNullableContext)
internal override void EnsureAllConstraintsAreResolved()
{
}

internal override ImmutableArray<TypeWithAnnotations> GetConstraintTypes(ConsList<TypeParameterSymbol> inProgress, bool canIgnoreNullableContext)
internal override ImmutableArray<TypeWithAnnotations> GetConstraintTypes(ConsList<TypeParameterSymbol> inProgress)
{
return ImmutableArray<TypeWithAnnotations>.Empty;
}
Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -337,20 +337,20 @@ public override TResult Accept<TResult>(CSharpSymbolVisitor<TResult> visitor)
return visitor.VisitArrayType(this);
}

internal override bool Equals(TypeSymbol? t2, TypeCompareKind comparison, IReadOnlyDictionary<TypeParameterSymbol, bool>? isValueTypeOverrideOpt = null)
internal override bool Equals(TypeSymbol? t2, TypeCompareKind comparison)
{
return this.Equals(t2 as ArrayTypeSymbol, comparison, isValueTypeOverrideOpt);
return this.Equals(t2 as ArrayTypeSymbol, comparison);
}

private bool Equals(ArrayTypeSymbol? other, TypeCompareKind comparison, IReadOnlyDictionary<TypeParameterSymbol, bool>? isValueTypeOverrideOpt)
private bool Equals(ArrayTypeSymbol? other, TypeCompareKind comparison)
{
if (ReferenceEquals(this, other))
{
return true;
}

if ((object?)other == null || !other.HasSameShapeAs(this) ||
!other.ElementTypeWithAnnotations.Equals(ElementTypeWithAnnotations, comparison, isValueTypeOverrideOpt))
!other.ElementTypeWithAnnotations.Equals(ElementTypeWithAnnotations, comparison))
{
return false;
}
Expand Down
Loading