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

Handle cases where 'field' keyword shouldn't generate a field #60806

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,10 @@ internal virtual ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsFo
return this.Next.GetDeclaredLocalFunctionsForScope(scopeDesignator);
}

internal virtual FieldSymbol? GetSymbolForPossibleFieldKeyword()
internal virtual FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
{
RoslynDebug.Assert(Next is object);
return this.Next.GetSymbolForPossibleFieldKeyword();
return this.Next.GetSymbolForPossibleFieldKeyword(diagnostics);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1563,7 +1563,7 @@ private BoundExpression BindIdentifier(
else if (node.Identifier.ContextualKind() == SyntaxKind.FieldKeyword &&
ContainingMember().CanHaveFieldKeywordBackingField())
{
if (GetSymbolForPossibleFieldKeyword() is { } backingField)
if (GetSymbolForPossibleFieldKeyword(diagnostics) is { } backingField)
{
expression = BindNonMethod(node, backingField, diagnostics, LookupResultKind.Viable, indexed: false, isError: false);
if (IsInsideNameof)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ internal override BoundStatement BindLockStatementParts(BindingDiagnosticBag dia
throw ExceptionUtilities.Unreachable;
}

internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
internal override FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
{
// There's supposed to either be a FieldKeywordBinder or a SpeculativeFieldKEywordBinder in the chain.
throw ExceptionUtilities.Unreachable;
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/FieldKeywordBinder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

Expand All @@ -19,9 +19,9 @@ internal FieldKeywordBinder(SourcePropertyAccessorSymbol accessor, Binder next)
_accessor = accessor;
}

internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
internal override FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
{
return _accessor.Property.GetOrCreateBackingFieldForFieldKeyword();
return _accessor.Property.GetOrCreateBackingFieldForFieldKeyword(diagnostics);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

Expand All @@ -19,7 +19,7 @@ internal SpeculativeFieldKeywordBinder(SourcePropertyAccessorSymbol accessor, Bi
_accessor = accessor;
}

internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
internal override FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
{
// field in the speculative model does not bind to a backing field if the original location was not a semi-auto property
return _accessor.Property.FieldKeywordBackingField;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ protected SourcePropertySymbolBase(
Debug.Assert(!IsIndexer);
// PROTOTYPE(semi-auto-props): Make sure that TestSemiAutoPropertyWithInitializer (when enabled back) is affected by this.
// That is, if we removed "hasInitializer", the test should fail, or any other test should get affected.
GetOrCreateBackingField(isCreatedForFieldKeyword: hasInitializer && !isAutoProperty, isEarlyConstructed: true);
GetOrCreateBackingField(isCreatedForFieldKeyword: hasInitializer && !isAutoProperty, isEarlyConstructed: true, diagnostics);
}

if (hasGetAccessor)
Expand All @@ -198,7 +198,7 @@ protected SourcePropertySymbolBase(
}
}

private SynthesizedBackingFieldSymbol? GetOrCreateBackingField(bool isCreatedForFieldKeyword, bool isEarlyConstructed)
private SynthesizedBackingFieldSymbol? GetOrCreateBackingField(bool isCreatedForFieldKeyword, bool isEarlyConstructed, BindingDiagnosticBag diagnostics)
{
Debug.Assert(!IsIndexer);
if (_lazyBackingFieldSymbol == _lazyBackingFieldSymbolSentinel)
Expand All @@ -209,16 +209,17 @@ protected SourcePropertySymbolBase(
this.IsStatic,
hasInitializer: (_propertyFlags & Flags.HasInitializer) != 0,
isCreatedForFieldKeyword: isCreatedForFieldKeyword,
isEarlyConstructed: isEarlyConstructed);
isEarlyConstructed: isEarlyConstructed,
diagnostics);
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Interlocked.CompareExchange(ref _lazyBackingFieldSymbol, backingField, _lazyBackingFieldSymbolSentinel);
}

return (SynthesizedBackingFieldSymbol?)_lazyBackingFieldSymbol;
}

internal SynthesizedBackingFieldSymbol? GetOrCreateBackingFieldForFieldKeyword()
internal SynthesizedBackingFieldSymbol? GetOrCreateBackingFieldForFieldKeyword(BindingDiagnosticBag diagnostics)
{
return GetOrCreateBackingField(isCreatedForFieldKeyword: true, isEarlyConstructed: false);
return GetOrCreateBackingField(isCreatedForFieldKeyword: true, isEarlyConstructed: false, diagnostics);
}

private void EnsureSignatureGuarded(BindingDiagnosticBag diagnostics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public SynthesizedBackingFieldSymbol(
bool isStatic,
bool hasInitializer,
bool isCreatedForFieldKeyword,
bool isEarlyConstructed)
bool isEarlyConstructed,
BindingDiagnosticBag diagnostics)
{
Debug.Assert(!string.IsNullOrEmpty(name));

Expand All @@ -65,12 +66,16 @@ public SynthesizedBackingFieldSymbol(
_backingFieldFlags |= Flags.IsCreatedForFieldKeyword;
}


if (isEarlyConstructed)
{
_backingFieldFlags |= Flags.IsEarlyConstructed;
}

if (ContainingType.IsInterface && !isStatic)
{
diagnostics.Add(ErrorCode.ERR_InterfacesCantContainFields, ErrorLocation);
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
}

// If it's not early constructed, it must have been created for field keyword.
Debug.Assert(IsEarlyConstructed || IsCreatedForFieldKeyword);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,142 @@ private void VerifyTypeIL(CSharpCompilation compilation, string typeName, string
CompileAndVerify(compilation).VerifyTypeIL(typeName, expected);
}

[Fact] // PROTOTYPE(semi-auto-props): Test with initializer when they're supported.
public void TestInInterface()
{
var comp = CreateCompilation(@"
public interface I
{
public int P { get => field; }
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
}
", targetFramework: TargetFramework.NetCoreApp); // setting TargetFramework for DefaultImplementationsOfInterfaces to exist.

var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics(
// (4,16): error CS0525: Interfaces cannot contain instance fields
// public int P { get => field; }
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "P").WithLocation(4, 16)
);
var @interface = comp.GetTypeByMetadataName("I");
Assert.Empty(@interface.GetMembers().OfType<FieldSymbol>());
Assert.Equal("System.Int32 I.<P>k__BackingField", @interface.GetFieldsToEmit().Single().ToTestDisplayString());
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact]
public void TestStaticInInterface()
{
var comp = CreateCompilation(@"
public interface I
{
public static int P { get => field; }
}
", targetFramework: TargetFramework.NetCoreApp); // setting TargetFramework for DefaultImplementationsOfInterfaces to exist.

var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics();
VerifyTypeIL(comp, "I", @"
.class interface public auto ansi abstract I
{
// Fields
.field private static initonly int32 '<P>k__BackingField'
.custom instance void [System.Runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
01 00 00 00
)
// Methods
.method public hidebysig specialname static
int32 get_P () cil managed
{
// Method begins at RVA 0x2050
// Code size 6 (0x6)
.maxstack 8
IL_0000: ldsfld int32 I::'<P>k__BackingField'
IL_0005: ret
} // end of method I::get_P
// Properties
.property int32 P()
{
.get int32 I::get_P()
}
} // end of class I
");
var @interface = comp.GetTypeByMetadataName("I");
Assert.Equal("System.Int32 I.<P>k__BackingField", @interface.GetFieldsToEmit().Single().ToTestDisplayString());
Assert.Empty(@interface.GetMembers().OfType<FieldSymbol>());
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact]
public void TestStaticInInterface_DefaultImplementationsOfInterfacesIsMissing()
{
var comp = CreateCompilation(@"
public interface I
{
public static int P { get => field; }
}
");
comp.MakeMemberMissing(SpecialMember.System_Runtime_CompilerServices_RuntimeFeature__DefaultImplementationsOfInterfaces);
var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics(
// (4,27): error CS8701: Target runtime doesn't support default interface implementation.
// public static int P { get => field; }
Diagnostic(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, "get").WithLocation(4, 27)
);
var @interface = comp.GetTypeByMetadataName("I");
Assert.Equal("System.Int32 I.<P>k__BackingField", @interface.GetFieldsToEmit().Single().ToTestDisplayString());
Assert.Empty(@interface.GetMembers().OfType<FieldSymbol>());
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact]
public void TestAbstract()
{
var comp = CreateCompilation(@"
public abstract class C
{
public abstract int P { get => field; }
}
"); // setting TargetFramework for DefaultImplementationsOfInterfaces to exist.

var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics(
// (4,29): error CS0500: 'C.P.get' cannot declare a body because it is marked abstract
// public abstract int P { get => field; }
Diagnostic(ErrorCode.ERR_AbstractHasBody, "get").WithArguments("C.P.get").WithLocation(4, 29)
);
var @class = comp.GetTypeByMetadataName("C");
Assert.Empty(@class.GetMembers().OfType<FieldSymbol>());
Assert.Empty(@class.GetFieldsToEmit());
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact]
public void TestExtern()
{
var comp = CreateCompilation(@"
public class C
{
public extern int P { get => field; }
}
");

var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics(
// (4,27): error CS0179: 'C.P.get' cannot be extern and declare a body
// public extern int P { get => field; }
Diagnostic(ErrorCode.ERR_ExternHasBody, "get").WithArguments("C.P.get").WithLocation(4, 27)
);
var @class = comp.GetTypeByMetadataName("C");
Assert.Empty(@class.GetMembers().OfType<FieldSymbol>());
Assert.Empty(@class.GetFieldsToEmit());
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact]
public void TestNameOfFieldInAttribute()
{
Expand Down