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

Support initializers in semi auto property #58512

Closed
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -1563,6 +1563,7 @@ private BoundExpression BindIdentifier(
{
if (accessor.Property.CreateBackingFieldForFieldKeyword() is { } backingField)
{
backingField.BoundToFieldKeyword();
expression = BindNonMethod(node, backingField, diagnostics, LookupResultKind.Viable, indexed: false, isError: false);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ internal Conversions Conversions
/// <summary>
/// Optional data collected during testing only.
/// Used for instance for nullable analysis (<see cref="NullableWalker.NullableAnalysisData"/>)
/// and inferred delegate types (<see cref="InferredDelegateTypeData"/>).
/// and semi auto implemented properties (<see cref="SourcePropertySymbolBase.AccessorBindingData"/>
/// and inferred delegate types (<see cref="InferredDelegateTypeData"/>)
/// and semi auto implemented properties (<see cref="SourcePropertySymbolBase.AccessorBindingData"/>.
/// </summary>
internal object? TestOnlyCompilationData;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ internal void NoteBinding()
}
}


protected const string DefaultIndexerName = "Item";

/// <summary>
Expand Down Expand Up @@ -410,11 +409,12 @@ private void EnsureSignature()
/// If the backing field is unknown, set it to null.
/// </summary>
/// <remarks>
/// This should be called only if we're sure the backing field can't be non-null value.
/// This should be called only if we're sure the backing field can't become non-null value if it's not already.
/// </remarks>
internal void MarkBackingFieldAsCalculated()
{
Interlocked.CompareExchange(ref _lazyBackingFieldSymbol, null, _lazyBackingFieldSymbolSentinel);
((SynthesizedBackingFieldSymbol)_lazyBackingFieldSymbol)?.MarkIsBoundToFieldKeywordAsCalculated();
Copy link
Member Author

Choose a reason for hiding this comment

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

No tests currently cover this. I'll either remove it if it's redundant or add a test.

Copy link
Contributor

Choose a reason for hiding this comment

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

No tests currently cover this. I'll either remove it if it's redundant or add a test.

Let's try getting into a working mode when a change without a proper testing is not presented for a review. If you'd like to make a change that changes a behavior, try starting with a test first.

}

/// <summary>
Expand Down Expand Up @@ -450,7 +450,7 @@ private void EnsureBackingFieldIsSynthesized()
}

// If binding both the getter and setter didn't get a backing field, set it to null so that we don't re-calculate.
Interlocked.CompareExchange(ref _lazyBackingFieldSymbol, null, _lazyBackingFieldSymbolSentinel);
MarkBackingFieldAsCalculated();

void noteAccessorBinding()
{
Expand Down Expand Up @@ -751,6 +751,73 @@ internal bool IsAutoProperty
}

#nullable enable

internal bool IsSemiAutoProperty
{
get
{
var backingField = FieldKeywordBackingField;
if (backingField is null)
{
return false;
}

if (!backingField.IsEarlyConstructed)
{
Debug.Assert(backingField.IsBoundToFieldKeyword == ThreeState.True);
return true;
}

Debug.Assert((_propertyFlags & Flags.HasInitializer) != 0);
EnsureBoundToFieldKeywordIsCalculated();
return backingField.IsBoundToFieldKeyword.Value();
}
}

/// <summary>
/// Ensures that <see cref="_lazyBackingFieldSymbol" /> is set if necessary.
/// </summary>
/// <remarks>
/// This method *may* trigger binding to figure out if we have a field keyword in an accessor.
/// </remarks>
private void EnsureBoundToFieldKeywordIsCalculated()
{
var backingField = FieldKeywordBackingField;
Debug.Assert(backingField is not null);
if (backingField.IsBoundToFieldKeyword.HasValue())
{
return;
}

// PROTOTYPE(semi-auto-props): Refactor common code.
if (this is SourcePropertySymbol propertySymbol)
{
if (propertySymbol.GetMethod is SourcePropertyAccessorSymbol { ContainsFieldKeyword: true } getMethod)
{
noteAccessorBinding();
var binder = getMethod.TryGetBodyBinder();
binder?.BindMethodBody(getMethod.SyntaxNode, BindingDiagnosticBag.Discarded);
}

if (!backingField.IsBoundToFieldKeyword.HasValue() &&
propertySymbol.SetMethod is SourcePropertyAccessorSymbol { ContainsFieldKeyword: true } setMethod)
{
noteAccessorBinding();
setMethod.TryGetBodyBinder()?.BindMethodBody(setMethod.SyntaxNode, BindingDiagnosticBag.Discarded);
}
}

backingField.MarkIsBoundToFieldKeywordAsCalculated();

void noteAccessorBinding()
{
if (DeclaringCompilation.TestOnlyCompilationData is AccessorBindingData accessorBindingData)
{
accessorBindingData.NoteBinding();
}
}
}

