Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Youssef1313 committed Apr 23, 2022
1 parent 6379850 commit 6840e22
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 26 deletions.
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(BindingDiagnosticBag diagnostics)
internal virtual FieldSymbol? GetSymbolForPossibleFieldKeyword()
{
RoslynDebug.Assert(Next is object);
return this.Next.GetSymbolForPossibleFieldKeyword(diagnostics);
return this.Next.GetSymbolForPossibleFieldKeyword();
}

/// <summary>
Expand Down
6 changes: 5 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1563,13 +1563,17 @@ private BoundExpression BindIdentifier(
else if (node.Identifier.ContextualKind() == SyntaxKind.FieldKeyword &&
ContainingMember().CanHaveFieldKeywordBackingField())
{
if (GetSymbolForPossibleFieldKeyword(diagnostics) is { } backingField)
if (GetSymbolForPossibleFieldKeyword() is { } backingField)
{
expression = BindNonMethod(node, backingField, diagnostics, LookupResultKind.Viable, indexed: false, isError: false);
if (IsInsideNameof)
{
Error(diagnostics, ErrorCode.ERR_FieldKeywordInsideNameOf, node);
}
else if (backingField.ContainingType.IsInterface && !backingField.IsStatic)
{
Error(diagnostics, ErrorCode.ERR_InterfacesCantContainFields, node);
}
}
}
}
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(BindingDiagnosticBag diagnostics)
internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
{
// There's supposed to either be a FieldKeywordBinder or a SpeculativeFieldKEywordBinder in the chain.
throw ExceptionUtilities.Unreachable;
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/FieldKeywordBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ internal FieldKeywordBinder(SourcePropertyAccessorSymbol accessor, Binder next)
_accessor = accessor;
}

internal override FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
{
return _accessor.Property.GetOrCreateBackingFieldForFieldKeyword(diagnostics);
return _accessor.Property.GetOrCreateBackingFieldForFieldKeyword();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal SpeculativeFieldKeywordBinder(SourcePropertyAccessorSymbol accessor, Bi
_accessor = accessor;
}

internal override FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
{
// 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, diagnostics);
GetOrCreateBackingField(isCreatedForFieldKeyword: hasInitializer && !isAutoProperty, isEarlyConstructed: true);
}

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

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

return (SynthesizedBackingFieldSymbol?)_lazyBackingFieldSymbol;
}

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

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

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


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

if (ContainingType.IsInterface && !isStatic)
{
diagnostics.Add(ErrorCode.ERR_InterfacesCantContainFields, ErrorLocation);
}

// 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,26 +82,85 @@ 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.
[Fact]
public void TestInInterface()
{
var comp = CreateCompilation(@"
public interface I
{
public int P { get => field; }
public int P1 { get => field; }
public int P2 { get => field; set => field = value; }
public int P3 { get { _ = field; return field; } set => field = value; }
public int P4 { get => field; } = 0;
public int P5 { get => field; set => field = value; } = 0;
public int P6 { get { _ = field; return field; } set => field = value; } = 0;
}
", 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)
// (4,28): error CS0525: Interfaces cannot contain instance fields
// public int P1 { get => field; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(4, 28),
// (6,28): error CS0525: Interfaces cannot contain instance fields
// public int P2 { get => field; set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(6, 28),
// (6,42): error CS0525: Interfaces cannot contain instance fields
// public int P2 { get => field; set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(6, 42),
// (8,31): error CS0525: Interfaces cannot contain instance fields
// public int P3 { get { _ = field; return field; } set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(8, 31),
// (8,45): error CS0525: Interfaces cannot contain instance fields
// public int P3 { get { _ = field; return field; } set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(8, 45),
// (8,61): error CS0525: Interfaces cannot contain instance fields
// public int P3 { get { _ = field; return field; } set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(8, 61),
// (10,16): error CS8053: Instance properties in interfaces cannot have initializers.
// public int P4 { get => field; } = 0;
Diagnostic(ErrorCode.ERR_InstancePropertyInitializerInInterface, "P4").WithArguments("I.P4").WithLocation(10, 16),
// (10,28): error CS0525: Interfaces cannot contain instance fields
// public int P4 { get => field; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(10, 28),
// (12,16): error CS8053: Instance properties in interfaces cannot have initializers.
// public int P5 { get => field; set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InstancePropertyInitializerInInterface, "P5").WithArguments("I.P5").WithLocation(12, 16),
// (12,28): error CS0525: Interfaces cannot contain instance fields
// public int P5 { get => field; set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(12, 28),
// (12,42): error CS0525: Interfaces cannot contain instance fields
// public int P5 { get => field; set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(12, 42),
// (14,16): error CS8053: Instance properties in interfaces cannot have initializers.
// public int P6 { get { _ = field; return field; } set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InstancePropertyInitializerInInterface, "P6").WithArguments("I.P6").WithLocation(14, 16),
// (14,31): error CS0525: Interfaces cannot contain instance fields
// public int P6 { get { _ = field; return field; } set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(14, 31),
// (14,45): error CS0525: Interfaces cannot contain instance fields
// public int P6 { get { _ = field; return field; } set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(14, 45),
// (14,61): error CS0525: Interfaces cannot contain instance fields
// public int P6 { get { _ = field; return field; } set => field = value; } = 0;
Diagnostic(ErrorCode.ERR_InterfacesCantContainFields, "field").WithLocation(14, 61)
);
var @interface = comp.GetTypeByMetadataName("I");
Assert.Empty(@interface.GetMembers().OfType<FieldSymbol>());
Assert.Equal("System.Int32 I.<P>k__BackingField", @interface.GetFieldsToEmit().Single().ToTestDisplayString());
var fieldsToEmit = @interface.GetFieldsToEmit().ToArray();
Assert.Equal(6, fieldsToEmit.Length);
Assert.Equal("System.Int32 I.<P1>k__BackingField", fieldsToEmit[0].ToTestDisplayString());
Assert.Equal("System.Int32 I.<P2>k__BackingField", fieldsToEmit[1].ToTestDisplayString());
Assert.Equal("System.Int32 I.<P3>k__BackingField", fieldsToEmit[2].ToTestDisplayString());
Assert.Equal("System.Int32 I.<P4>k__BackingField", fieldsToEmit[3].ToTestDisplayString());
Assert.Equal("System.Int32 I.<P5>k__BackingField", fieldsToEmit[4].ToTestDisplayString());
Assert.Equal("System.Int32 I.<P6>k__BackingField", fieldsToEmit[5].ToTestDisplayString());
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

Expand Down

0 comments on commit 6840e22

Please sign in to comment.