Skip to content

Commit

Permalink
Revert "Revert "Consider types with ref fields as managed (dotnet#63209…
Browse files Browse the repository at this point in the history 
…)" (dotnet#63367)"

This reverts commit 76bd0d9.
  • Loading branch information
@jcouv
jcouv committed Sep 16, 2022
1 parent f4b194b commit e40ecf2
Show file tree
Hide file tree
Showing 2 changed files with 187 additions and 71 deletions.
147 changes: 76 additions & 71 deletions src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs
View file
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. Unfortunately, this can result in infinite recursion.
/// if one of its instance fields is managed or a ref field. 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,13 +128,12 @@ 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 @@ -147,93 +146,99 @@ internal static ManagedKind GetManagedKind(NamedTypeSymbol type, ref CompoundUse
{
return ManagedKind.Unmanaged;
}
}

// 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))
static (bool definitelyManaged, bool hasGenerics) dependsOnDefinitelyManagedType(NamedTypeSymbol type, HashSet<Symbol> partialClosure, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
foreach (var member in type.GetInstanceFieldsAndEvents())
{
// 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);
}
Debug.Assert((object)type != null);

if ((object)field == null)
var hasGenerics = false;
if (partialClosure.Add(type))
{
foreach (var member in type.GetInstanceFieldsAndEvents())
{
continue;
}
// 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);
}

TypeSymbol fieldType = field.NonPointerType();
if (fieldType is null)
{
// pointers are unmanaged
continue;
}
if ((object)field == null)
{
continue;
}

fieldType.AddUseSiteInfo(ref useSiteInfo);
NamedTypeSymbol fieldNamedType = fieldType as NamedTypeSymbol;
if ((object)fieldNamedType == null)
{
if (fieldType.IsManagedType(ref useSiteInfo))
if (field.RefKind != RefKind.None)
{
// A ref struct which has a ref field is never considered unmanaged
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;
TypeSymbol fieldType = field.NonPointerType();
if (fieldType is null)
{
// pointers are unmanaged
continue;
}

case ThreeState.Unknown:
if (!fieldNamedType.OriginalDefinition.KnownCircularStruct)
{
var (definitelyManaged, childHasGenerics) = DependsOnDefinitelyManagedType(fieldNamedType, partialClosure, ref useSiteInfo);
hasGenerics = hasGenerics || childHasGenerics;
if (definitelyManaged)
fieldType.AddUseSiteInfo(ref useSiteInfo);
NamedTypeSymbol fieldNamedType = fieldType as NamedTypeSymbol;
if ((object)fieldNamedType == null)
{
if (fieldType.IsManagedType(ref useSiteInfo))
{
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)
{
return (true, hasGenerics);
var (definitelyManaged, childHasGenerics) = dependsOnDefinitelyManagedType(fieldNamedType, partialClosure, ref useSiteInfo);
hasGenerics = hasGenerics || childHasGenerics;
if (definitelyManaged)
{
return (true, hasGenerics);
}
}
}
continue;
continue;
}
}
}
}
}

return (false, hasGenerics);
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;

/// <summary>
/// Returns True or False if we can determine whether the type is managed
/// without looking at its fields and Unknown otherwise.
Expand Down
111 changes: 111 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7408,6 +7408,117 @@ 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

0 comments on commit e40ecf2

Please sign in to comment.