diff --git a/docs/features/readonly-instance-members.md b/docs/features/readonly-instance-members.md new file mode 100644 index 0000000000000..aaaf6b786295c --- /dev/null +++ b/docs/features/readonly-instance-members.md @@ -0,0 +1,181 @@ +# Readonly Instance Members + +Championed Issue: + +## Summary +[summary]: #summary + +Provide a way to specify individual instance members on a struct do not modify state, in the same way that `readonly struct` specifies no instance members modify state. + +It is worth noting that `readonly instance member` != `pure instance member`. A `pure` instance member guarantees no state will be modified. A `readonly` instance member only guarantees that instance state will not be modified. + +All instance members on a `readonly struct` are implicitly `readonly instance members`. Explicit `readonly instance members` declared on non-readonly structs would behave in the same manner. For example, they would still create hidden copies if you called an instance member (on the current instance or on a field of the instance) which was itself not-readonly. + +## Design +[design]: #design + +Allow a user to specify that an instance member is, itself, `readonly` and does not modify the state of the instance (with all the appropriate verification done by the compiler, of course). For example: + +```csharp +public struct Vector2 +{ + public float x; + public float y; + + public readonly float GetLengthReadonly() + { + return MathF.Sqrt(LengthSquared); + } + + public float GetLength() + { + return MathF.Sqrt(LengthSquared); + } + + public readonly float GetLengthIllegal() + { + var tmp = MathF.Sqrt(LengthSquared); + + x = tmp; // Compiler error, cannot write x + y = tmp; // Compiler error, cannot write y + + return tmp; + } + + public float LengthSquared + { + readonly get + { + return (x * x) + + (y * y); + } + } +} + +public static class MyClass +{ + public static float ExistingBehavior(in Vector2 vector) + { + // This code causes a hidden copy, the compiler effectively emits: + // var tmpVector = vector; + // return tmpVector.GetLength(); + // + // This is done because the compiler doesn't know that `GetLength()` + // won't mutate `vector`. + + return vector.GetLength(); + } + + public static float ReadonlyBehavior(in Vector2 vector) + { + // This code is emitted exactly as listed. There are no hidden + // copies as the `readonly` modifier indicates that the method + // won't mutate `vector`. + + return vector.GetLengthReadonly(); + } +} +``` + +Readonly can be applied to property accessors to indicate that `this` will not be mutated in the accessor. + +```csharp +public int Prop +{ + readonly get + { + return this._prop1; + } +} +``` + +When `readonly` is applied to the property syntax, it means that all accessors are `readonly`. + +```csharp +public readonly int Prop +{ + get + { + return this._store["Prop2"]; + } + set + { + this._store["Prop2"] = value; + } +} +``` + +Similar to the rules for property accessibility modifiers, redundant `readonly` modifiers are not allowed on properties. + +```csharp +public readonly int Prop1 { readonly get => 42; } // Not allowed +public int Prop2 { readonly get => this._store["Prop2"]; readonly set => this._store["Prop2"]; } // Not allowed +``` + +Readonly can only be applied to accessors which do not mutate the containing type. + +```csharp +public int Prop +{ + readonly get + { + return this._prop3; + } + set + { + this._prop3 = value; + } +} +``` + +### Auto-properties +Readonly cannot be applied to auto-implemented properties or their accessors. The compiler will treat all auto-implemented getters as readonly. + +### Events +Readonly can be applied to manually-implemented events, but not field-like events. Readonly cannot be applied to individual event accessors (add/remove). + +```csharp +// Allowed +public readonly event Action Event1 +{ + add { } + remove { } +} + +// Not allowed +public readonly event Action Event2; +public event Action Event3 +{ + readonly add { } + remove { } +} +public static readonly event Event4 +{ + add { } + remove { } +} +``` + +Some other syntax examples: + +* Expression bodied members: `public readonly float ExpressionBodiedMember => (x * x) + (y * y);` +* Generic constraints: `public readonly void GenericMethod(T value) where T : struct { }` + +The compiler would emit the instance member, as usual, and would additionally emit a compiler recognized attribute indicating that the instance member does not modify state. This effectively causes the hidden `this` parameter to become `in T` instead of `ref T`. + +This would allow the user to safely call said instance method without the compiler needing to make a copy. + +The restrictions would include: + +* The `readonly` modifier cannot be applied to static methods, constructors or destructors. +* The `readonly` modifier cannot be applied to delegates. +* The `readonly` modifier cannot be applied to members of class or interface. + +## Compiler API + +The following public API will be added: + +- `bool IMethodSymbol.IsDeclaredReadOnly { get; }` indicates that the member is readonly due to having a readonly keyword, or due to being an auto-implemented instance getter on a struct. +- `bool IMethodSymbol.IsEffectivelyReadOnly { get; }` indicates that the member is readonly due to IsDeclaredReadOnly, or by the containing type being readonly, except if this method is a constructor. + +It may be necessary to add additional public API to properties and events. This poses a challenge for properties because `ReadOnly` properties have an existing, different meaning in VB.NET and the `IMethodSymbol.IsReadOnly` API has already shipped to describe that scenario. This specific issue is tracked in https://github.com/dotnet/roslyn/issues/34213. diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs index b5b4750c35397..b3bbeeee01ef7 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs @@ -42,7 +42,7 @@ private struct PackedFlags { // We currently pack everything into a 32-bit int with the following layout: // - // | m|l|k|j|i|h|g|f|e|d|c|b|aaaaa| + // | n|m|l|k|j|i|h|g|f|e|d|c|b|aaaaa| // // a = method kind. 5 bits. // b = method kind populated. 1 bit. @@ -58,7 +58,9 @@ private struct PackedFlags // j = isUseSiteDiagnostic populated. 1 bit // k = isConditional populated. 1 bit // l = isOverriddenOrHiddenMembers populated. 1 bit - // 16 bits remain for future purposes. + // m = isReadOnly. 1 bit. + // n = isReadOnlyPopulated. 1 bit. + // 14 bits remain for future purposes. private const int MethodKindOffset = 0; @@ -75,6 +77,8 @@ private struct PackedFlags private const int IsUseSiteDiagnosticPopulatedBit = 0x1 << 13; private const int IsConditionalPopulatedBit = 0x1 << 14; private const int IsOverriddenOrHiddenMembersPopulatedBit = 0x1 << 15; + private const int IsReadOnlyBit = 0x1 << 16; + private const int IsReadOnlyPopulatedBit = 0x1 << 17; private int _bits; @@ -103,6 +107,8 @@ public MethodKind MethodKind public bool IsUseSiteDiagnosticPopulated => (_bits & IsUseSiteDiagnosticPopulatedBit) != 0; public bool IsConditionalPopulated => (_bits & IsConditionalPopulatedBit) != 0; public bool IsOverriddenOrHiddenMembersPopulated => (_bits & IsOverriddenOrHiddenMembersPopulatedBit) != 0; + public bool IsReadOnly => (_bits & IsReadOnlyBit) != 0; + public bool IsReadOnlyPopulated => (_bits & IsReadOnlyPopulatedBit) != 0; #if DEBUG static PackedFlags() @@ -130,6 +136,13 @@ public void InitializeIsExtensionMethod(bool isExtensionMethod) ThreadSafeFlagOperations.Set(ref _bits, bitsToSet); } + public void InitializeIsReadOnly(bool isReadOnly) + { + int bitsToSet = (isReadOnly ? IsReadOnlyBit : 0) | IsReadOnlyPopulatedBit; + Debug.Assert(BitsAreUnsetOrSame(_bits, bitsToSet)); + ThreadSafeFlagOperations.Set(ref _bits, bitsToSet); + } + public void InitializeMethodKind(MethodKind methodKind) { Debug.Assert((int)methodKind == ((int)methodKind & MethodKindMask)); @@ -1027,8 +1040,23 @@ public override ImmutableArray ExplicitInterfaceImplementations } } - // PROTOTYPE: need to round trip readonly attribute in metadata - internal override bool IsDeclaredReadOnly => false; + internal override bool IsDeclaredReadOnly + { + get + { + if (!_packedFlags.IsReadOnlyPopulated) + { + bool isReadOnly = false; + if (IsValidReadOnlyTarget) + { + var moduleSymbol = _containingType.ContainingPEModule; + isReadOnly = moduleSymbol.Module.HasIsReadOnlyAttribute(_handle); + } + _packedFlags.InitializeIsReadOnly(isReadOnly); + } + return _packedFlags.IsReadOnly; + } + } public override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken)) { diff --git a/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs index 0de3356fd1225..a7667b4aed096 100644 --- a/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs @@ -315,7 +315,9 @@ internal virtual bool IsExplicitInterfaceImplementation /// Indicates whether the method is effectively readonly, /// by either the method or the containing type being marked readonly. /// - internal bool IsEffectivelyReadOnly => (IsDeclaredReadOnly || ContainingType?.IsReadOnly == true) && MethodKind != MethodKind.Constructor; + internal bool IsEffectivelyReadOnly => (IsDeclaredReadOnly || ContainingType?.IsReadOnly == true) && IsValidReadOnlyTarget; + + protected bool IsValidReadOnlyTarget => !IsStatic && ContainingType.IsStructType() && MethodKind != MethodKind.Constructor; /// /// Returns interface methods explicitly implemented by this method. diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs index 9ffa94c99398d..0a584f8e714fc 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs @@ -1618,6 +1618,11 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r { base.AddSynthesizedAttributes(moduleBuilder, ref attributes); + if (IsDeclaredReadOnly) + { + AddSynthesizedAttribute(ref attributes, moduleBuilder.SynthesizeIsReadOnlyAttribute(this)); + } + bool isAsync = this.IsAsync; bool isIterator = this.IsIterator; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs index 2bae8706fe140..cb921d44b9f0a 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs @@ -1047,7 +1047,7 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, PartialMethodChecks(this, implementingPart, diagnostics); } - if (_refKind == RefKind.RefReadOnly) + if (_refKind == RefKind.RefReadOnly || IsDeclaredReadOnly) { this.DeclaringCompilation.EnsureIsReadOnlyAttributeExists(diagnostics, location, modifyCompilation: true); } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs index c49fc916ac6c1..671743c9a5a88 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs @@ -219,6 +219,12 @@ private SourcePropertyAccessorSymbol( } declarationModifiers |= propertyModifiers & ~DeclarationModifiers.Indexer; + // auto-implemented struct getters are implicitly 'readonly' + if (containingType.IsStructType() && !property.IsStatic && isAutoPropertyAccessor && methodKind == MethodKind.PropertyGet) + { + declarationModifiers |= DeclarationModifiers.ReadOnly; + } + // ReturnsVoid property is overridden in this class so // returnsVoid argument to MakeFlags is ignored. this.MakeFlags(methodKind, declarationModifiers, returnsVoid: false, isExtensionMethod: false, @@ -565,6 +571,16 @@ private ImmutableArray ComputeParameters(DiagnosticBag diagnost return parameters.ToImmutableAndFree(); } + internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, DiagnosticBag diagnostics) + { + base.AfterAddingTypeMembersChecks(conversions, diagnostics); + + if (IsDeclaredReadOnly) + { + this.DeclaringCompilation.EnsureIsReadOnlyAttributeExists(diagnostics, locations[0], modifyCompilation: true); + } + } + internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder attributes) { base.AddSynthesizedAttributes(moduleBuilder, ref attributes); diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs index b04593c8c4640..54bc556c235d7 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs @@ -1308,6 +1308,374 @@ public static explicit operator Test(Span value) CompileAndVerify(comp, expectedOutput: "SpanOpCalled", verify: Verification.Fails); } + [Fact] + public void ReadOnlyMembers_Metadata() + { + var csharp = @" +public struct S +{ + public void M1() {} + public readonly void M2() {} + + public int P1 { get; set; } + public readonly int P2 => 42; + public int P3 { readonly get => 123; } + public int P4 { readonly set {} } + public static int P5 { get; set; } +} +"; + CompileAndVerify(csharp, symbolValidator: validate); + + void validate(ModuleSymbol module) + { + var type = module.ContainingAssembly.GetTypeByMetadataName("S"); + var m1 = type.GetMethod("M1"); + var m2 = type.GetMethod("M2"); + + var p1 = type.GetProperty("P1"); + var p2 = type.GetProperty("P2"); + var p3 = type.GetProperty("P3"); + var p4 = type.GetProperty("P4"); + var p5 = type.GetProperty("P5"); + + var peModule = (PEModuleSymbol)module; + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m1).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m1).Signature.ReturnParam.Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m2).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m2).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p1).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p1.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p1.GetMethod).Signature.ReturnParam.Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p1.SetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p1.SetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p2).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p2.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p2.GetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p3).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p3.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p3.GetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p4).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p4.SetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p4.SetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p5).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p5.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p5.GetMethod).Signature.ReturnParam.Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p5.SetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p5.SetMethod).Signature.ReturnParam.Handle)); + + AssertDeclaresType(peModule, WellKnownType.System_Runtime_CompilerServices_IsReadOnlyAttribute, Accessibility.Internal); + } + } + + [Fact] + public void ReadOnlyMembers_RefReturn_Metadata() + { + var csharp = @" +public struct S +{ + static int i; + + public ref int M1() => ref i; + public readonly ref int M2() => ref i; + public ref readonly int M3() => ref i; + public readonly ref readonly int M4() => ref i; + + public ref int P1 { get => ref i; } + public readonly ref int P2 { get => ref i; } + public ref readonly int P3 { get => ref i; } + public readonly ref readonly int P4 { get => ref i; } +} +"; + CompileAndVerify(csharp, symbolValidator: validate); + + void validate(ModuleSymbol module) + { + var type = module.ContainingAssembly.GetTypeByMetadataName("S"); + var m1 = type.GetMethod("M1"); + var m2 = type.GetMethod("M2"); + var m3 = type.GetMethod("M3"); + var m4 = type.GetMethod("M4"); + + var p1 = type.GetProperty("P1"); + var p2 = type.GetProperty("P2"); + var p3 = type.GetProperty("P3"); + var p4 = type.GetProperty("P4"); + + var peModule = (PEModuleSymbol)module; + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m1).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m1).Signature.ReturnParam.Handle)); + + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m2).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m2).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m3).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m3).Signature.ReturnParam.Handle)); + + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m4).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m4).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p1).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p1.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p1.GetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p2).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p2.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p2.GetMethod).Signature.ReturnParam.Handle)); + + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p3).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p3.GetMethod).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p3.GetMethod).Signature.ReturnParam.Handle)); + + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p4).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p4.GetMethod).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p4.GetMethod).Signature.ReturnParam.Handle)); + + AssertDeclaresType(peModule, WellKnownType.System_Runtime_CompilerServices_IsReadOnlyAttribute, Accessibility.Internal); + } + } + + [Fact] + public void ReadOnlyStruct_ReadOnlyMembers_Metadata() + { + var csharp = @" +// note that both the type and member declarations are marked 'readonly' +public readonly struct S +{ + public void M1() {} + public readonly void M2() {} + + public int P1 { get; } + public readonly int P2 => 42; + public int P3 { readonly get => 123; } + public int P4 { readonly set {} } + public static int P5 { get; set; } +} +"; + CompileAndVerify(csharp, symbolValidator: validate); + + void validate(ModuleSymbol module) + { + var type = module.ContainingAssembly.GetTypeByMetadataName("S"); + var m1 = type.GetMethod("M1"); + var m2 = type.GetMethod("M2"); + + var p1 = type.GetProperty("P1"); + var p2 = type.GetProperty("P2"); + var p3 = type.GetProperty("P3"); + var p4 = type.GetProperty("P4"); + var p5 = type.GetProperty("P5"); + + var peModule = (PEModuleSymbol)module; + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m1).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m1).Signature.ReturnParam.Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m2).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)m2).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p1).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p1.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p1.GetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p2).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p2.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p2.GetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p3).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p3.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p3.GetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p4).Handle)); + Assert.True(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p4.SetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p4.SetMethod).Signature.ReturnParam.Handle)); + + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEPropertySymbol)p5).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p5.GetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p5.GetMethod).Signature.ReturnParam.Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p5.SetMethod).Handle)); + Assert.False(peModule.Module.HasIsReadOnlyAttribute(((PEMethodSymbol)p5.SetMethod).Signature.ReturnParam.Handle)); + + AssertDeclaresType(peModule, WellKnownType.System_Runtime_CompilerServices_IsReadOnlyAttribute, Accessibility.Internal); + } + } + + [Fact] + public void ReadOnlyMembers_MetadataRoundTrip() + { + var external = @" +using System; + +public struct S1 +{ + public void M1() {} + public readonly void M2() {} + + public int P1 { get; set; } + public readonly int P2 => 42; + public int P3 { readonly get => 123; } + public int P4 { readonly set {} } + public static int P5 { get; set; } + public readonly event Action E { add {} remove {} } +} + +public readonly struct S2 +{ + public void M1() {} + public int P1 { get; } + public int P2 => 42; + public int P3 { set {} } + public static int P4 { get; set; } + public event Action E { add {} remove {} } +} +"; + var externalComp = CreateCompilation(external); + verify(externalComp); + + var comp = CreateCompilation("", references: new[] { externalComp.EmitToImageReference() }); + verify(comp); + + var comp2 = CreateCompilation("", references: new[] { externalComp.ToMetadataReference() }); + verify(comp2); + + void verify(CSharpCompilation comp) + { + var s1 = comp.GetMember("S1"); + Assert.False(s1.GetMethod("M1").IsDeclaredReadOnly); + Assert.False(s1.GetMethod("M1").IsEffectivelyReadOnly); + + Assert.True(s1.GetMethod("M2").IsDeclaredReadOnly); + Assert.True(s1.GetMethod("M2").IsEffectivelyReadOnly); + + Assert.True(s1.GetProperty("P1").GetMethod.IsDeclaredReadOnly); + Assert.True(s1.GetProperty("P1").GetMethod.IsEffectivelyReadOnly); + Assert.False(s1.GetProperty("P1").SetMethod.IsDeclaredReadOnly); + Assert.False(s1.GetProperty("P1").SetMethod.IsEffectivelyReadOnly); + + Assert.True(s1.GetProperty("P2").GetMethod.IsDeclaredReadOnly); + Assert.True(s1.GetProperty("P2").GetMethod.IsEffectivelyReadOnly); + + Assert.True(s1.GetProperty("P3").GetMethod.IsDeclaredReadOnly); + Assert.True(s1.GetProperty("P3").GetMethod.IsEffectivelyReadOnly); + + Assert.True(s1.GetProperty("P4").SetMethod.IsDeclaredReadOnly); + Assert.True(s1.GetProperty("P4").SetMethod.IsEffectivelyReadOnly); + + Assert.False(s1.GetProperty("P5").GetMethod.IsDeclaredReadOnly); + Assert.False(s1.GetProperty("P5").GetMethod.IsEffectivelyReadOnly); + Assert.False(s1.GetProperty("P5").SetMethod.IsDeclaredReadOnly); + Assert.False(s1.GetProperty("P5").SetMethod.IsEffectivelyReadOnly); + + Assert.True(s1.GetEvent("E").AddMethod.IsDeclaredReadOnly); + Assert.True(s1.GetEvent("E").AddMethod.IsEffectivelyReadOnly); + + Assert.True(s1.GetEvent("E").RemoveMethod.IsDeclaredReadOnly); + Assert.True(s1.GetEvent("E").RemoveMethod.IsEffectivelyReadOnly); + + var s2 = comp.GetMember("S2"); + Assert.False(s2.GetMethod("M1").IsDeclaredReadOnly); + Assert.True(s2.GetMethod("M1").IsEffectivelyReadOnly); + + Assert.True(s2.GetProperty("P1").GetMethod.IsDeclaredReadOnly); + Assert.True(s2.GetProperty("P1").GetMethod.IsEffectivelyReadOnly); + + Assert.False(s2.GetProperty("P2").GetMethod.IsDeclaredReadOnly); + Assert.True(s2.GetProperty("P2").GetMethod.IsEffectivelyReadOnly); + + Assert.False(s2.GetProperty("P3").SetMethod.IsDeclaredReadOnly); + Assert.True(s2.GetProperty("P3").SetMethod.IsEffectivelyReadOnly); + + Assert.False(s2.GetProperty("P4").GetMethod.IsDeclaredReadOnly); + Assert.False(s2.GetProperty("P4").GetMethod.IsEffectivelyReadOnly); + Assert.False(s2.GetProperty("P4").SetMethod.IsDeclaredReadOnly); + Assert.False(s2.GetProperty("P4").SetMethod.IsEffectivelyReadOnly); + + Assert.False(s2.GetEvent("E").AddMethod.IsDeclaredReadOnly); + Assert.True(s2.GetEvent("E").AddMethod.IsEffectivelyReadOnly); + Assert.False(s2.GetEvent("E").RemoveMethod.IsDeclaredReadOnly); + Assert.True(s2.GetEvent("E").RemoveMethod.IsEffectivelyReadOnly); + } + } + + [Fact] + public void StaticReadOnlyMethod_FromMetadata() + { + var il = @" +.class private auto ansi '' +{ +} // end of class + +.class public auto ansi beforefieldinit System.Runtime.CompilerServices.IsReadOnlyAttribute + extends [mscorlib]System.Attribute +{ + // Methods + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + // Method begins at RVA 0x2050 + // 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 S + extends [mscorlib]System.ValueType +{ + .pack 0 + .size 1 + // Methods + .method public hidebysig + instance void M1 () cil managed + { + .custom instance void System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( + 01 00 00 00 + ) + // Method begins at RVA 0x2058 + // Code size 1 (0x1) + .maxstack 8 + IL_0000: ret + } // end of method S::M1 + + // Methods + .method public hidebysig static + void M2 () cil managed + { + .custom instance void System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( + 01 00 00 00 + ) + // Method begins at RVA 0x2058 + // Code size 1 (0x1) + .maxstack 8 + IL_0000: ret + } // end of method S::M2 +} // end of class S +"; + var ilRef = CompileIL(il); + + var comp = CreateCompilation("", references: new[] { ilRef }); + var s = comp.GetMember("S"); + + var m1 = s.GetMethod("M1"); + Assert.True(m1.IsDeclaredReadOnly); + Assert.True(m1.IsEffectivelyReadOnly); + + // even though the IsReadOnlyAttribute is in metadata, + // we ruled out the possibility of the method being readonly because it's static + var m2 = s.GetMethod("M2"); + Assert.False(m2.IsDeclaredReadOnly); + Assert.False(m2.IsEffectivelyReadOnly); + } + [Fact] public void ReadOnlyMethod_CallNormalMethod() { @@ -1436,6 +1804,164 @@ .locals init (S V_0, //copy }"); } + [Fact] + public void InMethod_CallMethodFromMetadata() + { + var external = @" +public struct S +{ + public int i; + + public readonly void M1() {} + + public void M2() + { + i = 23; + } +} +"; + var image = CreateCompilation(external).EmitToImageReference(); + + var csharp = @" +public static class C +{ + public static void M1(in S s) + { + // should not copy, no warning + s.M1(); + System.Console.Write(s.i); + + // should create local copy, warn in warning wave + s.M2(); + System.Console.Write(s.i); + + // explicit local copy, no warning + var copy = s; + copy.M2(); + System.Console.Write(copy.i); + } + + static void Main() + { + var s = new S { i = 1 }; + M1(in s); + } +} +"; + + var verifier = CompileAndVerify(csharp, references: new[] { image }, expectedOutput: "1123"); + // should warn about calling s.M2 in warning wave (see https://github.com/dotnet/roslyn/issues/33968) + verifier.VerifyDiagnostics(); + verifier.VerifyIL("C.M1", @" +{ + // Code size 68 (0x44) + .maxstack 1 + .locals init (S V_0, //copy + S V_1) + IL_0000: ldarg.0 + IL_0001: call ""void S.M1()"" + IL_0006: ldarg.0 + IL_0007: ldfld ""int S.i"" + IL_000c: call ""void System.Console.Write(int)"" + IL_0011: ldarg.0 + IL_0012: ldobj ""S"" + IL_0017: stloc.1 + IL_0018: ldloca.s V_1 + IL_001a: call ""void S.M2()"" + IL_001f: ldarg.0 + IL_0020: ldfld ""int S.i"" + IL_0025: call ""void System.Console.Write(int)"" + IL_002a: ldarg.0 + IL_002b: ldobj ""S"" + IL_0030: stloc.0 + IL_0031: ldloca.s V_0 + IL_0033: call ""void S.M2()"" + IL_0038: ldloc.0 + IL_0039: ldfld ""int S.i"" + IL_003e: call ""void System.Console.Write(int)"" + IL_0043: ret +}"); + } + + [Fact] + public void InMethod_CallGetAccessorFromMetadata() + { + var external = @" +public struct S +{ + public int i; + + public readonly int P1 => 42; + + public int P2 => i = 23; +} +"; + var image = CreateCompilation(external).EmitToImageReference(); + + var csharp = @" +public static class C +{ + public static void M1(in S s) + { + // should not copy, no warning + _ = s.P1; + System.Console.Write(s.i); + + // should create local copy, warn in warning wave + _ = s.P2; + System.Console.Write(s.i); + + // explicit local copy, no warning + var copy = s; + _ = copy.P2; + System.Console.Write(copy.i); + } + + static void Main() + { + var s = new S { i = 1 }; + M1(in s); + } +} +"; + + var verifier = CompileAndVerify(csharp, references: new[] { image }, expectedOutput: "1123"); + // should warn about calling s.M2 in warning wave (see https://github.com/dotnet/roslyn/issues/33968) + verifier.VerifyDiagnostics(); + verifier.VerifyIL("C.M1", @" +{ + // Code size 71 (0x47) + .maxstack 1 + .locals init (S V_0, //copy + S V_1) + IL_0000: ldarg.0 + IL_0001: call ""int S.P1.get"" + IL_0006: pop + IL_0007: ldarg.0 + IL_0008: ldfld ""int S.i"" + IL_000d: call ""void System.Console.Write(int)"" + IL_0012: ldarg.0 + IL_0013: ldobj ""S"" + IL_0018: stloc.1 + IL_0019: ldloca.s V_1 + IL_001b: call ""int S.P2.get"" + IL_0020: pop + IL_0021: ldarg.0 + IL_0022: ldfld ""int S.i"" + IL_0027: call ""void System.Console.Write(int)"" + IL_002c: ldarg.0 + IL_002d: ldobj ""S"" + IL_0032: stloc.0 + IL_0033: ldloca.s V_0 + IL_0035: call ""int S.P2.get"" + IL_003a: pop + IL_003b: ldloc.0 + IL_003c: ldfld ""int S.i"" + IL_0041: call ""void System.Console.Write(int)"" + IL_0046: ret +}"); + } + [Fact] public void ReadOnlyMethod_CallReadOnlyMethod() { @@ -1462,6 +1988,57 @@ .maxstack 2 comp.VerifyDiagnostics(); } + [Fact] + public void ReadOnlyGetAccessor_CallReadOnlyGetAccessor() + { + var csharp = @" +public struct S +{ + public int i; + public readonly int P1 { get => P2 + 1; } + public readonly int P2 { get => i; } +} +"; + var comp = CompileAndVerify(csharp); + comp.VerifyIL("S.P1.get", @" +{ + // Code size 9 (0x9) + .maxstack 2 + IL_0000: ldarg.0 + IL_0001: call ""int S.P2.get"" + IL_0006: ldc.i4.1 + IL_0007: add + IL_0008: ret +} +"); + comp.VerifyDiagnostics(); + } + + [Fact] + public void ReadOnlyGetAccessor_CallAutoGetAccessor() + { + var csharp = @" +public struct S +{ + public readonly int P1 { get => P2 + 1; } + public int P2 { get; } +} +"; + var comp = CompileAndVerify(csharp); + comp.VerifyIL("S.P1.get", @" +{ + // Code size 9 (0x9) + .maxstack 2 + IL_0000: ldarg.0 + IL_0001: call ""int S.P2.get"" + IL_0006: ldc.i4.1 + IL_0007: add + IL_0008: ret +} +"); + comp.VerifyDiagnostics(); + } + [Fact] public void InParam_CallReadOnlyMethod() { diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs index 048f228235e9b..e12b3e17a0a30 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs @@ -2078,9 +2078,24 @@ internal struct InternalStruct var refImage = comp.EmitToImageReference(emitRefOnly); var compWithRef = CreateEmptyCompilation("", references: new[] { MscorlibRef, refImage }, options: TestOptions.DebugDll.WithMetadataImportOptions(MetadataImportOptions.All)); + + var globalNamespace = compWithRef.SourceModule.GetReferencedAssemblySymbols().Last().GlobalNamespace; + AssertEx.Equal( - new[] { "", "InternalStruct" }, - compWithRef.SourceModule.GetReferencedAssemblySymbols().Last().GlobalNamespace.GetMembers().Select(m => m.ToDisplayString())); + new[] { "", "InternalStruct", "Microsoft", "System" }, + globalNamespace.GetMembers().Select(m => m.ToDisplayString())); + + AssertEx.Equal(new[] { "Microsoft.CodeAnalysis" }, globalNamespace.GetMember("Microsoft").GetMembers().Select(m => m.ToDisplayString())); + AssertEx.Equal( + new[] { "Microsoft.CodeAnalysis.EmbeddedAttribute" }, + globalNamespace.GetMember("Microsoft.CodeAnalysis").GetMembers().Select(m => m.ToDisplayString())); + + AssertEx.Equal( + new[] { "System.Runtime.CompilerServices" }, + globalNamespace.GetMember("System.Runtime").GetMembers().Select(m => m.ToDisplayString())); + AssertEx.Equal( + new[] { "System.Runtime.CompilerServices.IsReadOnlyAttribute" }, + globalNamespace.GetMember("System.Runtime.CompilerServices").GetMembers().Select(m => m.ToDisplayString())); AssertEx.Equal( new[] { "System.Int32 InternalStruct.

