Skip to content

Commit

Permalink
Merge pull request dotnet#20142 from VSadov/readonlyFields
Browse files Browse the repository at this point in the history
Disallowing writeable state in readonly structs
  • Loading branch information
VSadov authored Jun 13, 2017
2 parents 2ce8fe0 + f338d96 commit ae941fc
Show file tree
Hide file tree
Showing 10 changed files with 312 additions and 2 deletions.
29 changes: 28 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

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

9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5123,4 +5123,13 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_TypeReserved" xml:space="preserve">
<value>The type name '{0}' is reserved to be used by the compiler.</value>
</data>
<data name="ERR_FieldsInRoStruct" xml:space="preserve">
<value>Instance fields of readonly structs must be readonly.</value>
</data>
<data name="ERR_AutoPropsInRoStruct" xml:space="preserve">
<value>Auto-implemented instance properties in readonly structs must be readonly.</value>
</data>
<data name="ERR_FieldlikeEventsInRoStruct" xml:space="preserve">
<value>Field-like events are not allowed in readonly structs.</value>
</data>
</root>
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 @@ -1508,5 +1508,10 @@ internal enum ErrorCode
ERR_RefReturnReadonlyNotField2 = 8411,
ERR_ExplicitReservedAttr = 8412,
ERR_TypeReserved = 8413,

//PROTOTYPE(ReadonlyRefs): make err IDs contiguous before merging to master.
ERR_FieldsInRoStruct = 8514,
ERR_AutoPropsInRoStruct = 8515,
ERR_FieldlikeEventsInRoStruct = 8516,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ internal SourceFieldLikeEventSymbol(SourceMemberContainerTypeSymbol containingTy
// Don't initialize this.type - we'll just use the type of the field (which is lazy and handles var)
}

if (!IsStatic && ContainingType.IsReadOnly)
{
diagnostics.Add(ErrorCode.ERR_FieldlikeEventsInRoStruct, this.Locations[0]);
}

// Accessors will assume that Type is available.
_addMethod = new SynthesizedFieldLikeEventAccessorSymbol(this, isAdder: true);
_removeMethod = new SynthesizedFieldLikeEventAccessorSymbol(this, isAdder: false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ protected void ReportModifiersDiagnostics(DiagnosticBag diagnostics)
{
diagnostics.Add(ErrorCode.ERR_InstanceMemberInStaticClass, ErrorLocation, this);
}
else if (!IsStatic && !IsReadOnly && containingType.IsReadOnly)
{
diagnostics.Add(ErrorCode.ERR_FieldsInRoStruct, ErrorLocation);
}

// TODO: Consider checking presence of core type System.Runtime.CompilerServices.IsVolatile
// if there is a volatile modifier. Perhaps an appropriate error should be reported if the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ private SourcePropertySymbol(
_isAutoProperty = notRegularProperty && hasGetSyntax;
bool isReadOnly = hasGetSyntax && setSyntax == null;

if (_isAutoProperty && !isReadOnly && !IsStatic && ContainingType.IsReadOnly)
{
diagnostics.Add(ErrorCode.ERR_AutoPropsInRoStruct, location);
}

if (_isAutoProperty || hasInitializer)
{
if (_isAutoProperty)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1300,7 +1300,7 @@ internal static bool IsSpanType(this TypeSymbol type)
}

// the "System" must be in the global namespace
return ns.ContainingNamespace.IsGlobalNamespace;
return ns.ContainingNamespace?.IsGlobalNamespace == true;
}

