Skip to content

Commit

Permalink
Avoid cycles while binding type parameter constraints
Browse files Browse the repository at this point in the history
  • Loading branch information
AlekseyTs committed Nov 12, 2020
1 parent 50de29a commit a122504
Show file tree
Hide file tree
Showing 51 changed files with 1,098 additions and 341 deletions.
6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/BinderFlags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ internal enum BinderFlags : uint
/// </summary>
InEEMethodBinder = 1 << 30,

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

// Groups

AllClearedAtExecutableCodeBoundary = InLockBody | InCatchBlock | InCatchFilter | InFinallyBlock | InTryBlockOfTryCatch | InNestedFinallyBlock,
Expand Down
38 changes: 25 additions & 13 deletions src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
ImmutableArray<TypeParameterSymbol> typeParameters,
TypeParameterListSyntax typeParameterList,
SyntaxList<TypeParameterConstraintClauseSyntax> clauses,
ref IReadOnlyDictionary<TypeParameterSymbol, bool> isValueTypeOverride,
DiagnosticBag diagnostics,
bool performOnlyCycleSafeValidation,
bool isForOverride = false)
{
Debug.Assert(this.Flags.Includes(BinderFlags.GenericConstraintsClause));
Expand Down Expand Up @@ -99,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 Down Expand Up @@ -347,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 @@ -380,9 +380,13 @@ 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);
}
}
Expand Down Expand Up @@ -418,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 @@ -486,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 @@ -88,8 +88,11 @@ public sealed override ImmutableArray<TypeParameterSymbol> TypeParameters
get { return _typeParameters; }
}

public sealed override ImmutableArray<TypeParameterConstraintClause> GetTypeParameterConstraintClauses()
=> 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 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

0 comments on commit a122504

Please sign in to comment.