Skip to content

Commit

Permalink
Add support for init-only members (#43275)
Browse files Browse the repository at this point in the history
  • Loading branch information
jcouv authored Apr 30, 2020
1 parent 11201fd commit f423b85
Show file tree
Hide file tree
Showing 83 changed files with 4,655 additions and 364 deletions.
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))
{
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

0 comments on commit f423b85

Please sign in to comment.