internal static bool IsNonGenericTaskType(this TypeSymbol type, CSharpCompilation compilation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
<Compile Include="Semantics\PatternMatchingTests_Global.cs" />
<Compile Include="Semantics\BindingAsyncTasklikeMoreTests.cs" />
<Compile Include="Semantics\BindingAsyncTasklikeTests.cs" />
<Compile Include="Semantics\ReadOnlyStructsTests.cs" />
<Compile Include="Semantics\TargetTypedDefaultTests.cs" />
<Compile Include="Semantics\DeconstructionTests.cs" />
<Compile Include="Semantics\ImportsTests.cs" />
Expand Down
253 changes: 253 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Symbols.Retargeting;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Semantics
{
[CompilerTrait(CompilerFeature.ReadOnlyReferences)]
public class ReadOnlyStructsTests : CompilingTestBase
{
[Fact()]
public void WriteableInstanceAutoPropsInRoStructs()
{
var text = @"
public readonly struct A
{
// ok - no state
int ro => 5;
// ok - ro state
int ro1 {get;}
// error
int rw {get; set;}
// ok - static
static int rws {get; set;}
}
";
CreateStandardCompilation(text).VerifyDiagnostics(
// (11,9): error CS8515: Auto-implemented instance properties in readonly structs must be readonly.
// int rw {get; set;}
Diagnostic(ErrorCode.ERR_AutoPropsInRoStruct, "rw").WithLocation(11, 9)
);
}

[Fact()]
public void WriteableInstanceFieldsInRoStructs()
{
var text = @"
public readonly struct A
{
// ok
public static int s;
// ok
public readonly int ro;
// error
int x;
void AssignField()
{
// error
this.x = 1;
A a = default;
// OK
a.x = 2;
}
}
";
CreateStandardCompilation(text).VerifyDiagnostics(
// (11,9): error CS8514: Instance fields of readonly structs must be readonly.
// int x;
Diagnostic(ErrorCode.ERR_FieldsInRoStruct, "x").WithLocation(11, 9),
// (16,9): error CS1604: Cannot assign to 'this' because it is read-only
// this.x = 1;
Diagnostic(ErrorCode.ERR_AssgReadonlyLocal, "this.x").WithArguments("this").WithLocation(16, 9)
);
}

[Fact()]
public void EventsInRoStructs()
{
var text = @"
using System;
public readonly struct A : I1
{
//error
public event System.Action e;
//error
public event Action ei1;
//ok
public static event Action es;
A(int arg)
{
// ok
e = () => { };
ei1 = () => { };
es = () => { };
// ok
M1(ref e);
}
//ok
event Action I1.ei2
{
add
{
throw new NotImplementedException();
}
remove
{
throw new NotImplementedException();
}
}
void AssignEvent()
{
// error
e = () => { };
// error
M1(ref e);
}
static void M1(ref System.Action arg)
{
}
}
interface I1
{
event System.Action ei1;
event System.Action ei2;
}
";
CreateStandardCompilation(text).VerifyDiagnostics(
// (7,32): error CS8516: Field-like events are not allowed in readonly structs.
// public event System.Action e;
Diagnostic(ErrorCode.ERR_FieldlikeEventsInRoStruct, "e").WithLocation(7, 32),
// (10,25): error CS8516: Field-like events are not allowed in readonly structs.
// public event Action ei1;
Diagnostic(ErrorCode.ERR_FieldlikeEventsInRoStruct, "ei1").WithLocation(10, 25),
// (43,9): error CS1604: Cannot assign to 'this' because it is read-only
// e = () => { };
Diagnostic(ErrorCode.ERR_AssgReadonlyLocal, "e").WithArguments("this").WithLocation(43, 9),
// (46,16): error CS1604: Cannot assign to 'this' because it is read-only
// M1(ref e);
Diagnostic(ErrorCode.ERR_AssgReadonlyLocal, "e").WithArguments("this").WithLocation(46, 16)
);
}

private static string ilreadonlyStructWithWriteableFieldIL = @"
.class private auto ansi sealed beforefieldinit Microsoft.CodeAnalysis.EmbeddedAttribute
extends [mscorlib]System.Attribute
{
.custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )
.custom instance void Microsoft.CodeAnalysis.EmbeddedAttribute::.ctor() = ( 01 00 00 00 )
.method public hidebysig specialname rtspecialname
instance void .ctor() cil managed
{
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [mscorlib]System.Attribute::.ctor()
IL_0006: ret
} // end of method EmbeddedAttribute::.ctor
} // end of class Microsoft.CodeAnalysis.EmbeddedAttribute
.class private auto ansi sealed beforefieldinit System.Runtime.CompilerServices.IsReadOnlyAttribute
extends [mscorlib]System.Attribute
{
.custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )
.custom instance void Microsoft.CodeAnalysis.EmbeddedAttribute::.ctor() = ( 01 00 00 00 )
.method public hidebysig specialname rtspecialname
instance void .ctor() cil managed
{
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [mscorlib]System.Attribute::.ctor()
IL_0006: ret
} // end of method IsReadOnlyAttribute::.ctor
} // end of class System.Runtime.CompilerServices.IsReadOnlyAttribute
.class public sequential ansi sealed beforefieldinit S1
extends [mscorlib]System.ValueType
{
.custom instance void System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 )
// WRITEABLE FIELD!!!
.field public int32 'field'
} // end of class S1
";

[Fact()]
public void UseWriteableInstanceFieldsInRoStructs()
{
var csharp = @"
public class Program
{
public static void Main()
{
S1 s = new S1();
s.field = 123;
System.Console.WriteLine(s.field);
}
}
";

var comp = CreateCompilationWithCustomILSource(csharp, ilreadonlyStructWithWriteableFieldIL, options:TestOptions.ReleaseExe);

comp.VerifyDiagnostics();

CompileAndVerify(comp, expectedOutput:"123");
}

[Fact()]
public void UseWriteableInstanceFieldsInRoStructsErr()
{
var csharp = @"
public class Program
{
static readonly S1 s = new S1();
public static void Main()
{
s.field = 123;
System.Console.WriteLine(s.field);
}
}
";

var comp = CreateCompilationWithCustomILSource(csharp, ilreadonlyStructWithWriteableFieldIL, options: TestOptions.ReleaseExe);

comp.VerifyDiagnostics(
// (8,9): error CS1650: Fields of static readonly field 'Program.s' cannot be assigned to (except in a static constructor or a variable initializer)
// s.field = 123;
Diagnostic(ErrorCode.ERR_AssgReadonlyStatic2, "s.field").WithArguments("Program.s").WithLocation(8, 9)
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests
/// <summary>
/// this place is dedicated to binding related error tests
/// </summary>
[CompilerTrait(CompilerFeature.ReadOnlyReferences)]
public class SpanStackSafetyTests : CompilingTestBase
{
private static string spanSource = @"
Expand Down

0 comments on commit ae941fc

Please sign in to comment.