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.

Sign up for GitHub

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

Support unmanaged constraint #24817

Merged
merged 7 commits into from
Feb 24, 2018
Merged

Support unmanaged constraint #24817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0e6d4ce
Source changes for unmanaged (PROTOTYPE for metadata)
OmarTawfik Jan 26, 2018
fb7e482
Modreqs for unmanaged as well
OmarTawfik Feb 1, 2018
45bd5af
Finished roundtripping metadata
OmarTawfik Feb 2, 2018
7bb24dc
Added more tests
OmarTawfik Feb 13, 2018
53f3977
Fix CI
OmarTawfik Feb 14, 2018
3c47bf4
More tests
OmarTawfik Feb 14, 2018
97d0909
Clean up
OmarTawfik Feb 17, 2018
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
97 changes: 62 additions & 35 deletions src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ private TypeParameterConstraintClause BindTypeParameterConstraints(
{
var constraints = TypeParameterConstraintKind.None;
var constraintTypes = ArrayBuilder<TypeSymbol>.GetInstance();
var isStruct = false;

for (int i = 0, n = constraintsSyntax.Count; i < n; i++)
{
Expand All @@ -113,14 +112,17 @@ private TypeParameterConstraintClause BindTypeParameterConstraints(
diagnostics.Add(ErrorCode.ERR_RefValBoundMustBeFirst, syntax.GetFirstToken().GetLocation());
}

isStruct = true;
constraints |= TypeParameterConstraintKind.ValueType;
continue;
case SyntaxKind.ConstructorConstraint:
if (isStruct)
if ((constraints & TypeParameterConstraintKind.ValueType) != 0)
{
diagnostics.Add(ErrorCode.ERR_NewBoundWithVal, syntax.GetFirstToken().GetLocation());
}
if ((constraints & TypeParameterConstraintKind.Unmanaged) != 0)
{
diagnostics.Add(ErrorCode.ERR_NewBoundWithUnmanaged, syntax.GetFirstToken().GetLocation());
}

if (i != n - 1)
{
Expand All @@ -133,55 +135,80 @@ private TypeParameterConstraintClause BindTypeParameterConstraints(
{
var typeConstraintSyntax = (TypeConstraintSyntax)syntax;
var typeSyntax = typeConstraintSyntax.Type;
if (typeSyntax.Kind() != SyntaxKind.PredefinedType && !SyntaxFacts.IsName(typeSyntax.Kind()))
var typeSyntaxKind = typeSyntax.Kind();
if (typeSyntaxKind != SyntaxKind.PredefinedType && typeSyntaxKind != SyntaxKind.PointerType && !SyntaxFacts.IsName(typeSyntax.Kind()))
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typeSyntaxKind != SyntaxKind.PointerType [](start = 79, length = 40)

It is not obvious why we are no longer reporting this error for pointers. Could you elaborate? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment. This error is reported for pointers during binding typeSyntax (to prevent cycles). So we don't need a duplicate error here.

In reply to: 172416549 [](ancestors = 172416549)

{
diagnostics.Add(ErrorCode.ERR_BadConstraintType, typeSyntax.GetLocation());
}

var type = this.BindType(typeSyntax, diagnostics);
var type = BindTypeOrUnmanagedKeyword(typeSyntax, diagnostics, out var isUnmanaged);

// Only valid constraint types are included in ConstraintTypes
// 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 (!IsValidConstraintType(typeConstraintSyntax, type, diagnostics))
if (isUnmanaged)
{
continue;
}
if (constraints != 0 || constraintTypes.Any())
{
diagnostics.Add(ErrorCode.ERR_UnmanagedConstraintMustBeAlone, typeSyntax.GetLocation());
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diagnostics.Add(ErrorCode.ERR_UnmanagedConstraintMustBeAlone, typeSyntax.GetLocation()); [](start = 36, length = 88)

Why can't it be combined with an interface? #Closed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be able to combine with interfaces. I think we discussed this on the tesplan review as a potential hole/followup issue.

In reply to: 172416658 [](ancestors = 172416658)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vlad took care of that.

In reply to: 173018531 [](ancestors = 173018531,172416658)

continue;
}

if (constraintTypes.Contains(type))
{
// "Duplicate constraint '{0}' for type parameter '{1}'"
Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type, name);
// This should produce diagnostics if the types are missing
GetWellKnownType(WellKnownType.System_Runtime_InteropServices_UnmanagedType, diagnostics, typeSyntax);
GetSpecialType(SpecialType.System_ValueType, diagnostics, typeSyntax);

constraints |= TypeParameterConstraintKind.Unmanaged;
continue;
}

if (type.TypeKind == TypeKind.Class)
else
{
// 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
// drop this additional class, we may end up with conflicting class constraints.
// Only valid constraint types are included in ConstraintTypes
// 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 (!IsValidConstraintType(typeConstraintSyntax, type, diagnostics))
{
continue;
}

if (constraintTypes.Contains(type))
{
// "Duplicate constraint '{0}' for type parameter '{1}'"
Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type, name);
continue;
}

if (constraintTypes.Count > 0)
if ((constraints & TypeParameterConstraintKind.Unmanaged) != 0)
{
// "The class type constraint '{0}' must come before any other constraints"
Error(diagnostics, ErrorCode.ERR_ClassBoundNotFirst, syntax, type);
diagnostics.Add(ErrorCode.ERR_UnmanagedConstraintMustBeAlone, typeSyntax.GetLocation());
continue;
}

if ((constraints & (TypeParameterConstraintKind.ReferenceType | TypeParameterConstraintKind.ValueType)) != 0)
if (type.TypeKind == TypeKind.Class)
{
switch (type.SpecialType)
// 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
// drop this additional class, we may end up with conflicting class constraints.

if (constraintTypes.Count > 0)
{
// "The class type constraint '{0}' must come before any other constraints"
Error(diagnostics, ErrorCode.ERR_ClassBoundNotFirst, syntax, type);
continue;
}

if ((constraints & (TypeParameterConstraintKind.ReferenceType | TypeParameterConstraintKind.ValueType)) != 0)
{
case SpecialType.System_Enum:
case SpecialType.System_Delegate:
case SpecialType.System_MulticastDelegate:
break;

default:
// "'{0}': cannot specify both a constraint class and the 'class' or 'struct' constraint"
Error(diagnostics, ErrorCode.ERR_RefValBoundWithClass, syntax, type);
continue;
switch (type.SpecialType)
{
case SpecialType.System_Enum:
case SpecialType.System_Delegate:
case SpecialType.System_MulticastDelegate:
break;

default:
// "'{0}': cannot specify both a constraint class and the 'class' or 'struct' constraint"
Error(diagnostics, ErrorCode.ERR_RefValBoundWithClass, syntax, type);
continue;
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2340,7 +2340,7 @@ private BoundExpression BindOutVariableDeclarationArgument(
if (typeSyntax.IsVar)
{
var ignored = DiagnosticBag.GetInstance();
BindTypeOrAlias(typeSyntax, ignored, out isVar);
BindTypeOrAliasOrVarKeyword(typeSyntax, ignored, out isVar);
ignored.Free();

if (isVar)
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ private BoundPattern BindDeclarationPattern(

bool isVar;
AliasSymbol aliasOpt;
TypeSymbol declType = BindType(typeSyntax, diagnostics, out isVar, out aliasOpt);
TypeSymbol declType = BindTypeOrVarKeyword(typeSyntax, diagnostics, out isVar, out aliasOpt);
if (isVar)
{
declType = operandType;
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ declarationNode is VariableDesignationSyntax ||
// we want to treat the declaration as an explicitly typed declaration.

RefKind refKind;
TypeSymbol declType = BindType(typeSyntax.SkipRef(out refKind), diagnostics, out isVar, out alias);
TypeSymbol declType = BindTypeOrVarKeyword(typeSyntax.SkipRef(out refKind), diagnostics, out isVar, out alias);
Debug.Assert((object)declType != null || isVar);

if (isVar)
Expand Down Expand Up @@ -2153,7 +2153,7 @@ internal BoundStatement BindForOrUsingOrFixedDeclarations(VariableDeclarationSyn

AliasSymbol alias;
bool isVar;
TypeSymbol declType = BindType(typeSyntax, diagnostics, out isVar, out alias);
TypeSymbol declType = BindTypeOrVarKeyword(typeSyntax, diagnostics, out isVar, out alias);

Debug.Assert((object)declType != null || isVar);

Expand Down
Loading