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

Add support for init-only members #43275

Merged
merged 22 commits into from
Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion docs/contributing/Compiler Test Plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ This document provides guidance for thinking about language interactions and tes
- Partial method
- Named and optional parameters
- String interpolation
- Properties (read-write, read-only, write-only, auto-property, expression-bodied)
- Properties (read-write, read-only, init-only, write-only, auto-property, expression-bodied)
- Interfaces (implicit vs. explicit interface member implementation)
- Delegates
- Multi-declaration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,9 @@ private static void ComputeDeclarations(

case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
{
var t = (AccessorDeclarationSyntax)node;
var blocks = ArrayBuilder<SyntaxNode>.GetInstance();
Expand Down
58 changes: 56 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,8 @@ private bool CheckFieldValueKind(SyntaxNode node, BoundFieldAccess fieldAccess,
{
MethodSymbol containingMethod = (MethodSymbol)containing;
MethodKind desiredMethodKind = fieldIsStatic ? MethodKind.StaticConstructor : MethodKind.Constructor;
canModifyReadonly = containingMethod.MethodKind == desiredMethodKind;
canModifyReadonly = (containingMethod.MethodKind == desiredMethodKind) ||
isAssignedFromInitOnlySetterOnThis(fieldAccess.ReceiverOpt);
}
else if (containing.Kind == SymbolKind.Field)
{
Expand Down Expand Up @@ -752,6 +753,23 @@ private bool CheckFieldValueKind(SyntaxNode node, BoundFieldAccess fieldAccess,

// for other fields defer to the receiver.
return CheckIsValidReceiverForVariable(node, fieldAccess.ReceiverOpt, valueKind, diagnostics);

bool isAssignedFromInitOnlySetterOnThis(BoundExpression receiver)
{
// bad: other.readonlyField = ...
// bad: base.readonlyField = ...
if (!(receiver is BoundThisReference))
Copy link
Contributor

@AlekseyTs AlekseyTs Apr 26, 2020

Choose a reason for hiding this comment

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

if (!(receiver is BoundThisReference)) [](start = 16, length = 38)

It looks like the speclet doesn't have this restriction. Is only says: "Assign readonly fields declared on the same type". However, I think the intention was to have this restriction. Please make sure to reconcile the difference. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated speclet


In reply to: 415209113 [](ancestors = 415209113)

{
return false;
}

if (!(ContainingMemberOrLambda is MethodSymbol method))
{
return false;
}

return method.IsInitOnly;
}
}

private bool CheckSimpleAssignmentValueKind(SyntaxNode node, BoundAssignmentOperator assignment, BindValueKind valueKind, DiagnosticBag diagnostics)
Expand Down Expand Up @@ -977,7 +995,7 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
{
var setMethod = propertySymbol.GetOwnOrInheritedSetMethod();

if ((object)setMethod == null)
if (setMethod is null)
{
var containing = this.ContainingMemberOrLambda;
if (!AccessingAutoPropertyFromConstructor(receiver, propertySymbol, containing))
Expand All @@ -988,6 +1006,13 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
}
else
{
if (setMethod.IsInitOnly &&
!isAllowedInitOnlySet(receiver))
{
Error(diagnostics, ErrorCode.ERR_AssignmentInitOnly, node, propertySymbol);
return false;
}

var accessThroughType = this.GetAccessThroughType(receiver);
bool failedThroughTypeCheck;
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
Expand Down Expand Up @@ -1076,6 +1101,35 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
}

return true;

bool isAllowedInitOnlySet(BoundExpression receiver)
{
// ok: new C() { InitOnlyProperty = ... }
if (receiver is BoundObjectOrCollectionValuePlaceholder)
{
return true;
}

// bad: other.InitOnlyProperty = ...
if (!(receiver is BoundThisReference || receiver is BoundBaseReference))
{
return false;
}

var containingMember = ContainingMemberOrLambda;
if (!(containingMember is MethodSymbol method))
{
return false;
}

if (method.MethodKind == MethodKind.Constructor || method.IsInitOnly)
{
// ok: setting on `this` or `base` from an instance constructor or init-only setter
return true;
}

return false;
}
}

private bool IsBadBaseAccess(SyntaxNode node, BoundExpression receiverOpt, Symbol member, DiagnosticBag diagnostics,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public override Binder VisitAccessorDeclaration(AccessorDeclarationSyntax parent
var propertySymbol = GetPropertySymbol((BasePropertyDeclarationSyntax)propertyOrEventDecl, resultBinder);
if ((object)propertySymbol != null)
{
accessor = (parent.Kind() == SyntaxKind.SetAccessorDeclaration) ? propertySymbol.SetMethod : propertySymbol.GetMethod;
accessor = (parent.Kind() == SyntaxKind.GetAccessorDeclaration) ? propertySymbol.GetMethod : propertySymbol.SetMethod;
}
break;
}
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,7 @@ private static ImmutableArray<Symbol> PerformCrefOverloadResolution(ArrayBuilder
containingType: null,
name: null,
refKind: RefKind.None,
isInitOnly: false,
returnType: default,
refCustomModifiers: ImmutableArray<CustomModifier>.Empty,
explicitInterfaceImplementations: ImmutableArray<MethodSymbol>.Empty);
Expand Down
13 changes: 11 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1442,9 +1442,18 @@ internal static bool ReportUseSiteDiagnostics(Symbol symbol, DiagnosticBag diagn
/// </summary>
internal NamedTypeSymbol GetWellKnownType(WellKnownType type, DiagnosticBag diagnostics, SyntaxNode node)
{
NamedTypeSymbol typeSymbol = this.Compilation.GetWellKnownType(type);
return GetWellKnownType(this.Compilation, type, diagnostics, node.Location);
}

/// <summary>
/// This is a layer on top of the Compilation version that generates a diagnostic if the well-known
/// type isn't found.
/// </summary>
internal static NamedTypeSymbol GetWellKnownType(CSharpCompilation compilation, WellKnownType type, DiagnosticBag diagnostics, Location location)
{
NamedTypeSymbol typeSymbol = compilation.GetWellKnownType(type);
Debug.Assert((object)typeSymbol != null, "Expect an error type if well-known type isn't found");
ReportUseSiteDiagnostics(typeSymbol, diagnostics, node);
ReportUseSiteDiagnostics(typeSymbol, diagnostics, location);
return typeSymbol;
}

Expand Down
20 changes: 19 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,9 @@
<data name="ERR_BadMemberFlag" xml:space="preserve">
<value>The modifier '{0}' is not valid for this item</value>
</data>
<data name="ERR_BadInitAccessor" xml:space="preserve">
<value>The 'init' accessor is not valid on static members</value>
</data>
<data name="ERR_BadMemberProtection" xml:space="preserve">
<value>More than one protection modifier</value>
</data>
Expand Down Expand Up @@ -1592,6 +1595,9 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_ExplicitPropertyAddingAccessor" xml:space="preserve">
<value>'{0}' adds an accessor not found in interface member '{1}'</value>
</data>
<data name="ERR_ExplicitPropertyMismatchInitOnly" xml:space="preserve">
<value>Accessors '{0}' and '{1}' should both be init-only or neither</value>
</data>
<data name="ERR_ExplicitPropertyMissingAccessor" xml:space="preserve">
<value>Explicit interface implementation '{0}' is missing accessor '{1}'</value>
</data>
Expand Down Expand Up @@ -2891,7 +2897,7 @@ A catch() block after a catch (System.Exception e) block can catch non-CLS excep
<value>Members of readonly field '{0}' cannot be used as a ref or out value (except in a constructor)</value>
</data>
<data name="ERR_AssgReadonly" xml:space="preserve">
<value>A readonly field cannot be assigned to (except in a constructor of the class in which the field is defined or a variable initializer))</value>
<value>A readonly field cannot be assigned to (except in a constructor or init-only setter of the class in which the field is defined or a variable initializer))</value>
</data>
<data name="ERR_AssgReadonly2" xml:space="preserve">
<value>Members of readonly field '{0}' cannot be modified (except in a constructor or a variable initializer)</value>
Expand Down Expand Up @@ -4918,6 +4924,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_CantChangeRefReturnOnOverride" xml:space="preserve">
<value>'{0}' must match by reference return of overridden member '{1}'</value>
</data>
<data name="ERR_CantChangeInitOnlyOnOverride" xml:space="preserve">
<value>'{0}' must match by init-only of overridden member '{1}'</value>
</data>
<data name="ERR_MustNotHaveRefReturn" xml:space="preserve">
<value>By-reference returns may only be used in methods that return by reference</value>
</data>
Expand All @@ -4930,6 +4939,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_CloseUnimplementedInterfaceMemberWrongRefReturn" xml:space="preserve">
<value>'{0}' does not implement interface member '{1}'. '{2}' cannot implement '{1}' because it does not have matching return by reference.</value>
</data>
<data name="ERR_CloseUnimplementedInterfaceMemberWrongInitOnly" xml:space="preserve">
<value>'{0}' does not implement interface member '{1}'. '{2}' cannot implement '{1}'.</value>
</data>
<data name="ERR_BadIteratorReturnRef" xml:space="preserve">
<value>The body of '{0}' cannot be an iterator block because '{0}' returns by reference</value>
</data>
Expand Down Expand Up @@ -6061,6 +6073,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeatureRecords" xml:space="preserve">
<value>records</value>
</data>
<data name="IDS_FeatureInitOnlySetters" xml:space="preserve">
<value>init-only setters</value>
</data>
<data name="ERR_BadRecordDeclaration" xml:space="preserve">
<value>Records must have both a 'data' modifier and non-empty parameter list</value>
</data>
Expand All @@ -6079,4 +6094,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_GeneratorFailedDuringInitialization_Title" xml:space="preserve">
<value>Generator failed to initialize.</value>
</data>
<data name="ERR_AssignmentInitOnly" xml:space="preserve">
<value>Init-only property or indexer '{0}' can only be assigned in an object initializer, or on 'this' or 'base' in an instance constructor or an 'init' accessor.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -2098,6 +2098,7 @@ internal protected virtual CSharpSyntaxNode GetBindableSyntaxNode(CSharpSyntaxNo
{
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
case SyntaxKind.MethodDeclaration:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ internal override BoundNode Bind(Binder binder, CSharpSyntaxNode node, Diagnosti
case SyntaxKind.DestructorDeclaration:
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
return binder.BindMethodBody(node, diagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,7 @@ private MemberSemanticModel GetMemberModel(int position)
case SyntaxKind.RemoveAccessorDeclaration:
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
// NOTE: not UnknownAccessorDeclaration since there's no corresponding method symbol from which to build a member model.
outsideMemberDecl = !LookupPosition.IsInBody(position, (AccessorDeclarationSyntax)memberDecl);
break;
Expand Down Expand Up @@ -836,6 +837,7 @@ internal override MemberSemanticModel GetMemberModel(SyntaxNode node)

case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
// NOTE: not UnknownAccessorDeclaration since there's no corresponding method symbol from which to build a member model.
Expand Down Expand Up @@ -1034,6 +1036,7 @@ private MemberSemanticModel CreateMemberModel(CSharpSyntaxNode node)
}
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
{
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1779,6 +1779,11 @@ internal enum ErrorCode

ERR_BadRecordDeclaration = 8800,
ERR_DuplicateRecordConstructor = 8801,
ERR_AssignmentInitOnly = 8802,
ERR_CantChangeInitOnlyOnOverride = 8803,
ERR_CloseUnimplementedInterfaceMemberWrongInitOnly = 8804,
ERR_ExplicitPropertyMismatchInitOnly = 8805,
ERR_BadInitAccessor = 8806,

// Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)
}
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,11 @@ internal enum MessageID
IDS_FeatureLocalFunctionAttributes = MessageBase + 12766,
IDS_FeatureExternLocalFunctions = MessageBase + 12767,
IDS_FeatureMemberNotNull = MessageBase + 12768,

IDS_FeatureNativeInt = MessageBase + 12769,
IDS_FeatureTargetTypedObjectCreation = MessageBase + 12770,
IDS_FeatureRecords = MessageBase + 12771,
IDS_FeatureInitOnlySetters = MessageBase + 12772,
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -307,6 +309,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
case MessageID.IDS_FeatureMemberNotNull:
case MessageID.IDS_FeatureRecords:
case MessageID.IDS_FeatureNativeInt:
case MessageID.IDS_FeatureInitOnlySetters: // semantic check
return LanguageVersion.Preview;

// C# 8.0 features.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ private static SyntaxNode MethodDeclarationIfAvailable(SyntaxNode body)
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
case SyntaxKind.ConstructorDeclaration:
case SyntaxKind.OperatorDeclaration:

Expand All @@ -589,6 +590,7 @@ private static Text.TextSpan SkipAttributes(SyntaxNode syntax)

case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
AccessorDeclarationSyntax accessorSyntax = (AccessorDeclarationSyntax)syntax;
return SkipAttributes(syntax, accessorSyntax.AttributeLists, accessorSyntax.Modifiers, accessorSyntax.Keyword, null);

Expand Down
Loading