k__BackingField", "InternalStruct..ctor()" }, diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 2778ffb82d561..9d377fc3201e9 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -997,6 +997,26 @@ public readonly event Action E { add {} remove {} } Diagnostic(ErrorCode.ERR_BadMemberFlag, "E").WithArguments("readonly").WithLocation(9, 45)); } + [Fact] + public void ReadOnlyClass_NormalMethod() + { + var csharp = @" +public readonly class C +{ + int i; + void M() + { + i++; + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (2,23): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly class C + Diagnostic(ErrorCode.ERR_BadMemberFlag, "C").WithArguments("readonly").WithLocation(2, 23)); + } + [Fact] public void ReadOnlyInterface() { @@ -1563,6 +1583,34 @@ public void ReadOnlyDelegate() Diagnostic(ErrorCode.ERR_BadMemberFlag, "Del").WithArguments("readonly").WithLocation(2, 30)); } + [Fact] + public void ReadOnlyLocalFunction() + { + var csharp = @" +public struct S +{ + void M1() + { + local(); + readonly void local() {} + } + readonly void M2() + { + local(); + readonly void local() {} + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (7,9): error CS0106: The modifier 'readonly' is not valid for this item + // readonly void local() {} + Diagnostic(ErrorCode.ERR_BadMemberFlag, "readonly").WithArguments("readonly").WithLocation(7, 9), + // (12,9): error CS0106: The modifier 'readonly' is not valid for this item + // readonly void local() {} + Diagnostic(ErrorCode.ERR_BadMemberFlag, "readonly").WithArguments("readonly").WithLocation(12, 9)); + } + [Fact] public void ReadOnlyIndexer() {