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

Readonly members emit changes #33954

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 src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2990,7 +2990,7 @@ internal static bool HasHome(
return true;
}

if (!IsAnyReadOnly(addressKind) && type.IsReadOnly)
if (!IsAnyReadOnly(addressKind) && method.IsEffectivelyReadOnly)
{
return method.MethodKind == MethodKind.Constructor;
}
Expand Down
7 changes: 7 additions & 0 deletions src/Compilers/CSharp/Portable/CodeGen/EmitAddress.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ private LocalDefinition EmitAddress(BoundExpression expression, AddressKind addr

if (expression.Type.IsValueType)
{

if (!HasHome(expression, addressKind))
{
// a readonly method is calling a non-readonly method, therefore we need to copy 'this'
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
goto default;
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
}

_builder.EmitLoadArgumentOpcode(0);
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1688,7 +1688,7 @@ private bool IsReadOnlyCall(MethodSymbol method, NamedTypeSymbol methodContainin
{
Debug.Assert(methodContainingType.IsVerifierValue(), "only struct calls can be readonly");

if (methodContainingType.IsReadOnly && method.MethodKind != MethodKind.Constructor)
if (method.IsEffectivelyReadOnly && method.MethodKind != MethodKind.Constructor)
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ public sealed override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementat
get { return ImmutableArray<MethodSymbol>.Empty; }
}

internal sealed override bool IsDeclaredReadOnly => true;

public sealed override ImmutableArray<CustomModifier> RefCustomModifiers
{
get { return ImmutableArray<CustomModifier>.Empty; }
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
get { return ImmutableArray<MethodSymbol>.Empty; }
}

internal override bool IsDeclaredReadOnly => false;

internal override int ParameterCount
{
get { return 0; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,9 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
}
}

// PROTOTYPE: need to round trip readonly attribute in metadata
internal override bool IsDeclaredReadOnly => false;

public override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken))
{
return PEDocumentationCommentUtils.GetDocumentationComment(this, _containingType.ContainingPEModule, preferredCulture, cancellationToken, ref AccessUncommonFields()._lazyDocComment);
Expand Down
15 changes: 15 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,21 @@ internal virtual bool IsExplicitInterfaceImplementation
get { return ExplicitInterfaceImplementations.Any(); }
}

// PROTOTYPE: this should become public API
/// <summary>
/// Indicates whether the method is declared readonly, i.e.
/// whether 'this' is readonly in the scope of the method.
/// See also <see cref="IsEffectivelyReadOnly"/>
/// </summary>
internal abstract bool IsDeclaredReadOnly { get; }

// PROTOTYPE: this should become public API
/// <summary>
/// Indicates whether the method is effectively readonly,
/// by either the method or the containing type being marked readonly.
/// </summary>
internal bool IsEffectivelyReadOnly => IsDeclaredReadOnly || ContainingType?.IsReadOnly == true;

/// <summary>
/// Returns interface methods explicitly implemented by this method.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,9 @@ internal override bool IsExplicitInterfaceImplementation
get { return false; }
}

// extension methods are static and therefore not readonly
internal override bool IsDeclaredReadOnly => false;

public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
{
get { return ImmutableArray<MethodSymbol>.Empty; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ internal override bool IsMetadataFinal
}
}

internal override bool IsDeclaredReadOnly => false;

internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree) { throw ExceptionUtilities.Unreachable; }

#endregion
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Source/LambdaSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,8 @@ internal override bool GenerateDebugInfo
get { return true; }
}

internal override bool IsDeclaredReadOnly => false;

public override ImmutableArray<TypeParameterConstraintClause> GetTypeParameterConstraintClauses(bool early) => ImmutableArray<TypeParameterConstraintClause>.Empty;

internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,8 @@ internal override TypeSymbol IteratorElementType

internal bool IsExpressionBodied => _syntax.Body == null && _syntax.ExpressionBody != null;

internal override bool IsDeclaredReadOnly => false;

public override DllImportData GetDllImportData() => null;

internal override ImmutableArray<string> GetAppliedConditionalSymbols() => ImmutableArray<string>.Empty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ public sealed override bool IsAsync
}
}

internal bool IsReadOnly
internal override bool IsDeclaredReadOnly
{
get
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ private void CheckModifiers(bool hasBody, Location location, DiagnosticBag diagn
// The modifier '{0}' is not valid for this item
diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.VirtualKeyword));
}
else if (IsStatic && IsReadOnly)
else if (IsStatic && IsDeclaredReadOnly)
{
// The modifier '{0}' is not valid for this item
diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ private void CheckModifiers(Location location, bool hasBody, bool isAutoProperty
{
diagnostics.Add(AccessCheck.GetProtectedMemberInSealedTypeError(ContainingType), location, this);
}
else if (IsStatic && IsReadOnly && !_property.HasReadOnlyModifier)
else if (IsStatic && IsDeclaredReadOnly && !_property.HasReadOnlyModifier)
{
// The modifier '{0}' is not valid for this item
diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
get { return ImmutableArray<MethodSymbol>.Empty; }
}

internal sealed override bool IsDeclaredReadOnly => false;

internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false)
{
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,8 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
get { return ImmutableArray<MethodSymbol>.Empty; }
}

internal sealed override bool IsDeclaredReadOnly => false;

internal override bool SynthesizesLoweredBoundBody
{
get { return true; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,7 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l
{
throw ExceptionUtilities.Unreachable;
}

internal override bool IsDeclaredReadOnly => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
}
}

internal override bool IsDeclaredReadOnly => true;

public override ImmutableArray<CustomModifier> RefCustomModifiers
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,8 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
}
}

internal override bool IsDeclaredReadOnly => false;

public sealed override bool IsImplicitlyDeclared
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,5 +320,7 @@ internal override bool GenerateDebugInfo
return UnderlyingMethod.GenerateDebugInfo;
}
}

internal override bool IsDeclaredReadOnly => UnderlyingMethod.IsDeclaredReadOnly;
}
}
Loading