/// <summary>
/// Backing field for automatically implemented property, or
/// for a property with an initializer.
Expand Down Expand Up @@ -825,7 +892,7 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions,
bool hasInitializer = (_propertyFlags & Flags.HasInitializer) != 0;
if (hasInitializer)
{
CheckInitializer(IsAutoProperty, ContainingType.IsInterface, IsStatic, Location, diagnostics);
CheckInitializer(IsAutoProperty || IsSemiAutoProperty, ContainingType.IsInterface, IsStatic, Location, diagnostics);
}

if (IsAutoPropertyWithGetAccessor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

#nullable disable

using System;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Emit;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
Expand All @@ -18,11 +20,34 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
/// </summary>
internal sealed class SynthesizedBackingFieldSymbol : FieldSymbolWithAttributesAndModifiers
{
[Flags]
private enum Flags
{
HasInitializer = 1 << 0,
IsCreatedForFieldKeyword = 1 << 1,
IsEarlyConstructed = 1 << 2,
}

private readonly SourcePropertySymbolBase _property;
private readonly string _name;
internal bool HasInitializer { get; }
internal bool IsCreatedForFieldKeyword { get; }
internal bool IsEarlyConstructed { get; }
private readonly Flags _backingFieldFlags;
private int _lazyIsBoundToFieldKeyword;

internal bool HasInitializer => (_backingFieldFlags & Flags.HasInitializer) != 0;

/// <summary>
/// We set <see cref="IsCreatedForFieldKeyword" /> to true when early constructing a property with initializer.
/// <c>field</c> may or may not be a keyword.
/// </summary>
internal bool IsCreatedForFieldKeyword => (_backingFieldFlags & Flags.IsCreatedForFieldKeyword) != 0; // PROTOTYPE(semi-auto-props): Find better names?

/// <summary>
/// Determines whether <c>field</c> is actually the keyword
/// </summary>
internal ThreeState IsBoundToFieldKeyword => (ThreeState)_lazyIsBoundToFieldKeyword;

internal bool IsEarlyConstructed => (_backingFieldFlags & Flags.IsEarlyConstructed) != 0;

protected override DeclarationModifiers Modifiers { get; }

public SynthesizedBackingFieldSymbol(
Expand All @@ -43,14 +68,42 @@ public SynthesizedBackingFieldSymbol(
(isStatic ? DeclarationModifiers.Static : DeclarationModifiers.None);

_property = property;
HasInitializer = hasInitializer;
IsCreatedForFieldKeyword = isCreatedForfieldKeyword;
IsEarlyConstructed = isEarlyConstructed;
if (hasInitializer)
{
_backingFieldFlags |= Flags.HasInitializer;
}

if (isCreatedForfieldKeyword)
{
_backingFieldFlags |= Flags.IsCreatedForFieldKeyword;
}


if (isEarlyConstructed)
{
_backingFieldFlags |= Flags.IsEarlyConstructed;
}
else
{
_lazyIsBoundToFieldKeyword = (int)ThreeState.True;
}

// If it's not early constructed, it must have been created for field keyword.
Debug.Assert(IsEarlyConstructed || IsCreatedForFieldKeyword);
}

internal void BoundToFieldKeyword()
{
Debug.Assert(IsCreatedForFieldKeyword);
Debug.Assert(_lazyIsBoundToFieldKeyword is (int)ThreeState.Unknown or (int)ThreeState.True);
_lazyIsBoundToFieldKeyword = (int)ThreeState.True;
}

internal void MarkIsBoundToFieldKeywordAsCalculated()
{
Interlocked.CompareExchange(ref _lazyIsBoundToFieldKeyword, (int)ThreeState.False, (int)ThreeState.Unknown);
}

protected override IAttributeTargetSymbol AttributeOwner
=> _property.AttributesOwner;

Expand Down
Loading