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

Specially handle more scenarios for type parameters with 'allows ref struct' constraint #73232

Merged
merged 4 commits into from
Apr 30, 2024
Merged

Specially handle more scenarios for type parameters with 'allows ref struct' constraint #73232

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
adec2e0
Specially handle more scenarios for type parameters with 'allows ref …
AlekseyTs Apr 25, 2024
baf8bce
More scenarios
AlekseyTs Apr 27, 2024
d3b024d
Fixup
AlekseyTs Apr 27, 2024
56482d2
Update src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
AlekseyTs Apr 29, 2024
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
10 changes: 5 additions & 5 deletions 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 @@ -1872,7 +1872,7 @@ private BoundExpression SynthesizeMethodGroupReceiver(CSharpSyntaxNode syntax, A

private bool IsBadLocalOrParameterCapture(Symbol symbol, TypeSymbol type, RefKind refKind)
{
if (refKind != RefKind.None || type.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (refKind != RefKind.None || type.IsRestrictedType())
Copy link
Member

@cston cston Apr 29, 2024 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

if (refKind != RefKind.None || type.IsRestrictedType())

Are we testing ErrorCode.ERR_AnonDelegateCantUseLocal? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we testing ErrorCode.ERR_AnonDelegateCantUseLocal?

No, I think this code path is covered by a test for ERR_AnonDelegateCantUseRefLike.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add the following test in the next PR:

        [Fact]
        public void IllegalCapturing_03()
        {
            var source = @"
ref struct R1
{
}

class C
{
    void M<T>(R1 r1, T t)
        where T : allows ref struct
    {
        R1 r2 = r1;
        T t2 = t;

        var d1 = () => r2;
        var d2 = () => t2;
    }
}
";
            var comp = CreateCompilation(source, targetFramework: s_targetFrameworkSupportingByRefLikeGenerics);

            comp.VerifyEmitDiagnostics(
                // (14,24): error CS8175: Cannot use ref local 'r2' inside an anonymous method, lambda expression, or query expression
                //         var d1 = () => r2;
                Diagnostic(ErrorCode.ERR_AnonDelegateCantUseLocal, "r2").WithArguments("r2").WithLocation(14, 24),
                // (15,24): error CS8175: Cannot use ref local 't2' inside an anonymous method, lambda expression, or query expression
                //         var d2 = () => t2;
                Diagnostic(ErrorCode.ERR_AnonDelegateCantUseLocal, "t2").WithArguments("t2").WithLocation(15, 24)
                );
        }

{
var containingMethod = this.ContainingMemberOrLambda as MethodSymbol;
if ((object)containingMethod != null && (object)symbol.ContainingSymbol != (object)containingMethod)
Expand Down Expand Up @@ -2046,7 +2046,7 @@ private BoundExpression BindNonMethod(SimpleNameSyntax node, Symbol symbol, Bind
}
else
{
Debug.Assert(parameter.Type.IsRefLikeType); // PROTOTYPE(RefStructInterfaces): adjust?
Debug.Assert(parameter.Type.IsRefLikeTypeOrAllowsByRefLike());
Error(diagnostics, ErrorCode.ERR_AnonDelegateCantUseRefLike, node, parameter.Name);
}
}
Expand All @@ -2056,7 +2056,7 @@ private BoundExpression BindNonMethod(SimpleNameSyntax node, Symbol symbol, Bind
bool capture = (this.ContainingMember() is MethodSymbol containingMethod && (object)primaryCtor != containingMethod);

if (capture &&
(parameter.RefKind != RefKind.None || parameter.Type.IsRestrictedType()) && // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
(parameter.RefKind != RefKind.None || parameter.Type.IsRestrictedType()) &&
!IsInsideNameof)
{
if (parameter.RefKind != RefKind.None)
Expand All @@ -2069,7 +2069,7 @@ private BoundExpression BindNonMethod(SimpleNameSyntax node, Symbol symbol, Bind
}
else
{
Debug.Assert(parameter.Type.IsRefLikeType); // PROTOTYPE(RefStructInterfaces): adjust?
Debug.Assert(parameter.Type.IsRefLikeTypeOrAllowsByRefLike());
Error(diagnostics, ErrorCode.ERR_UnsupportedPrimaryConstructorParameterCapturingRefLike, node, parameter.Name);
}
}
Expand Down Expand Up @@ -3187,7 +3187,7 @@ internal static void CheckRestrictedTypeInAsyncMethod(Symbol containingSymbol, T
Debug.Assert(errorCode is ErrorCode.ERR_BadSpecialByRefLocal or ErrorCode.ERR_BadSpecialByRefUsing or ErrorCode.ERR_BadSpecialByRefLock);
if (containingSymbol.Kind == SymbolKind.Method
&& ((MethodSymbol)containingSymbol).IsAsync
&& type.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
&& type.IsRestrictedType())
{
if (errorCode == ErrorCode.ERR_BadSpecialByRefLock)
{
Expand Down
34 changes: 33 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3568,8 +3568,35 @@ internal static ConstantValue GetIsOperatorConstantResult(

// * If either type is a restricted type, the type check isn't supported because
// a restricted type cannot be boxed or unboxed into.
Copy link
Member

@jjonescz jjonescz Apr 29, 2024 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

Consider adjusting this comment to reflect the implementation changes. #Resolved

if (targetType.IsRestrictedType() || operandType.IsRestrictedType()) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (targetType.IsRestrictedType() || operandType.IsRestrictedType())
{
if (targetType is TypeParameterSymbol { AllowByRefLike: true })
{
if (!operandType.IsRefLikeType && operandType is not TypeParameterSymbol)
{
return null;
}
}
else if (operandType is not TypeParameterSymbol { AllowByRefLike: true })
{
if (targetType.IsRefLikeType)
{
if (operandType is TypeParameterSymbol)
{
Debug.Assert(operandType is TypeParameterSymbol { AllowByRefLike: false });
return ConstantValue.False;
}
}
else if (operandType.IsRefLikeType)
{
if (targetType is TypeParameterSymbol)
{
Debug.Assert(targetType is TypeParameterSymbol { AllowByRefLike: false });
return ConstantValue.False;
}
}
}

return ConstantValue.Bad;
}

Expand Down Expand Up @@ -3981,6 +4008,11 @@ internal static ConstantValue GetAsOperatorConstantResult(TypeSymbol operandType

if (!isOperatorConstantResult.BooleanValue)
{
if (operandType?.IsRefLikeType == true)
{
return ConstantValue.Bad;
}

return ConstantValue.Null;
}
}
Expand Down
1 change: 0 additions & 1 deletion 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 @@ -752,7 +752,6 @@ internal MethodSymbol TryFindDisposePatternMethod(BoundExpression expr, SyntaxNo
{
Debug.Assert(expr is object);
Debug.Assert(expr.Type is object);
// PROTOTYPE(RefStructInterfaces): adjust?
Debug.Assert(expr.Type.IsRefLikeType || hasAwait); // pattern dispose lookup is only valid on ref structs or asynchronous usings
Copy link
Member

@jjonescz jjonescz Apr 29, 2024 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

Is pattern dispose not allowed on allows ref struct type parameters? Do we have tests for that? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is pattern dispose not allowed on allows ref struct type parameters? Do we have tests for that?

I think Using_06 covers that. The fact is also explicitly stated in https://github.com/dotnet/csharplang/blob/main/proposals/ref-struct-interfaces.md#using-statement section.


var result = PerformPatternMethodLookup(expr,
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ private static TypeWithAnnotations DelegateReturnTypeWithAnnotations(MethodSymbo
getEffectiveScopeFromSymbol = true;
}
}
else if (type.IsRefLikeType() && ParameterSyntax(i)?.Modifiers.Any(SyntaxKind.ParamsKeyword) == true) // PROTOTYPE(RefStructInterfaces): adjust?
else if (type.IsRefLikeTypeOrAllowsByRefLike() && ParameterSyntax(i)?.Modifiers.Any(SyntaxKind.ParamsKeyword) == true)
{
scope = ScopedKind.ScopedValue;
if (_unboundLambda.ParameterAttributes(i).Any())
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3989,7 +3989,7 @@ private void EmitStaticCast(TypeSymbol to, SyntaxNode syntax)

private void EmitBox(TypeSymbol type, SyntaxNode syntaxNode)
{
Debug.Assert(!type.IsRefLikeType); // PROTOTYPE(RefStructInterfaces): adjust?
Debug.Assert(!type.IsRefLikeType);

_builder.EmitOpCode(ILOpCode.Box);
EmitSymbolToken(type, syntaxNode);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11784,7 +11784,7 @@ protected override void VisitInterpolatedStringHandlerConstructor(BoundExpressio

public override BoundNode? VisitStackAllocArrayCreation(BoundStackAllocArrayCreation node)
{
Debug.Assert(node.Type is null || node.Type.IsErrorType() || node.Type.IsRefLikeType); // PROTOTYPE(RefStructInterfaces): adjust?
Debug.Assert(node.Type is null || node.Type.IsErrorType() || node.Type.IsRefLikeType);
return VisitStackAllocArrayCreationBase(node);
}

Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Lowering/ClosureConversion/ClosureConversion.Analysis.Tree.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,9 @@ private void AddDiagnosticIfRestrictedType(Symbol capturedVariable, SyntaxNode s
return;
}

if (type.IsRestrictedType() == true) // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
if (type.IsRestrictedType() == true)
{
Debug.Assert(false); // Add test(s) for scenarios that hit this code path
_diagnostics.Add(ErrorCode.ERR_SpecialByRefInLambda, syntax.Location, type);
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Lowering/Instrumentation/LocalStateTracingInstrumenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,10 @@ _ when variableType.IsPointerOrFunctionPointer()
=> WellKnownMember.Microsoft_CodeAnalysis_Runtime_LocalStoreTracker__LogLocalStorePointer,
_ when !variableType.IsManagedTypeNoUseSiteDiagnostics
=> WellKnownMember.Microsoft_CodeAnalysis_Runtime_LocalStoreTracker__LogLocalStoreUnmanaged,
_ when variableType.IsRefLikeType && !hasOverriddenToString(variableType) // PROTOTYPE(RefStructInterfaces): adjust?
_ when variableType.IsRefLikeType && !hasOverriddenToString(variableType)
=> null, // not possible to invoke ToString on ref struct that doesn't override it
_ when variableType is TypeParameterSymbol { AllowByRefLike: true }
=> null, // not possible to invoke ToString on ref struct type parameter
_ when variableType.TypeKind is TypeKind.Struct
// we'll emit ToString constrained virtcall to avoid boxing the struct
=> WellKnownMember.Microsoft_CodeAnalysis_Runtime_LocalStoreTracker__LogLocalStoreString,
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ private BoundExpression Spill(
continue;

case BoundKind.Sequence:
if (refKind != RefKind.None || expression.Type?.IsRefLikeType == true) // PROTOTYPE(RefStructInterfaces): adjust?
if (refKind != RefKind.None || expression.Type?.IsRefLikeTypeOrAllowsByRefLike() == true)
{
var sequence = (BoundSequence)expression;

Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,11 @@ private static bool HoistInDebugBuild(Symbol symbol)
{
ParameterSymbol parameter =>
// in Debug build hoist all parameters that can be hoisted:
!parameter.Type.IsRestrictedType(), // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
!parameter.Type.IsRestrictedType(),
LocalSymbol { IsConst: false, IsPinned: false, IsRef: false } local =>
// hoist all user-defined locals and long-lived temps that can be hoisted:
local.SynthesizedKind.MustSurviveStateMachineSuspension() &&
!local.Type.IsRestrictedType(), // PROTOTYPE(RefStructInterfaces): Is this doing the right thing for 'allows ref struct' type parameters?
!local.Type.IsRestrictedType(),
_ => false
};
}
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/AnonymousTypeField.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ public AnonymousTypeField(
bool isParams = false,
bool hasUnscopedRefAttribute = false)
{
Debug.Assert(!isParams || !typeWithAnnotations.Type.IsTypeParameter());
this.Name = name;
this.Location = location;
this.TypeWithAnnotations = typeWithAnnotations;
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,8 @@ public bool IsSZArray() =>
_extensions.IsSZArray(DefaultType);
public bool IsRefLikeType() =>
_extensions.IsRefLikeType(DefaultType);
public bool IsRefLikeTypeOrAllowsByRefLike() =>
_extensions.IsRefLikeTypeOrAllowsByRefLike(DefaultType);
public bool IsStatic =>
_extensions.IsStatic(DefaultType);
public bool IsRestrictedType(bool ignoreSpanLikeTypes = false) =>
Expand Down Expand Up @@ -865,6 +867,7 @@ internal static Extensions Create(ImmutableArray<CustomModifier> customModifiers
internal abstract bool IsVoid(TypeSymbol typeSymbol);
internal abstract bool IsSZArray(TypeSymbol typeSymbol);
internal abstract bool IsRefLikeType(TypeSymbol typeSymbol);
internal abstract bool IsRefLikeTypeOrAllowsByRefLike(TypeSymbol typeSymbol);

internal abstract TypeWithAnnotations WithTypeAndModifiers(TypeWithAnnotations type, TypeSymbol typeSymbol, ImmutableArray<CustomModifier> customModifiers);

Expand Down Expand Up @@ -896,6 +899,7 @@ public NonLazyType(ImmutableArray<CustomModifier> customModifiers)
internal override bool IsVoid(TypeSymbol typeSymbol) => typeSymbol.IsVoidType();
internal override bool IsSZArray(TypeSymbol typeSymbol) => typeSymbol.IsSZArray();
internal override bool IsRefLikeType(TypeSymbol typeSymbol) => typeSymbol.IsRefLikeType;
internal override bool IsRefLikeTypeOrAllowsByRefLike(TypeSymbol typeSymbol) => typeSymbol.IsRefLikeTypeOrAllowsByRefLike();

internal override TypeSymbol GetNullableUnderlyingTypeOrSelf(TypeSymbol typeSymbol) => typeSymbol.StrippedType();

Expand Down Expand Up @@ -968,6 +972,7 @@ public LazySubstitutedType(ImmutableArray<CustomModifier> customModifiers, TypeP
internal override bool IsVoid(TypeSymbol typeSymbol) => typeSymbol.IsVoidType();
internal override bool IsSZArray(TypeSymbol typeSymbol) => typeSymbol.IsSZArray();
internal override bool IsRefLikeType(TypeSymbol typeSymbol) => typeSymbol.IsRefLikeType;
internal override bool IsRefLikeTypeOrAllowsByRefLike(TypeSymbol typeSymbol) => typeSymbol.IsRefLikeTypeOrAllowsByRefLike();

internal override NullableAnnotation GetResolvedAnnotation(NullableAnnotation defaultAnnotation)
{
Expand Down Expand Up @@ -1068,6 +1073,7 @@ public LazyNullableTypeParameter(CSharpCompilation compilation, TypeWithAnnotati
internal override bool IsVoid(TypeSymbol typeSymbol) => false;
internal override bool IsSZArray(TypeSymbol typeSymbol) => false;
internal override bool IsRefLikeType(TypeSymbol typeSymbol) => false;
internal override bool IsRefLikeTypeOrAllowsByRefLike(TypeSymbol typeSymbol) => typeSymbol.IsRefLikeTypeOrAllowsByRefLike();
internal override bool IsStatic(TypeSymbol typeSymbol) => false;

private TypeSymbol GetResolvedType()
Expand Down
82 changes: 79 additions & 3 deletions src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ private static string MemoryToString(void* address, int size)
private static string WithHelpers(string source)
=> source + s_helpers;

private static readonly TargetFramework s_targetFramework = TargetFramework.Net70;
private const TargetFramework s_targetFramework = TargetFramework.Net70;

private static readonly Verification s_verification = Verification.Fails with
{
Expand All @@ -180,13 +180,13 @@ private static string WithHelpers(string source)
"""
};

private CompilationVerifier CompileAndVerify(string source, string? ilVerifyMessage = null, string? expectedOutput = null)
private CompilationVerifier CompileAndVerify(string source, string? ilVerifyMessage = null, string? expectedOutput = null, TargetFramework targetFramework = s_targetFramework)
=> CompileAndVerify(
source,
options: (expectedOutput != null) ? TestOptions.UnsafeDebugExe : TestOptions.UnsafeDebugDll,
emitOptions: s_emitOptions,
verify: s_verification with { ILVerifyMessage = ilVerifyMessage + Environment.NewLine + s_verification.ILVerifyMessage },
targetFramework: s_targetFramework,
targetFramework: targetFramework,
expectedOutput: expectedOutput);

// Only used to diagnose test verification failures (rename CompileAndVerify to CompileAndVerifyFails and rerun).
Expand Down Expand Up @@ -1682,6 +1682,82 @@ .locals init (Microsoft.CodeAnalysis.Runtime.LocalStoreTracker V_0,
}");
}

[Fact]
public void RefStructTypeParameter()
{
var source = WithHelpers("""
S.F(new S());

ref struct S
{
ref int X;

public static void F<T>(T p)
where T : struct, allows ref struct
{
int a = 1;
var x = p = default(T);
}
}
""");
var verifier = CompileAndVerify(
source,
targetFramework: TargetFramework.Net80, // PROTOTYPE(RefStructInterfaces): Switch to supporting target framework once we have its ref assemblies.
expectedOutput: @"
<Main>$: Entered
<Main>$: P'args'[0] = System.String[]
F: Entered
F: L1 = 1
F: Returned
<Main>$: Returned
");

// writes to x and p are not logged since we can't invoke ToString()
verifier.VerifyMethodBody("S.F<T>(T)", @"
{
// Code size 46 (0x2e)
.maxstack 3
.locals init (Microsoft.CodeAnalysis.Runtime.LocalStoreTracker V_0,
int V_1, //a
T V_2) //x
// sequence point: <hidden>
IL_0000: ldtoken ""void S.F<T>(T)""
IL_0005: call ""Microsoft.CodeAnalysis.Runtime.LocalStoreTracker Microsoft.CodeAnalysis.Runtime.LocalStoreTracker.LogMethodEntry(int)""
IL_000a: stloc.0
.try
{
// sequence point: {
IL_000b: nop
// sequence point: int a = 1;
IL_000c: ldloca.s V_0
IL_000e: ldc.i4.1
IL_000f: dup
IL_0010: stloc.1
IL_0011: ldc.i4.1
IL_0012: call ""void Microsoft.CodeAnalysis.Runtime.LocalStoreTracker.LogLocalStore(uint, int)""
IL_0017: nop
// sequence point: var x = p = default(T);
IL_0018: ldarga.s V_0
IL_001a: initobj ""T""
IL_0020: ldarg.0
IL_0021: stloc.2
// sequence point: }
IL_0022: leave.s IL_002d
}
finally
{
// sequence point: <hidden>
IL_0024: ldloca.s V_0
IL_0026: call ""void Microsoft.CodeAnalysis.Runtime.LocalStoreTracker.LogReturn()""
IL_002b: nop
IL_002c: endfinally
}
// sequence point: }
IL_002d: ret
}
");
}

[Fact]
public void UnmanagedRefStruct()
{
Expand Down
Loading