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

Features/unmanaged constructed types #32704

Merged
merged 47 commits into from
Jan 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
4c0a9ee
Allow unmanaged generic structs (#31148)
RikkiGibson Dec 6, 2018
e6c7e51
Merge branch 'master' into merge-p2
RikkiGibson Dec 6, 2018
036adc9
Merge pull request #31597 from RikkiGibson/merge-p2
RikkiGibson Dec 7, 2018
7b278f4
Add verification test
RikkiGibson Dec 7, 2018
7f630da
Skip verification in NestedGenericStructContainingPointer
RikkiGibson Dec 10, 2018
97a570e
Add SimpleGenericStructPointer_ILValidation
RikkiGibson Dec 11, 2018
80f47aa
Add regression test for issue #31439
RikkiGibson Dec 11, 2018
b7f0250
Add tests ensuring CSharp8 language version is required for the feature
RikkiGibson Dec 17, 2018
1f66018
Add MessageID.IDS_FeatureUnmanagedGenericStructs
RikkiGibson Dec 17, 2018
80721b4
Update ManagedAddr binder checks to require CSharp8 for unmanaged gen…
RikkiGibson Dec 17, 2018
b0c39b5
Fix sizeof error location
RikkiGibson Dec 19, 2018
d5697c6
Fix supported feature check for AddressOf
RikkiGibson Dec 19, 2018
1d694ad
Add simple ref struct test
RikkiGibson Dec 19, 2018
7773650
Add simple fixed statement tests for generic structs
RikkiGibson Dec 19, 2018
361b608
Add IsManagedType_GenericStruct test
RikkiGibson Dec 19, 2018
cb9e361
Allow multiple feature diagnostics for generic struct fixed statement
RikkiGibson Dec 20, 2018
994ca35
Add IsManagedType_GenericStruct_ErrorTypeArg test
RikkiGibson Dec 20, 2018
6c74419
Simplify most FeatureUnmanagedGenericStructs.CheckFeatureAvailability…
RikkiGibson Dec 20, 2018
9405b94
Add tests for array of generic struct as fixed initializer
RikkiGibson Dec 31, 2018
7dc2142
Quick fix for fixed initializers of array of generic struct
RikkiGibson Dec 31, 2018
ffcd0d9
Use Binder CheckFeatureAvailability for simplicity. Set hasErrors bas…
RikkiGibson Jan 2, 2019
597b7e9
Add CheckManagedAddr helper
RikkiGibson Jan 2, 2019
99a5302
Add partial reproducer for #32103
RikkiGibson Jan 3, 2019
f6f8498
Add feature availability checks when using a generic struct as an unm…
RikkiGibson Jan 3, 2019
9b1187e
Add ManagedKind property on TypeSymbol
RikkiGibson Jan 4, 2019
3529e2d
Fix MockNamedTypeSymbol
RikkiGibson Jan 4, 2019
15f2c61
Remove redundant arity check
RikkiGibson Jan 4, 2019
87d19f7
Check StructContainingTuple in C# 8
RikkiGibson Jan 4, 2019
e761590
Fix test name
RikkiGibson Jan 7, 2019
45388ce
Use IsFeatureEnabled API
RikkiGibson Jan 7, 2019
f0def60
Use expression bodies for simple ManagedKind overrides
RikkiGibson Jan 7, 2019
7be8538
Add ManagedKind API tests to UnsafeTests.cs
RikkiGibson Jan 8, 2019
ac8852c
Add StructContainingGenericTuple_Unmanaged
RikkiGibson Jan 8, 2019
2af8c74
Add doc comments for ManagedKind
RikkiGibson Jan 8, 2019
ca410a2
Simplify feature not available diagnostic in constraint checker
RikkiGibson Jan 8, 2019
ce27265
Move from ref param to tuple return
RikkiGibson Jan 9, 2019
0a83014
UnmanagedGenericStructs -> UnmanagedConstructedTypes
RikkiGibson Jan 9, 2019
71a1f82
Fixes from feedback
RikkiGibson Jan 10, 2019
b9e214f
Add test demonstrating expected behavior around multiple constraint v…
RikkiGibson Jan 10, 2019
eaa0cd8
Merge pull request #31602 from RikkiGibson/unmanaged-constructed-type…
RikkiGibson Jan 11, 2019
a3e0b73
Merge remote-tracking branch 'upstream/dev16.1-preview1' into feature…
RikkiGibson Jan 15, 2019
86f0984
Fix merge conflicts and test failures
RikkiGibson Jan 15, 2019
8bbdd4a
Merge pull request #32448 from RikkiGibson/features/unmanaged-constru…
RikkiGibson Jan 16, 2019
8f97bd9
Finish testing for unmanaged constructed types (#32591)
RikkiGibson Jan 22, 2019
ebd8e94
Merge remote-tracking branch 'upstream/dev16.1-preview1' into feature…
RikkiGibson Jan 22, 2019
8ccfd1e
Fix merge conflicts
RikkiGibson Jan 22, 2019
ffc2957
Merge pull request #32677 from RikkiGibson/features/unmanaged-constru…
RikkiGibson Jan 23, 2019
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
35 changes: 23 additions & 12 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1142,13 +1142,7 @@ private BoundExpression BindSizeOf(SizeOfExpressionSyntax node, DiagnosticBag di
AliasSymbol alias;
TypeSymbol type = this.BindType(typeSyntax, diagnostics, out alias).TypeSymbol;

bool typeHasErrors = type.IsErrorType();

if (!typeHasErrors && type.IsManagedType)
{
diagnostics.Add(ErrorCode.ERR_ManagedAddr, node.Location, type);
typeHasErrors = true;
}
bool typeHasErrors = type.IsErrorType() || CheckManagedAddr(type, node, diagnostics);

BoundTypeExpression boundType = new BoundTypeExpression(typeSyntax, alias, type, typeHasErrors);
ConstantValue constantValue = GetConstantSizeOf(type);
Expand All @@ -1157,6 +1151,24 @@ private BoundExpression BindSizeOf(SizeOfExpressionSyntax node, DiagnosticBag di
this.GetSpecialType(SpecialType.System_Int32, diagnostics, node), hasErrors);
}

/// <returns>true if managed type-related errors were found, otherwise false.</returns>
private static bool CheckManagedAddr(TypeSymbol type, SyntaxNode node, DiagnosticBag diagnostics)
{
var managedKind = type.ManagedKind;
if (managedKind == ManagedKind.Managed)
{
diagnostics.Add(ErrorCode.ERR_ManagedAddr, node.Location, type);
return true;
}
else if (managedKind == ManagedKind.UnmanagedWithGenerics)
{
var supported = CheckFeatureAvailability(node, MessageID.IDS_FeatureUnmanagedConstructedTypes, diagnostics);
return !supported;
}

return false;
}

internal static ConstantValue GetConstantSizeOf(TypeSymbol type)
{
return ConstantValue.CreateSizeOf((type.GetEnumUnderlyingType() ?? type).SpecialType);
Expand Down Expand Up @@ -2882,9 +2894,9 @@ private BoundExpression BindImplicitStackAllocArrayCreationExpression(ImplicitSt
bestType = CreateErrorType();
}

if (!bestType.IsErrorType() && bestType.IsManagedType)
if (!bestType.IsErrorType())
{
Error(diagnostics, ErrorCode.ERR_ManagedAddr, node, bestType);
CheckManagedAddr(bestType, node, diagnostics);
}

return BindStackAllocWithInitializer(
Expand Down Expand Up @@ -3235,10 +3247,9 @@ private BoundExpression BindStackAllocArrayCreationExpression(
var elementType = BindType(elementTypeSyntax, diagnostics);

TypeSymbol type = GetStackAllocType(node, elementType, diagnostics, out bool hasErrors);
if (!elementType.IsErrorType() && elementType.IsManagedType)
if (!elementType.IsErrorType())
{
Error(diagnostics, ErrorCode.ERR_ManagedAddr, elementTypeSyntax, elementType.TypeSymbol);
hasErrors = true;
hasErrors = hasErrors || CheckManagedAddr(elementType.TypeSymbol, elementTypeSyntax, diagnostics);
}

SyntaxList<ArrayRankSpecifierSyntax> rankSpecifiers = arrayTypeSyntax.RankSpecifiers;
Expand Down
5 changes: 2 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2117,10 +2117,9 @@ private BoundExpression BindAddressOfExpression(PrefixUnaryExpressionSyntax node
bool allowManagedAddressOf = Flags.Includes(BinderFlags.AllowManagedAddressOf);
if (!allowManagedAddressOf)
{
if (!hasErrors && isManagedType)
if (!hasErrors)
{
hasErrors = true;
Error(diagnostics, ErrorCode.ERR_ManagedAddr, node, operandType);
hasErrors = CheckManagedAddr(operandType, node, diagnostics);
}

if (!hasErrors)
Expand Down
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1209,9 +1209,8 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym
}
}

if (elementType.IsManagedType)
if (CheckManagedAddr(elementType, initializerSyntax, diagnostics))
{
Error(diagnostics, ErrorCode.ERR_ManagedAddr, initializerSyntax, elementType);
hasErrors = true;
}

Expand Down
25 changes: 14 additions & 11 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,9 @@ internal NamespaceOrTypeOrAliasSymbolWithAnnotations BindNamespaceOrTypeOrAliasS
// Invalid constraint type. A type used as a constraint must be an interface, a non-sealed class or a type parameter.
Error(diagnostics, ErrorCode.ERR_BadConstraintType, node);
}
else if (elementType.IsManagedType)
else
{
// "Cannot take the address of, get the size of, or declare a pointer to a managed type ('{0}')"
Error(diagnostics, ErrorCode.ERR_ManagedAddr, node, elementType.TypeSymbol);
CheckManagedAddr(elementType.TypeSymbol, node, diagnostics);
}

return TypeSymbolWithAnnotations.Create(new PointerTypeSymbol(elementType));
Expand Down Expand Up @@ -2233,6 +2232,17 @@ internal static bool CheckFeatureAvailability(SyntaxTree tree, MessageID feature
return false;
}

internal static CSDiagnosticInfo GetLanguageVersionDiagnosticInfo(LanguageVersion availableVersion, MessageID feature)
{
LanguageVersion requiredVersion = feature.RequiredVersion();
if (requiredVersion > availableVersion)
{
return new CSDiagnosticInfo(availableVersion.GetErrorCode(), feature.Localize(), new CSharpRequiredLanguageVersion(requiredVersion));
}

return null;
}

private static CSDiagnosticInfo GetFeatureAvailabilityDiagnosticInfo(SyntaxTree tree, MessageID feature)
{
CSharpParseOptions options = (CSharpParseOptions)tree.Options;
Expand All @@ -2248,14 +2258,7 @@ private static CSDiagnosticInfo GetFeatureAvailabilityDiagnosticInfo(SyntaxTree
return new CSDiagnosticInfo(ErrorCode.ERR_FeatureIsExperimental, feature.Localize(), requiredFeature);
}

LanguageVersion availableVersion = options.LanguageVersion;
LanguageVersion requiredVersion = feature.RequiredVersion();
if (requiredVersion > availableVersion)
{
return new CSDiagnosticInfo(availableVersion.GetErrorCode(), feature.Localize(), new CSharpRequiredLanguageVersion(requiredVersion));
}

return null;
return GetLanguageVersionDiagnosticInfo(options.LanguageVersion, feature);
}
}
}
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Imports.cs
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ private void Validate()
{
var typeSymbol = (TypeSymbol)@using.NamespaceOrType;
var location = @using.UsingDirective?.Name.Location ?? NoLocation.Singleton;
typeSymbol.CheckAllConstraints(conversions, location, semanticDiagnostics);
typeSymbol.CheckAllConstraints(_compilation, conversions, location, semanticDiagnostics);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3188,7 +3188,7 @@ private MemberResolutionResult<TMember> IsApplicable<TMember>(
var parameterTypes = leastOverriddenMember.GetParameterTypes();
for (int i = 0; i < parameterTypes.Length; i++)
{
if (!parameterTypes[i].TypeSymbol.CheckAllConstraints(Conversions))
if (!parameterTypes[i].TypeSymbol.CheckAllConstraints(Compilation, Conversions))
{
return new MemberResolutionResult<TMember>(member, leastOverriddenMember, MemberAnalysisResult.ConstructedParameterFailedConstraintsCheck(i));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,7 @@ private bool HadConstructedParameterFailedConstraintCheck(
// formal parameter type.

TypeSymbol formalParameterType = method.ParameterTypes[result.Result.BadParameter].TypeSymbol;
formalParameterType.CheckAllConstraints(conversions, location, diagnostics);
formalParameterType.CheckAllConstraints((CSharpCompilation)compilation, conversions, location, diagnostics);

return true;
}
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@
<data name="IDS_FeatureAsyncStreams" xml:space="preserve">
<value>async streams</value>
</data>
<data name="IDS_FeatureUnmanagedConstructedTypes" xml:space="preserve">
<value>unmanaged constructed types</value>
</data>
<data name="IDS_FeatureDefaultLiteral" xml:space="preserve">
<value>default literal</value>
</data>
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ internal enum MessageID
IDS_Disposable = MessageBase + 12753,
IDS_FeatureUsingDeclarations = MessageBase + 12754,
IDS_FeatureStaticLocalFunctions = MessageBase + 12755,
IDS_FeatureUnmanagedConstructedTypes = MessageBase + 12756
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -252,6 +253,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
case MessageID.IDS_FeatureRecursivePatterns:
case MessageID.IDS_FeatureUsingDeclarations:
case MessageID.IDS_FeatureStaticLocalFunctions:
case MessageID.IDS_FeatureUnmanagedConstructedTypes: // semantic check
return LanguageVersion.CSharp8;

// C# 7.3 features.
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/AliasSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ internal void CheckConstraints(DiagnosticBag diagnostics)
{
var corLibrary = this.ContainingAssembly.CorLibrary;
var conversions = new TypeConversions(corLibrary);
target.CheckAllConstraints(conversions, _locations[0], diagnostics);
target.CheckAllConstraints(DeclaringCompilation, conversions, _locations[0], diagnostics);
}
}

Expand Down
8 changes: 1 addition & 7 deletions src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,7 @@ public override bool IsValueType
}
}

internal sealed override bool IsManagedType
{
get
{
return true;
}
}
internal sealed override ManagedKind ManagedKind => ManagedKind.Managed;

public sealed override bool IsRefLikeType
{
Expand Down
84 changes: 49 additions & 35 deletions src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,7 @@ private static void StructDependsClosure(NamedTypeSymbol type, HashSet<Symbol> p
/// IsManagedType is simple for most named types:
/// enums are not managed;
/// non-enum, non-struct named types are managed;
/// generic types and their nested types are managed;
/// type parameters are managed;
/// type parameters are managed unless an 'unmanaged' constraint is present;
/// all special types have spec'd values (basically, (non-string) primitives) are not managed;
///
/// Only structs are complicated, because the definition is recursive. A struct type is managed
Expand All @@ -106,30 +105,40 @@ private static void StructDependsClosure(NamedTypeSymbol type, HashSet<Symbol> p
/// be managed even if it had no fields. e.g. struct S { S s; } is not managed, but struct S { S s; object o; }
/// is because we can point to object.
/// </summary>
internal static bool IsManagedType(NamedTypeSymbol type)
internal static ManagedKind GetManagedKind(NamedTypeSymbol type)
{
// If this is a type with an obvious answer, return quickly.
switch (IsManagedTypeHelper(type))
var (isManaged, hasGenerics) = IsManagedTypeHelper(type);
var definitelyManaged = isManaged == ThreeState.True;
if (isManaged == ThreeState.Unknown)
{
case ThreeState.True:
return true;
case ThreeState.False:
return false;
// Otherwise, we have to build and inspect the closure of depended-upon types.
var hs = PooledHashSet<Symbol>.GetInstance();
var result = DependsOnDefinitelyManagedType(type, hs);
definitelyManaged = result.definitelyManaged;
hasGenerics = hasGenerics || result.hasGenerics;
hs.Free();
}

// Otherwise, we have to build and inspect the closure of depended-upon types.
var hs = PooledHashSet<Symbol>.GetInstance();
bool result = DependsOnDefinitelyManagedType(type, hs);
hs.Free();
return result;

if (definitelyManaged)
{
return ManagedKind.Managed;
}
else if (hasGenerics)
{
return ManagedKind.UnmanagedWithGenerics;
}
else
{
return ManagedKind.Unmanaged;
}
}

private static bool DependsOnDefinitelyManagedType(NamedTypeSymbol type, HashSet<Symbol> partialClosure)
private static (bool definitelyManaged, bool hasGenerics) DependsOnDefinitelyManagedType(NamedTypeSymbol type, HashSet<Symbol> partialClosure)
{
Debug.Assert((object)type != null);

// NOTE: unlike in StructDependsClosure, we don't have to check for expanding cycles,
// because as soon as we see something with non-zero arity we kick out (generic => managed).
var hasGenerics = false;
if (partialClosure.Add(type))
{
foreach (var member in type.GetInstanceFieldsAndEvents())
Expand Down Expand Up @@ -170,45 +179,56 @@ private static bool DependsOnDefinitelyManagedType(NamedTypeSymbol type, HashSet
{
if (fieldType.IsManagedType)
{
return true;
return (true, hasGenerics);
}
}
else
{
// NOTE: don't use IsManagedType on a NamedTypeSymbol - that could lead
var result = IsManagedTypeHelper(fieldNamedType);
hasGenerics = hasGenerics || result.hasGenerics;
// NOTE: don't use ManagedKind.get on a NamedTypeSymbol - that could lead
// to infinite recursion.
switch (IsManagedTypeHelper(fieldNamedType))
switch (result.isManaged)
{
case ThreeState.True:
return true;
return (true, hasGenerics);

case ThreeState.False:
continue;

case ThreeState.Unknown:
if (DependsOnDefinitelyManagedType(fieldNamedType, partialClosure))
if (!fieldNamedType.OriginalDefinition.KnownCircularStruct)
{
return true;
var (definitelyManaged, childHasGenerics) = DependsOnDefinitelyManagedType(fieldNamedType, partialClosure);
hasGenerics = hasGenerics || childHasGenerics;
if (definitelyManaged)
{
return (true, hasGenerics);
}
}
continue;
}
}
}
}

return false;
return (false, hasGenerics);
}

/// <summary>
/// Returns a boolean value if we can determine whether the type is managed
/// without looking at its fields and Unset otherwise.
/// </summary>
private static ThreeState IsManagedTypeHelper(NamedTypeSymbol type)
private static (ThreeState isManaged, bool hasGenerics) IsManagedTypeHelper(NamedTypeSymbol type)
{
// To match dev10, we treat enums as their underlying types.
if (type.IsEnumType())
{
type = type.GetEnumUnderlyingType();
}

bool hasGenerics = type.TupleUnderlyingTypeOrSelf().GetArity() > 0;

// Short-circuit common cases.
switch (type.SpecialType)
{
Expand All @@ -231,26 +251,20 @@ private static ThreeState IsManagedTypeHelper(NamedTypeSymbol type)
case SpecialType.System_TypedReference:
case SpecialType.System_ArgIterator:
case SpecialType.System_RuntimeArgumentHandle:
return ThreeState.False;
return (ThreeState.False, hasGenerics);
case SpecialType.None:
default:
// CONSIDER: could provide cases for other common special types.
break; // Proceed with additional checks.
}

if (type.AllTypeArgumentCount() > 0)
{
return ThreeState.True;
}

switch (type.TypeKind)
{
case TypeKind.Enum:
return ThreeState.False;
return (ThreeState.False, hasGenerics);
case TypeKind.Struct:
return ThreeState.Unknown;
return (ThreeState.Unknown, hasGenerics);
default:
return ThreeState.True;
return (ThreeState.True, hasGenerics);
}
}

Expand Down
Loading