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

Revert "Consider types with ref fields as managed (#63209)" #63367

Merged
merged 1 commit into from
Aug 12, 2022
Merged

Revert "Consider types with ref fields as managed (#63209)" #63367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
147 changes: 71 additions & 76 deletions src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ private static void StructDependsClosure(NamedTypeSymbol type, HashSet<Symbol> p
/// 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
/// if one of its instance fields is managed or a ref field. Unfortunately, this can result in infinite recursion.
/// if one of its instance fields is managed. Unfortunately, this can result in infinite recursion.
/// If the closure is finite, and we don't find anything definitely managed, then we return true.
/// If the closure is infinite, we disregard all but a representative of any expanding cycle.
///
Expand All @@ -128,12 +128,13 @@ internal static ManagedKind GetManagedKind(NamedTypeSymbol type, ref CompoundUse
{
// Otherwise, we have to build and inspect the closure of depended-upon types.
var hs = PooledHashSet<Symbol>.GetInstance();
var result = dependsOnDefinitelyManagedType(type, hs, ref useSiteInfo);
var result = DependsOnDefinitelyManagedType(type, hs, ref useSiteInfo);
definitelyManaged = result.definitelyManaged;
hasGenerics = hasGenerics || result.hasGenerics;
hs.Free();
}


if (definitelyManaged)
{
return ManagedKind.Managed;
Expand All @@ -146,98 +147,92 @@ internal static ManagedKind GetManagedKind(NamedTypeSymbol type, ref CompoundUse
{
return ManagedKind.Unmanaged;
}
}

static (bool definitelyManaged, bool hasGenerics) dependsOnDefinitelyManagedType(NamedTypeSymbol type, HashSet<Symbol> partialClosure, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
Debug.Assert((object)type != null);
// NOTE: If we do not check HasPointerType, we will unconditionally
// bind Type and that may cause infinite recursion.
// HasPointerType can use syntax directly and break recursion.
internal static TypeSymbol NonPointerType(this FieldSymbol field) =>
field.HasPointerType ? null : field.Type;

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

var hasGenerics = false;
if (partialClosure.Add(type))
var hasGenerics = false;
if (partialClosure.Add(type))
{
foreach (var member in type.GetInstanceFieldsAndEvents())
{
foreach (var member in type.GetInstanceFieldsAndEvents())
// Only instance fields (including field-like events) affect the outcome.
FieldSymbol field;
switch (member.Kind)
{
// Only instance fields (including field-like events) affect the outcome.
FieldSymbol field;
switch (member.Kind)
{
case SymbolKind.Field:
field = (FieldSymbol)member;
Debug.Assert((object)(field.AssociatedSymbol as EventSymbol) == null,
"Didn't expect to find a field-like event backing field in the member list.");
break;
case SymbolKind.Event:
field = ((EventSymbol)member).AssociatedField;
break;
default:
throw ExceptionUtilities.UnexpectedValue(member.Kind);
}
case SymbolKind.Field:
field = (FieldSymbol)member;
Debug.Assert((object)(field.AssociatedSymbol as EventSymbol) == null,
"Didn't expect to find a field-like event backing field in the member list.");
break;
case SymbolKind.Event:
field = ((EventSymbol)member).AssociatedField;
break;
default:
throw ExceptionUtilities.UnexpectedValue(member.Kind);
}

if ((object)field == null)
{
continue;
}
if ((object)field == null)
{
continue;
}

if (field.RefKind != RefKind.None)
{
// A ref struct which has a ref field is never considered unmanaged
return (true, hasGenerics);
}
TypeSymbol fieldType = field.NonPointerType();
if (fieldType is null)
{
// pointers are unmanaged
continue;
}

TypeSymbol fieldType = field.NonPointerType();
if (fieldType is null)
fieldType.AddUseSiteInfo(ref useSiteInfo);
NamedTypeSymbol fieldNamedType = fieldType as NamedTypeSymbol;
if ((object)fieldNamedType == null)
{
if (fieldType.IsManagedType(ref useSiteInfo))
{
// pointers are unmanaged
continue;
return (true, hasGenerics);
}

fieldType.AddUseSiteInfo(ref useSiteInfo);
NamedTypeSymbol fieldNamedType = fieldType as NamedTypeSymbol;
if ((object)fieldNamedType == null)
}
else
{
var result = IsManagedTypeHelper(fieldNamedType);
hasGenerics = hasGenerics || result.hasGenerics;
// NOTE: don't use ManagedKind.get on a NamedTypeSymbol - that could lead
// to infinite recursion.
switch (result.isManaged)
{
if (fieldType.IsManagedType(ref useSiteInfo))
{
case ThreeState.True:
return (true, hasGenerics);
}
}
else
{
var result = IsManagedTypeHelper(fieldNamedType);
hasGenerics = hasGenerics || result.hasGenerics;
// NOTE: don't use ManagedKind.get on a NamedTypeSymbol - that could lead
// to infinite recursion.
switch (result.isManaged)
{
case ThreeState.True:
return (true, hasGenerics);

case ThreeState.False:
continue;

case ThreeState.Unknown:
if (!fieldNamedType.OriginalDefinition.KnownCircularStruct)

case ThreeState.False:
continue;

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

return (false, hasGenerics);
}
}

// NOTE: If we do not check HasPointerType, we will unconditionally
// bind Type and that may cause infinite recursion.
// HasPointerType can use syntax directly and break recursion.
internal static TypeSymbol NonPointerType(this FieldSymbol field) =>
field.HasPointerType ? null : field.Type;
return (false, hasGenerics);
}

/// <summary>
/// Returns True or False if we can determine whether the type is managed
Expand Down
111 changes: 0 additions & 111 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7211,117 +7211,6 @@ static ref int F2()
Diagnostic(ErrorCode.ERR_EscapeVariable, "s1").WithArguments("s1").WithLocation(7, 20));
}

[Fact, WorkItem(63104, "https://github.com/dotnet/roslyn/issues/63104")]
public void RefFieldsConsideredManaged()
{
var source = @"
unsafe
{
StructWithRefField* p = stackalloc StructWithRefField[10]; // 1, 2
C.M<StructWithRefField>(); // 3
}

public ref struct StructWithRefField
{
public ref byte RefField;
}

class C
{
public static void M<T>() where T : unmanaged { }
}";
var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugExe, runtimeFeature: RuntimeFlag.ByRefFields);
comp.VerifyDiagnostics(
// (4,5): error CS0208: Cannot take the address of, get the size of, or declare a pointer to a managed type ('StructWithRefField')
// StructWithRefField* p = stackalloc StructWithRefField[10]; // 1, 2
Diagnostic(ErrorCode.ERR_ManagedAddr, "StructWithRefField*").WithArguments("StructWithRefField").WithLocation(4, 5),
// (4,40): error CS0208: Cannot take the address of, get the size of, or declare a pointer to a managed type ('StructWithRefField')
// StructWithRefField* p = stackalloc StructWithRefField[10]; // 1, 2
Diagnostic(ErrorCode.ERR_ManagedAddr, "StructWithRefField").WithArguments("StructWithRefField").WithLocation(4, 40),
// (5,7): error CS0306: The type 'StructWithRefField' may not be used as a type argument
// C.M<StructWithRefField>(); // 3
Diagnostic(ErrorCode.ERR_BadTypeArgument, "M<StructWithRefField>").WithArguments("StructWithRefField").WithLocation(5, 7)
);

Assert.True(comp.GetTypeByMetadataName("StructWithRefField").IsManagedTypeNoUseSiteDiagnostics);
}

[Fact, WorkItem(63104, "https://github.com/dotnet/roslyn/issues/63104")]
public void RefFieldsConsideredManaged_Generic()
{
var source = @"
unsafe
{
StructWithIndirectRefField* p = stackalloc StructWithIndirectRefField[10]; // 1, 2
C.M<StructWithRefField>(); // 3
}

ref struct StructWithIndirectRefField
{
public StructWithRefField<int> Field;
}
ref struct StructWithRefField<T>
{
public ref T RefField;
}

class C
{
public static void M<T>() where T : unmanaged { }
}";
var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugExe, runtimeFeature: RuntimeFlag.ByRefFields);
comp.VerifyDiagnostics(
// (4,5): error CS0208: Cannot take the address of, get the size of, or declare a pointer to a managed type ('StructWithIndirectRefField')
// StructWithIndirectRefField* p = stackalloc StructWithIndirectRefField[10]; // 1, 2
Diagnostic(ErrorCode.ERR_ManagedAddr, "StructWithIndirectRefField*").WithArguments("StructWithIndirectRefField").WithLocation(4, 5),
// (4,48): error CS0208: Cannot take the address of, get the size of, or declare a pointer to a managed type ('StructWithIndirectRefField')
// StructWithIndirectRefField* p = stackalloc StructWithIndirectRefField[10]; // 1, 2
Diagnostic(ErrorCode.ERR_ManagedAddr, "StructWithIndirectRefField").WithArguments("StructWithIndirectRefField").WithLocation(4, 48),
// (5,9): error CS0305: Using the generic type 'StructWithRefField<T>' requires 1 type arguments
// C.M<StructWithRefField>(); // 3
Diagnostic(ErrorCode.ERR_BadArity, "StructWithRefField").WithArguments("StructWithRefField<T>", "type", "1").WithLocation(5, 9),
// (10,36): warning CS0649: Field 'StructWithIndirectRefField.Field' is never assigned to, and will always have its default value
// public StructWithRefField<int> Field;
Diagnostic(ErrorCode.WRN_UnassignedInternalField, "Field").WithArguments("StructWithIndirectRefField.Field", "").WithLocation(10, 36),
// (14,18): warning CS0649: Field 'StructWithRefField<T>.RefField' is never assigned to, and will always have its default value
// public ref T RefField;
Diagnostic(ErrorCode.WRN_UnassignedInternalField, "RefField").WithArguments("StructWithRefField<T>.RefField", "").WithLocation(14, 18)
);

Assert.True(comp.GetTypeByMetadataName("StructWithIndirectRefField").IsManagedTypeNoUseSiteDiagnostics);
}

[Fact, WorkItem(63104, "https://github.com/dotnet/roslyn/issues/63104")]
public void RefFieldsConsideredManaged_Indirect()
{
var source = @"
unsafe
{
StructWithIndirectRefField* p = stackalloc StructWithIndirectRefField[10]; // 1, 2
}

public ref struct StructWithIndirectRefField
{
public StructWithRefField Field;
}

public ref struct StructWithRefField
{
public ref byte RefField;
}";
var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugExe, runtimeFeature: RuntimeFlag.ByRefFields);
comp.VerifyDiagnostics(
// (4,5): error CS0208: Cannot take the address of, get the size of, or declare a pointer to a managed type ('StructWithIndirectRefField')
// StructWithIndirectRefField* p = stackalloc StructWithIndirectRefField[10]; // 1, 2
Diagnostic(ErrorCode.ERR_ManagedAddr, "StructWithIndirectRefField*").WithArguments("StructWithIndirectRefField").WithLocation(4, 5),
// (4,48): error CS0208: Cannot take the address of, get the size of, or declare a pointer to a managed type ('StructWithIndirectRefField')
// StructWithIndirectRefField* p = stackalloc StructWithIndirectRefField[10]; // 1, 2
Diagnostic(ErrorCode.ERR_ManagedAddr, "StructWithIndirectRefField").WithArguments("StructWithIndirectRefField").WithLocation(4, 48)
);

Assert.True(comp.GetTypeByMetadataName("StructWithIndirectRefField").IsManagedTypeNoUseSiteDiagnostics);
}

// Breaking change in C#11: Cannot return an 'out' parameter by reference.
[Fact]
public void BreakingChange_ReturnOutByRef()
Expand Down