From 7761d6b8e6ecbe58a43fea5fd035ca0842e77b30 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 25 Mar 2022 12:30:33 -0700 Subject: [PATCH 01/12] Support SetsRequiredMembersAttribute The SetsRequiredMembersAttribute prevents the compiler from checking the required member list of a type when calling that constructor, and suppresses any errors from a base type's list being invalid. Specification: https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md Test plan: https://github.com/dotnet/roslyn/issues/57046 --- .../Portable/Binder/Binder_Expressions.cs | 45 +- .../Compiler/MethodBodySynthesizer.Lowered.cs | 2 + .../Portable/Compiler/MethodCompiler.cs | 4 +- .../Portable/FlowAnalysis/NullableWalker.cs | 7 +- .../IteratorFinallyMethodSymbol.cs | 2 + .../AnonymousType.SynthesizedMethodBase.cs | 2 + .../Portable/Symbols/ErrorMethodSymbol.cs | 10 + .../FunctionPointerMethodSymbol.cs | 1 + .../Symbols/Metadata/PE/PEMethodSymbol.cs | 33 +- .../CSharp/Portable/Symbols/MethodSymbol.cs | 7 + .../Symbols/ReducedExtensionMethodSymbol.cs | 2 + .../Symbols/SignatureOnlyMethodSymbol.cs | 2 + .../Source/SourceConstructorSymbolBase.cs | 35 + .../Symbols/Source/SourceMethodSymbol.cs | 3 + .../SourceMethodSymbolWithAttributes.cs | 2 +- .../Portable/Symbols/SymbolExtensions.cs | 3 +- .../Records/SynthesizedRecordCopyCtor.cs | 9 + .../Synthesized/SynthesizedDelegateSymbol.cs | 2 + .../SynthesizedEntryPointSymbol.cs | 2 + .../SynthesizedGlobalMethodSymbol.cs | 2 + .../SynthesizedImplementationMethod.cs | 2 + .../SynthesizedInstanceConstructor.cs | 1 + ...SynthesizedInteractiveInitializerMethod.cs | 2 + .../SynthesizedIntrinsicOperatorSymbol.cs | 2 + .../SynthesizedStaticConstructor.cs | 2 + .../Portable/Symbols/TypeSymbolExtensions.cs | 6 + .../Symbols/Wrapped/WrappedMethodSymbol.cs | 2 + .../Symbol/Symbols/RequiredMembersTests.cs | 1199 +++++++++++------ .../Attributes/AttributeDescription.cs | 1 + ...CommonMethodEarlyWellKnownAttributeData.cs | 18 + .../Core/Portable/WellKnownMember.cs | 1 + .../Core/Portable/WellKnownMembers.cs | 8 + src/Compilers/Core/Portable/WellKnownTypes.cs | 4 +- .../Test/Utilities/CSharp/CSharpTestBase.cs | 13 + 34 files changed, 995 insertions(+), 441 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs index a5fa7098a451d..ff5ce48abf376 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs @@ -5846,25 +5846,7 @@ internal bool TryPerformConstructorOverloadResolution( } } - if (suppressUnsupportedRequiredMembersError && useSiteInfo.AccumulatesDiagnostics && useSiteInfo.Diagnostics is { Count: not 0 }) - { - diagnostics.AddDependencies(useSiteInfo); - foreach (var diagnostic in useSiteInfo.Diagnostics) - { - // We don't want to report this error here because we'll report ERR_RequiredMembersBaseTypeInvalid. That error is suppressable by the - // user using the `SetsRequiredMembers` attribute on the constructor, so reporting this error would prevent that from working. - if ((ErrorCode)diagnostic.Code == ErrorCode.ERR_RequiredMembersInvalid) - { - continue; - } - - diagnostics.ReportUseSiteDiagnostic(diagnostic, errorLocation); - } - } - else - { - diagnostics.Add(errorLocation, useSiteInfo); - } + ReportConstructorUseSiteDiagnostics(errorLocation, diagnostics, suppressUnsupportedRequiredMembersError, useSiteInfo); if (succeededIgnoringAccessibility) { @@ -5921,6 +5903,31 @@ internal bool TryPerformConstructorOverloadResolution( return succeededConsideringAccessibility; } + internal static bool ReportConstructorUseSiteDiagnostics(Location errorLocation, BindingDiagnosticBag diagnostics, bool suppressUnsupportedRequiredMembersError, CompoundUseSiteInfo useSiteInfo) + { + if (suppressUnsupportedRequiredMembersError && useSiteInfo.AccumulatesDiagnostics && useSiteInfo.Diagnostics is { Count: not 0 }) + { + diagnostics.AddDependencies(useSiteInfo); + foreach (var diagnostic in useSiteInfo.Diagnostics) + { + // We don't want to report this error here because we'll report ERR_RequiredMembersBaseTypeInvalid. That error is suppressable by the + // user using the `SetsRequiredMembers` attribute on the constructor, so reporting this error would prevent that from working. + if ((ErrorCode)diagnostic.Code == ErrorCode.ERR_RequiredMembersInvalid) + { + continue; + } + + diagnostics.ReportUseSiteDiagnostic(diagnostic, errorLocation); + } + + return true; + } + else + { + return diagnostics.Add(errorLocation, useSiteInfo); + } + } + private ImmutableArray GetAccessibleConstructorsForOverloadResolution(NamedTypeSymbol type, ref CompoundUseSiteInfo useSiteInfo) { ImmutableArray allInstanceConstructors; diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodBodySynthesizer.Lowered.cs b/src/Compilers/CSharp/Portable/Compiler/MethodBodySynthesizer.Lowered.cs index e61efd3329cb3..f359679064a45 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodBodySynthesizer.Lowered.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodBodySynthesizer.Lowered.cs @@ -195,6 +195,8 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState, F.CloseMethod(F.ThrowNull()); } } + + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } internal abstract partial class MethodToClassRewriter diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index 29ed8c74a05ca..fa62c3d18bfb0 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -2142,7 +2142,9 @@ private static BoundCall GenerateBaseCopyConstructorInitializer(SynthesizedRecor return null; } - if (Binder.ReportUseSite(baseConstructor, diagnostics, diagnosticsLocation)) + var constructorUseSiteInfo = CompoundUseSiteInfo.DiscardedDependencies; + constructorUseSiteInfo.Add(baseConstructor.GetUseSiteInfo()); + if (Binder.ReportConstructorUseSiteDiagnostics(diagnosticsLocation, diagnostics, suppressUnsupportedRequiredMembersError: constructor.HasSetsRequiredMembers, constructorUseSiteInfo)) { return null; } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 123ff8ce5b272..10b7730eefdb7 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -871,6 +871,8 @@ void makeNotNullMembersMaybeNull() } } + return; + ImmutableArray getMembersNeedingDefaultInitialState() { if (_hasInitialState) @@ -880,12 +882,13 @@ ImmutableArray getMembersNeedingDefaultInitialState() // We don't use a default initial state for value type instance constructors without `: this()` because // any usages of uninitialized fields will get definite assignment errors anyway. - bool hasConstructorInitializer = method.HasThisConstructorInitializer(out _); - if (!hasConstructorInitializer && (!method.ContainingType.IsValueType || method.IsStatic)) + if (!method.HasThisConstructorInitializer(out _) && (!method.ContainingType.IsValueType || method.IsStatic)) { return membersToBeInitialized(method.ContainingType, includeAllMembers: true); } + // We want to presume all required members of the type are uninitialized, and in addition we want to set all fields to + // default if we can get to this constructor by doing so (ie, : this() in a value type). return membersToBeInitialized(method.ContainingType, includeAllMembers: method.IncludeFieldInitializersInBody()); static ImmutableArray membersToBeInitialized(NamedTypeSymbol containingType, bool includeAllMembers) diff --git a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorFinallyMethodSymbol.cs b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorFinallyMethodSymbol.cs index 1240b4c79e1c7..93d78aeb2f272 100644 --- a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorFinallyMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorFinallyMethodSymbol.cs @@ -255,5 +255,7 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l { return _stateMachineType.KickoffMethod.CalculateLocalSyntaxOffset(localPosition, localTree); } + + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.SynthesizedMethodBase.cs b/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.SynthesizedMethodBase.cs index 6a19be4e5f635..41e9f8a26619e 100644 --- a/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.SynthesizedMethodBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.SynthesizedMethodBase.cs @@ -231,6 +231,8 @@ internal sealed override int CalculateLocalSyntaxOffset(int localPosition, Synta { throw ExceptionUtilities.Unreachable; } + + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } } diff --git a/src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs index 297301f6b0a83..c28022499e6cd 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.Symbols @@ -274,5 +275,14 @@ internal override bool GenerateDebugInfo } internal sealed override bool IsNullableAnalysisEnabled() => false; + + protected override bool HasSetsRequiredMembersImpl + { + get + { + Debug.Assert(MethodKind == MethodKind.Constructor); + return false; + } + } } } diff --git a/src/Compilers/CSharp/Portable/Symbols/FunctionPointers/FunctionPointerMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/FunctionPointers/FunctionPointerMethodSymbol.cs index 2f41d2d20166b..3d33ae01354b3 100644 --- a/src/Compilers/CSharp/Portable/Symbols/FunctionPointers/FunctionPointerMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/FunctionPointers/FunctionPointerMethodSymbol.cs @@ -830,5 +830,6 @@ public override bool IsVararg internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree) => throw ExceptionUtilities.Unreachable; internal override IEnumerable GetSecurityInformation() => throw ExceptionUtilities.Unreachable; internal sealed override bool IsNullableAnalysisEnabled() => throw ExceptionUtilities.Unreachable; + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs index 13dffcbcdea8c..4f1422e1d986c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs @@ -47,7 +47,7 @@ private struct PackedFlags { // We currently pack everything into a 32-bit int with the following layout: // - // |u|t|s|r|q|p|ooo|n|m|l|k|j|i|h|g|f|e|d|c|b|aaaaa| + // |w|v|u|t|s|r|q|p|ooo|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. @@ -72,7 +72,9 @@ private struct PackedFlags // s = IsInitOnlyBit. 1 bit. // t = IsInitOnlyPopulatedBit. 1 bit. // u = IsUnmanagedCallersOnlyAttributePopulated. 1 bit. - // 6 bits remain for future purposes. + // v = IsSetsRequiredMembersBit. 1 bit. + // w = IsSetsRequiredMembersPopulated. 1 bit. + // 4 bits remain for future purposes. private const int MethodKindOffset = 0; private const int MethodKindMask = 0x1F; @@ -98,6 +100,8 @@ private struct PackedFlags private const int IsInitOnlyBit = 0x1 << 24; private const int IsInitOnlyPopulatedBit = 0x1 << 25; private const int IsUnmanagedCallersOnlyAttributePopulatedBit = 0x1 << 26; + private const int HasSetsRequiredMembersBit = 0x1 << 27; + private const int HasSetsRequiredMembersPopulatedBit = 0x1 << 28; private int _bits; @@ -134,6 +138,8 @@ public MethodKind MethodKind public bool IsInitOnly => (_bits & IsInitOnlyBit) != 0; public bool IsInitOnlyPopulated => (_bits & IsInitOnlyPopulatedBit) != 0; public bool IsUnmanagedCallersOnlyAttributePopulated => (_bits & IsUnmanagedCallersOnlyAttributePopulatedBit) != 0; + public bool HasSetsRequiredMembers => (_bits & HasSetsRequiredMembersBit) != 0; + public bool HasSetsRequiredMembersPopulated => (_bits & HasSetsRequiredMembersPopulatedBit) != 0; #if DEBUG static PackedFlags() @@ -240,6 +246,14 @@ public void SetIsUnmanagedCallersOnlyAttributePopulated() { ThreadSafeFlagOperations.Set(ref _bits, IsUnmanagedCallersOnlyAttributePopulatedBit); } + + public bool InitializeSetsRequiredMembersBit(bool value) + { + int bitsToSet = HasSetsRequiredMembersPopulatedBit; + if (value) bitsToSet |= HasSetsRequiredMembersBit; + + return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet); + } } /// @@ -1407,6 +1421,21 @@ internal override ImmutableArray GetAppliedConditionalSymbols() } } + protected override bool HasSetsRequiredMembersImpl + { + get + { + Debug.Assert(MethodKind == MethodKind.Constructor); + if (!_packedFlags.HasSetsRequiredMembersPopulated) + { + var result = _containingType.ContainingPEModule.Module.HasAttribute(_handle, AttributeDescription.SetsRequiredMembersAttribute); + _packedFlags.InitializeSetsRequiredMembersBit(result); + } + + return _packedFlags.HasSetsRequiredMembers; + } + } + internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree) { throw ExceptionUtilities.Unreachable; diff --git a/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs index 85030a8a2eddf..9cc089a91b540 100644 --- a/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs @@ -579,6 +579,13 @@ public bool IsConditional } } + /// + /// Returns true if this is a constructor attributed with HasSetsRequiredMembers + /// + internal bool HasSetsRequiredMembers => MethodKind == MethodKind.Constructor && HasSetsRequiredMembersImpl; + + protected abstract bool HasSetsRequiredMembersImpl { get; } + /// /// Some method kinds do not participate in overriding/hiding (e.g. constructors). /// diff --git a/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs index acaf3d1f1e2d6..13b88243aa82d 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs @@ -588,6 +588,8 @@ public override int GetHashCode() return _reducedFrom.GetHashCode(); } + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; + #nullable enable private sealed class ReducedExtensionMethodParameterSymbol : WrappedParameterSymbol diff --git a/src/Compilers/CSharp/Portable/Symbols/SignatureOnlyMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/SignatureOnlyMethodSymbol.cs index 59a09861eb1cf..bb92c1a6e5afb 100644 --- a/src/Compilers/CSharp/Portable/Symbols/SignatureOnlyMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/SignatureOnlyMethodSymbol.cs @@ -172,6 +172,8 @@ internal override bool IsMetadataFinal internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree) { throw ExceptionUtilities.Unreachable; } + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; + #endregion } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbolBase.cs index 1c35bb314b9a9..4689b1cf024a1 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbolBase.cs @@ -246,5 +246,40 @@ internal sealed override int CalculateLocalSyntaxOffset(int position, SyntaxTree protected abstract CSharpSyntaxNode GetInitializer(); protected abstract bool IsWithinExpressionOrBlockBody(int position, out int offset); + +#nullable enable + protected sealed override bool HasSetsRequiredMembersImpl + => GetEarlyDecodedWellKnownAttributeData()?.HasSetsRequiredMembersAttribute == true; + + internal sealed override (CSharpAttributeData?, BoundAttribute?) EarlyDecodeWellKnownAttribute(ref EarlyDecodeWellKnownAttributeArguments arguments) + { + if (arguments.SymbolPart == AttributeLocation.None) + { + if (CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.SetsRequiredMembersAttribute)) + { + var earlyData = arguments.GetOrCreateData(); + earlyData.HasSetsRequiredMembersAttribute = true; + + if (ContainingType.IsWellKnownSetsRequiredMembersAttribute()) + { + // Avoid a binding cycle for this scenario. + return (null, null); + } + + var (attributeData, boundAttribute) = arguments.Binder.GetAttribute(arguments.AttributeSyntax, arguments.AttributeType, beforeAttributePartBound: null, afterAttributePartBound: null, out bool hasAnyDiagnostics); + + if (!hasAnyDiagnostics) + { + return (attributeData, boundAttribute); + } + else + { + return (null, null); + } + } + } + + return base.EarlyDecodeWellKnownAttribute(ref arguments); + } } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbol.cs index 276e745c4547e..127b3aecc35dc 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbol.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.Symbols { @@ -78,5 +79,7 @@ internal void ReportAsyncParameterErrors(BindingDiagnosticBag diagnostics, Locat static Location getLocation(ParameterSymbol parameter, Location location) => parameter.Locations.FirstOrDefault() ?? location; } + + protected override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs index ae7179d9ef187..e3102401bc21e 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs @@ -318,7 +318,7 @@ public override ImmutableArray GetReturnTypeAttributes() } #nullable enable - internal sealed override (CSharpAttributeData?, BoundAttribute?) EarlyDecodeWellKnownAttribute(ref EarlyDecodeWellKnownAttributeArguments arguments) + internal override (CSharpAttributeData?, BoundAttribute?) EarlyDecodeWellKnownAttribute(ref EarlyDecodeWellKnownAttributeArguments arguments) { Debug.Assert(arguments.SymbolPart == AttributeLocation.None || arguments.SymbolPart == AttributeLocation.Return); diff --git a/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs index 30000f258033d..f5d9ea7070f6b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs @@ -843,7 +843,6 @@ internal static bool HasAsyncMethodBuilderAttribute(this Symbol symbol, [NotNull internal static bool IsRequired(this Symbol symbol) => symbol is FieldSymbol { IsRequired: true } or PropertySymbol { IsRequired: true }; internal static bool ShouldCheckRequiredMembers(this MethodSymbol method) - // PROTOTYPE(req): Check for the SetsRequiredMembersAttribute and return false for that case - => method.MethodKind == MethodKind.Constructor; + => method.MethodKind == MethodKind.Constructor && !method.HasSetsRequiredMembers; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs index 511c0e79b50e0..87d8720c55e8a 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs @@ -64,6 +64,11 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r var compilation = this.DeclaringCompilation; AddSynthesizedAttribute(ref attributes, compilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor)); Debug.Assert(WellKnownMembers.IsSynthesizedAttributeOptional(WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor)); + + if (HasSetsRequiredMembersImpl) + { + AddSynthesizedAttribute(ref attributes, compilation.TrySynthesizeAttribute(WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor)); + } } internal static MethodSymbol? FindCopyConstructor(NamedTypeSymbol containingType, NamedTypeSymbol within, ref CompoundUseSiteInfo useSiteInfo) @@ -127,5 +132,9 @@ internal static bool HasCopyConstructorSignature(MethodSymbol member) method.Parameters[0].Type.Equals(containingType, TypeCompareKind.AllIgnoreOptions) && method.Parameters[0].RefKind == RefKind.None; } + + protected sealed override bool HasSetsRequiredMembersImpl + // If the record type has a required members error, then it does have required members of some kind, we emit the SetsRequiredMembers attribute. + => !ContainingType.AllRequiredMembers.IsEmpty || ContainingType.HasRequiredMembersError; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs index e5c17ffc70887..b8c36fde38207 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs @@ -238,5 +238,7 @@ public override bool IsExtern { get { return false; } } + + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs index 6778b714ce4a4..22aa8e6b65c79 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs @@ -307,6 +307,8 @@ private static BoundCall CreateParameterlessCall(CSharpSyntaxNode syntax, BoundE { WasCompilerGenerated = true }; } + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; + /// A synthesized entrypoint that forwards all calls to an async Main Method internal sealed class AsyncForwardEntryPoint : SynthesizedEntryPointSymbol { diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs index 4b66d9d66ebfe..893bb85ea8ed1 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs @@ -335,5 +335,7 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l } internal sealed override bool IsNullableAnalysisEnabled() => false; + + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedImplementationMethod.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedImplementationMethod.cs index 23fe785515aa3..eeecc9bf23d90 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedImplementationMethod.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedImplementationMethod.cs @@ -265,5 +265,7 @@ internal override ImmutableArray GetAppliedConditionalSymbols() { return ImmutableArray.Empty; } + + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInstanceConstructor.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInstanceConstructor.cs index 0c27009d34a61..47ee92c9851bc 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInstanceConstructor.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInstanceConstructor.cs @@ -316,5 +316,6 @@ internal virtual void GenerateMethodBodyStatements(SyntheticBoundNodeFactory fac // overridden in a derived class to add extra statements to the body of the generated constructor } + protected override bool HasSetsRequiredMembersImpl => false; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInteractiveInitializerMethod.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInteractiveInitializerMethod.cs index c6f7bc3fbe8dc..69baa906a9872 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInteractiveInitializerMethod.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInteractiveInitializerMethod.cs @@ -275,5 +275,7 @@ private static void CalculateReturnType( : compilation.GetTypeByReflectionType(submissionReturnTypeOpt, diagnostics); returnType = taskT.Construct(resultType); } + + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs index 95de7c40285d0..d9960d15c5940 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs @@ -423,6 +423,8 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l internal sealed override bool IsNullableAnalysisEnabled() => false; + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; + public override bool Equals(Symbol obj, TypeCompareKind compareKind) { if (obj == (object)this) diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedStaticConstructor.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedStaticConstructor.cs index 1166c2218d61a..948bd9f64071d 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedStaticConstructor.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedStaticConstructor.cs @@ -428,5 +428,7 @@ private bool CalculateShouldEmit(ImmutableArray boundInitializ return false; } + + protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs index a83da25243d79..c82986c2d5a9f 100644 --- a/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs @@ -2002,6 +2002,12 @@ private static bool IsWellKnownCompilerServicesTopLevelType(this TypeSymbol type internal static bool IsCompilerServicesTopLevelType(this TypeSymbol typeSymbol) => typeSymbol.ContainingType is null && IsContainedInNamespace(typeSymbol, "System", "Runtime", "CompilerServices"); + internal static bool IsWellKnownSetsRequiredMembersAttribute(this TypeSymbol type) + => type.Name == "SetsRequiredMembersAttribute" && type.IsWellKnownDiagnosticsCodeAnalysisTopLevelType(); + + private static bool IsWellKnownDiagnosticsCodeAnalysisTopLevelType(this TypeSymbol typeSymbol) + => typeSymbol.ContainingType is null && IsContainedInNamespace(typeSymbol, "System", "Diagnostics", "CodeAnalysis"); + private static bool IsContainedInNamespace(this TypeSymbol typeSymbol, string outerNS, string midNS, string innerNS) { var innerNamespace = typeSymbol.ContainingNamespace; diff --git a/src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedMethodSymbol.cs index 19343a9b2d55b..06f156ca7690b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedMethodSymbol.cs @@ -358,5 +358,7 @@ internal override bool GenerateDebugInfo internal override bool IsDeclaredReadOnly => UnderlyingMethod.IsDeclaredReadOnly; internal override bool IsInitOnly => UnderlyingMethod.IsInitOnly; + + protected sealed override bool HasSetsRequiredMembersImpl => UnderlyingMethod.HasSetsRequiredMembers; } } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index bc1701d1b9733..5463fe8577d55 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -24,10 +24,16 @@ Namespace System.Runtime.CompilerServices Public Class RequiredMemberAttribute Inherits Attribute End Class +End Namespace +Namespace System.Diagnostics.CodeAnalysis + + Public Class SetsRequiredMembersAttribute + Inherits Attribute + End Class End Namespace"; private static CSharpCompilation CreateCompilationWithRequiredMembers(CSharpTestSource source, IEnumerable? references = null, CSharpParseOptions? parseOptions = null, CSharpCompilationOptions? options = null, string? assemblyName = null, TargetFramework targetFramework = TargetFramework.Standard) - => CreateCompilation(new[] { source, RequiredMemberAttribute }, references, options: options, parseOptions: parseOptions, assemblyName: assemblyName, targetFramework: targetFramework); + => CreateCompilation(new[] { source, RequiredMemberAttribute, SetsRequiredMembersAttribute }, references, options: options, parseOptions: parseOptions, assemblyName: assemblyName, targetFramework: targetFramework); private Compilation CreateVisualBasicCompilationWithRequiredMembers(string source) => CreateVisualBasicCompilation(new[] { source, RequiredMemberAttributeVB }); @@ -1138,7 +1144,6 @@ class C } "); - // PROTOTYPE(req): Confirm with LDM whether we want a warning here. comp.VerifyDiagnostics(); } @@ -1153,7 +1158,6 @@ class C } "); - // PROTOTYPE(req): Confirm with LDM whether we want an error here. comp.VerifyDiagnostics( // (5,29): error CS9505: Required member 'C.Prop' must be settable. // public required ref int Prop => ref i; @@ -1170,17 +1174,21 @@ class C { public required readonly int Field; public required int Prop1 { get; } + public required int Prop2 { get; protected set; } } "); - // PROTOTYPE(req): Confirm with LDM whether we want an error here. comp.VerifyDiagnostics( // (5,34): error CS9505: Required member 'C.Field' must be settable. // public required readonly int Field; Diagnostic(ErrorCode.ERR_RequiredMemberMustBeSettable, "Field").WithArguments("C.Field").WithLocation(5, 34), // (6,25): error CS9505: Required member 'C.Prop1' must be settable. // public required int Prop1 { get; } - Diagnostic(ErrorCode.ERR_RequiredMemberMustBeSettable, "Prop1").WithArguments("C.Prop1").WithLocation(6, 25) + Diagnostic(ErrorCode.ERR_RequiredMemberMustBeSettable, "Prop1").WithArguments("C.Prop1").WithLocation(6, 25), + // PROTOTYPE(req): Better error message? + // (7,25): error CS9503: Required member 'C.Prop2' cannot be less visible or have a setter less visible than the containing type 'C'. + // public required int Prop2 { get; protected set; } + Diagnostic(ErrorCode.ERR_RequiredMemberCannotBeLessVisibleThanContainingType, "Prop2").WithArguments("C.Prop2", "C").WithLocation(7, 25) ); } @@ -1265,6 +1273,33 @@ public class C comp.VerifyDiagnostics(expectedDiagnostics); } + [Theory] + [CombinatorialData] + public void EnforcedRequiredMembers_NoInheritance_NoneSet_HasSetsRequiredMembers(bool useMetadataReference, [CombinatorialValues("", " C")] string constructor) + { + var c = @" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public required int Prop { get; set; } + public required int Field; + + [SetsRequiredMembers] + public C() {} +} +"; + + var creation = $@"C c = new{constructor}();"; + var comp = CreateCompilationWithRequiredMembers(new[] { c, creation }); + + comp.VerifyDiagnostics(); + + var cComp = CreateCompilationWithRequiredMembers(c); + comp = CreateCompilation(creation, references: new[] { useMetadataReference ? cComp.ToMetadataReference() : cComp.EmitToImageReference() }); + + comp.VerifyDiagnostics(); + } + [Theory] [CombinatorialData] public void EnforcedRequiredMembers_NoInheritance_PartialSet(bool useMetadataReference, [CombinatorialValues("", " C")] string constructor) @@ -1407,6 +1442,40 @@ End Class ); } + [Fact] + public void EnforcedRequiredMembers_NoInheritance_Unsettable_HasSetsRequiredMembers() + { + var c = @" +using System.Diagnostics.CodeAnalysis; +var c = new C(); +c = new C() { Prop1 = 1, Field1 = 1 }; + +public class C +{ + public required int Prop1 { get; } + public required readonly int Field1; + + [SetsRequiredMembers] + public C() {} +} +"; + var comp = CreateCompilationWithRequiredMembers(c); + comp.VerifyDiagnostics( + // (4,15): error CS0200: Property or indexer 'C.Prop1' cannot be assigned to -- it is read only + // c = new C() { Prop1 = 1, Field1 = 1 }; + Diagnostic(ErrorCode.ERR_AssgReadonlyProp, "Prop1").WithArguments("C.Prop1").WithLocation(4, 15), + // (4,26): error CS0191: A readonly field cannot be assigned to (except in a constructor or init-only setter of the type in which the field is defined or a variable initializer) + // c = new C() { Prop1 = 1, Field1 = 1 }; + Diagnostic(ErrorCode.ERR_AssgReadonly, "Field1").WithLocation(4, 26), + // (8,25): error CS9505: Required member 'C.Prop1' must be settable. + // public required int Prop1 { get; } + Diagnostic(ErrorCode.ERR_RequiredMemberMustBeSettable, "Prop1").WithArguments("C.Prop1").WithLocation(8, 25), + // (9,34): error CS9505: Required member 'C.Field1' must be settable. + // public required readonly int Field1; + Diagnostic(ErrorCode.ERR_RequiredMemberMustBeSettable, "Field1").WithArguments("C.Field1").WithLocation(9, 34) + ); + } + [Fact] public void EnforcedRequiredMembers_NoInheritance_DisallowedNestedObjectInitializer() { @@ -1434,6 +1503,30 @@ public class D ); } + [Fact] + public void EnforcedRequiredMembers_NoInheritance_DisallowedNestedObjectInitializer_HasSetsRequiredMembers() + { + var c = @" +using System.Diagnostics.CodeAnalysis; +var c = new C() { D1 = { NestedProp = 1 }, D2 = { NestedProp = 2 } }; + +public class C +{ + public required D D1 { get; set; } + public required D D2; + + [SetsRequiredMembers] + public C() {} +} +public class D +{ + public int NestedProp { get; set; } +} +"; + var comp = CreateCompilationWithRequiredMembers(c); + comp.VerifyDiagnostics(); + } + [Fact] public void EnforcedRequiredMembers_NoInheritance_NestedObjectCreationAllowed() { @@ -1478,6 +1571,27 @@ public class C ); } + [Fact] + public void EnforcedRequiredMembers_NoInheritance_DisallowedNestedCollectionInitializer_HasSetsRequiredMember() + { + var c = @" +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +var c = new C() { L1 = { 1, 2, 3 }, L2 = { 4, 5, 6 } }; + +public class C +{ + public required List L1 { get; set; } + public required List L2; + + [SetsRequiredMembers] + public C() {} +} +"; + var comp = CreateCompilationWithRequiredMembers(c); + comp.VerifyDiagnostics(); + } + [Fact] public void EnforcedRequiredMembers_NoInheritance_NestedObjectCreationWithCollectionInitializerAllowed() { @@ -1543,6 +1657,48 @@ public class Derived : Base comp.VerifyDiagnostics(expectedDiagnostics); } + [Theory] + [CombinatorialData] + public void EnforcedRequiredMembers_Inheritance_NoneSet_HasSetsRequiredMembers(bool useMetadataReference) + { + var @base = @" +public class Base +{ + public required int Prop1 { get; set; } + public required int Field1; +} +"; + + var derived = @" +using System.Diagnostics.CodeAnalysis; + +public class Derived : Base +{ + public required int Prop2 { get; set; } + public required int Field2; + + [SetsRequiredMembers] + public Derived() {} +} +"; + + var code = @"_ = new Derived();"; + + var comp = CreateCompilationWithRequiredMembers(new[] { @base, derived, code }); + + comp.VerifyDiagnostics(); + + var baseComp = CreateCompilationWithRequiredMembers(@base); + baseComp.VerifyEmitDiagnostics(); + var baseRef = useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference(); + + var derivedComp = CreateCompilation(derived, new[] { baseRef }); + derivedComp.VerifyEmitDiagnostics(); + + comp = CreateCompilation(code, new[] { baseRef, useMetadataReference ? derivedComp.ToMetadataReference() : derivedComp.EmitToImageReference() }); + comp.VerifyDiagnostics(); + } + [Theory] [CombinatorialData] public void EnforcedRequiredMembers_Inheritance_PartialSet(bool useMetadataReference) @@ -1676,6 +1832,52 @@ public class Derived : Base Assert.IsType(baseSymbol); } + [Fact] + public void EnforcedRequiredMembers_ThroughRetargeting_NoneSet_HasSetsRequiredMembers() + { + var retargetedCode = @"public class C {}"; + + var originalC = CreateCompilation(new AssemblyIdentity("Ret", new Version(1, 0, 0, 0), isRetargetable: true), retargetedCode, TargetFrameworkUtil.StandardReferences); + + var @base = @" +public class Base +{ + public required C Prop1 { get; set; } + public required C Field1; +} +"; + + var originalCRef = originalC.ToMetadataReference(); + var baseComp = CreateCompilationWithRequiredMembers(@base, new[] { originalCRef }, targetFramework: TargetFramework.Standard); + + var derived = @" +using System.Diagnostics.CodeAnalysis; +public class Derived : Base +{ + public required C Prop2 { get; set; } + public required C Field2; + + [SetsRequiredMembers] + public Derived() {} +} +"; + + var baseRef = baseComp.ToMetadataReference(); + var derivedComp = CreateCompilation(derived, new[] { baseRef, originalCRef }, targetFramework: TargetFramework.Standard); + + var retargetedC = CreateCompilation(new AssemblyIdentity("Ret", new Version(2, 0, 0, 0), isRetargetable: true), retargetedCode, TargetFrameworkUtil.StandardReferences); + + var code = @" +_ = new Derived(); +"; + + var comp = CreateCompilation(code, new[] { baseRef, derivedComp.ToMetadataReference(), retargetedC.ToMetadataReference() }, targetFramework: TargetFramework.Standard); + comp.VerifyDiagnostics(); + + var baseSymbol = comp.GetTypeByMetadataName("Derived"); + Assert.IsType(baseSymbol); + } + [Fact] public void EnforcedRequiredMembers_ThroughRetargeting_AllSet() { @@ -1749,6 +1951,41 @@ public class Derived : Base comp.VerifyDiagnostics(expectedDiagnostics); } + [Theory] + [CombinatorialData] + public void EnforcedRequiredMembers_Override_NoneSet_HasSetsRequiredMembers(bool useMetadataReference) + { + var @base = @" +public class Base +{ + public virtual required int Prop1 { get; set; } +} +"; + + var code = @" +using System.Diagnostics.CodeAnalysis; +_ = new Derived(); + +public class Derived : Base +{ + public override required int Prop1 { get; set; } + + [SetsRequiredMembers] + public Derived() {} +} +"; + + var comp = CreateCompilationWithRequiredMembers(new[] { @base, code }); + + comp.VerifyDiagnostics(); + + var baseComp = CreateCompilationWithRequiredMembers(@base); + baseComp.VerifyEmitDiagnostics(); + + comp = CreateCompilation(code, new[] { useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference() }); + comp.VerifyDiagnostics(); + } + [Theory] [CombinatorialData] public void EnforcedRequiredMembers_Override_PartialSet(bool useMetadataReference) @@ -1933,6 +2170,44 @@ class Derived3 : Derived { }"; ); } + [Fact] + public void EnforcedRequiredMembers_ShadowedInSource_01_HasSetsRequiredMembers() + { + var vb = @" +Imports System.Runtime.CompilerServices + +Public Class Base + + Public Property P As Integer +End Class + + +Public Class Derived + Inherits Base + + Public Shadows Property P As Integer +End Class +"; + + var vbComp = CreateVisualBasicCompilationWithRequiredMembers(vb); + CompileAndVerify(vbComp).VerifyDiagnostics(); + + var c = @" +using System.Diagnostics.CodeAnalysis; +_ = new Derived2(); + +class Derived2 : Derived +{ + [SetsRequiredMembers] + public Derived2() {} + [SetsRequiredMembers] + public Derived2(int x) {} +}"; + + var comp = CreateCompilation(c, new[] { vbComp.EmitToImageReference() }); + comp.VerifyDiagnostics(); + } + [Fact] public void EnforcedRequiredMembers_ShadowedInSource_02() { @@ -2016,6 +2291,187 @@ class Derived3 : Derived { }"; ); } + + /// + /// This IL is the equivalent of: + /// public record Derived : Base + /// { + /// public {propertyIsRequired ? required : ""} new int P { get; init; } + /// } + /// + private static string GetShadowingRecordIl(bool propertyIsRequired) + { + var propertyAttribute = propertyIsRequired ? + """ + .custom instance void [original]RequiredMemberAttribute::.ctor() = ( + 01 00 00 00 + ) + """ : ""; + return $$""" + .assembly extern original {} + + .class public auto ansi beforefieldinit Derived + extends [original]Base + implements class [mscorlib]System.IEquatable`1 + { + .custom instance void [original]RequiredMemberAttribute::.ctor() = ( + 01 00 00 00 + ) + // Fields + .field private initonly int32 '

k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( + 01 00 00 00 00 00 00 00 + ) + + // Methods + .method family hidebysig specialname virtual + instance class [mscorlib]System.Type get_EqualityContract () cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Derived::get_EqualityContract + + .method public hidebysig specialname + instance int32 get_P () cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Derived::get_P + + .method public hidebysig specialname + instance void modreq([mscorlib]System.Runtime.CompilerServices.IsExternalInit) set_P ( + int32 'value' + ) cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Derived::set_P + + .method public hidebysig virtual + instance string ToString () cil managed + { + ldnull + throw + } // end of method Derived::ToString + + .method family hidebysig virtual + instance bool PrintMembers ( + class [mscorlib]System.Text.StringBuilder builder + ) cil managed + { + ldnull + throw + } // end of method Derived::PrintMembers + + .method public hidebysig specialname static + bool op_Inequality ( + class Derived left, + class Derived right + ) cil managed + { + ldnull + throw + } // end of method Derived::op_Inequality + + .method public hidebysig specialname static + bool op_Equality ( + class Derived left, + class Derived right + ) cil managed + { + ldnull + throw + } // end of method Derived::op_Equality + + .method public hidebysig virtual + instance int32 GetHashCode () cil managed + { + ldnull + throw + } // end of method Derived::GetHashCode + + .method public hidebysig virtual + instance bool Equals ( + object obj + ) cil managed + { + ldnull + throw + } // end of method Derived::Equals + + .method public final hidebysig virtual + instance bool Equals ( + class [original]Base other + ) cil managed + { + ldnull + throw + } // end of method Derived::Equals + + .method public hidebysig newslot virtual + instance bool Equals ( + class Derived other + ) cil managed + { + ldnull + throw + } // end of method Derived::Equals + + .method public hidebysig newslot virtual + instance class Derived '$' () cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.PreserveBaseOverridesAttribute::.ctor() = ( + 01 00 00 00 + ) + .override method instance class [original]Base [original]Base::'$'() + ldnull + throw + } // end of method Derived::'$' + + .method family hidebysig specialname rtspecialname + instance void .ctor ( + class Derived original + ) cil managed + { + ldnull + throw + } // end of method Derived::.ctor + + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + ldnull + throw + } // end of method Derived::.ctor + + // Properties + .property instance class [mscorlib]System.Type EqualityContract() + { + .get instance class [mscorlib]System.Type Derived::get_EqualityContract() + } + .property instance int32 P() + { + {{propertyAttribute}} + .get instance int32 Derived::get_P() + .set instance void modreq([mscorlib]System.Runtime.CompilerServices.IsExternalInit) Derived::set_P(int32) + } + + } // end of class Derived + """; + } + [Fact] public void EnforcedRequiredMembers_ShadowedInSource_04() { @@ -2029,174 +2485,7 @@ public record Base var originalComp = CreateCompilationWithRequiredMembers(new[] { original, IsExternalInitTypeDefinition }, assemblyName: "original"); CompileAndVerify(originalComp, verify: ExecutionConditionUtil.IsCoreClr ? Verification.Passes : Verification.Skipped).VerifyDiagnostics(); - // This IL is the equivalent of: - // public record Derived : Base - // { - // public new int P { get; init; } - // } - - var ilSource = @" -.assembly extern original {} - -.class public auto ansi beforefieldinit Derived - extends [original]Base - implements class [mscorlib]System.IEquatable`1 -{ - .custom instance void [original]RequiredMemberAttribute::.ctor() = ( - 01 00 00 00 - ) - // Fields - .field private initonly int32 '

k__BackingField' - .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( - 01 00 00 00 - ) - .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( - 01 00 00 00 00 00 00 00 - ) - - // Methods - .method family hidebysig specialname virtual - instance class [mscorlib]System.Type get_EqualityContract () cil managed - { - .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( - 01 00 00 00 - ) - ldnull - throw - } // end of method Derived::get_EqualityContract - - .method public hidebysig specialname - instance int32 get_P () cil managed - { - .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( - 01 00 00 00 - ) - ldnull - throw - } // end of method Derived::get_P - - .method public hidebysig specialname - instance void modreq([mscorlib]System.Runtime.CompilerServices.IsExternalInit) set_P ( - int32 'value' - ) cil managed - { - .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( - 01 00 00 00 - ) - ldnull - throw - } // end of method Derived::set_P - - .method public hidebysig virtual - instance string ToString () cil managed - { - ldnull - throw - } // end of method Derived::ToString - - .method family hidebysig virtual - instance bool PrintMembers ( - class [mscorlib]System.Text.StringBuilder builder - ) cil managed - { - ldnull - throw - } // end of method Derived::PrintMembers - - .method public hidebysig specialname static - bool op_Inequality ( - class Derived left, - class Derived right - ) cil managed - { - ldnull - throw - } // end of method Derived::op_Inequality - - .method public hidebysig specialname static - bool op_Equality ( - class Derived left, - class Derived right - ) cil managed - { - ldnull - throw - } // end of method Derived::op_Equality - - .method public hidebysig virtual - instance int32 GetHashCode () cil managed - { - ldnull - throw - } // end of method Derived::GetHashCode - - .method public hidebysig virtual - instance bool Equals ( - object obj - ) cil managed - { - ldnull - throw - } // end of method Derived::Equals - - .method public final hidebysig virtual - instance bool Equals ( - class [original]Base other - ) cil managed - { - ldnull - throw - } // end of method Derived::Equals - - .method public hidebysig newslot virtual - instance bool Equals ( - class Derived other - ) cil managed - { - ldnull - throw - } // end of method Derived::Equals - - .method public hidebysig newslot virtual - instance class Derived '$' () cil managed - { - .custom instance void [mscorlib]System.Runtime.CompilerServices.PreserveBaseOverridesAttribute::.ctor() = ( - 01 00 00 00 - ) - .override method instance class [original]Base [original]Base::'$'() - ldnull - throw - } // end of method Derived::'$' - - .method family hidebysig specialname rtspecialname - instance void .ctor ( - class Derived original - ) cil managed - { - ldnull - throw - } // end of method Derived::.ctor - - .method public hidebysig specialname rtspecialname - instance void .ctor () cil managed - { - ldnull - throw - } // end of method Derived::.ctor - - // Properties - .property instance class [mscorlib]System.Type EqualityContract() - { - .get instance class [mscorlib]System.Type Derived::get_EqualityContract() - } - .property instance int32 P() - { - .get instance int32 Derived::get_P() - .set instance void modreq([mscorlib]System.Runtime.CompilerServices.IsExternalInit) Derived::set_P(int32) - } - -} // end of class Derived -"; + var ilSource = GetShadowingRecordIl(propertyIsRequired: false); var il = CompileIL(ilSource); @@ -2218,40 +2507,21 @@ record DerivedDerived3() : Derived; "; var comp = CreateCompilation(c, new[] { il, originalComp.EmitToImageReference() }); - // PROTOTYPE(req): do we want to take the effort to remove some of these duplicate errors? comp.VerifyDiagnostics( - // (6,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. - // record DerivedDerived1 : Derived - Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived1").WithArguments("Derived").WithLocation(6, 8), - // (6,8): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. - // record DerivedDerived1 : Derived - Diagnostic(ErrorCode.ERR_RequiredMembersInvalid, "DerivedDerived1").WithArguments("Derived").WithLocation(6, 8), // (8,12): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. // public DerivedDerived1() Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived1").WithArguments("Derived").WithLocation(8, 12), // (12,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. // record DerivedDerived2 : Derived Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived2").WithArguments("Derived").WithLocation(12, 8), - // (12,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. - // record DerivedDerived2 : Derived - Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived2").WithArguments("Derived").WithLocation(12, 8), - // (12,8): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. - // record DerivedDerived2 : Derived - Diagnostic(ErrorCode.ERR_RequiredMembersInvalid, "DerivedDerived2").WithArguments("Derived").WithLocation(12, 8), // (15,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. // record DerivedDerived3() : Derived; - Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived3").WithArguments("Derived").WithLocation(15, 8), - // (15,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. - // record DerivedDerived3() : Derived; - Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived3").WithArguments("Derived").WithLocation(15, 8), - // (15,8): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. - // record DerivedDerived3() : Derived; - Diagnostic(ErrorCode.ERR_RequiredMembersInvalid, "DerivedDerived3").WithArguments("Derived").WithLocation(15, 8) + Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived3").WithArguments("Derived").WithLocation(15, 8) ); } [Fact] - public void EnforcedRequiredMembers_ShadowedInSource_06() + public void EnforcedRequiredMembers_ShadowedInSource_04_HasSetsRequiredMembers() { var original = @" public record Base @@ -2263,178 +2533,90 @@ public record Base var originalComp = CreateCompilationWithRequiredMembers(new[] { original, IsExternalInitTypeDefinition }, assemblyName: "original"); CompileAndVerify(originalComp, verify: ExecutionConditionUtil.IsCoreClr ? Verification.Passes : Verification.Skipped).VerifyDiagnostics(); - // This IL is the equivalent of: - // public record Derived : Base - // { - // public new required int P { get; init; } - // } - - var ilSource = @" -.assembly extern original {} - -.class public auto ansi beforefieldinit Derived - extends [original]Base - implements class [mscorlib]System.IEquatable`1 -{ - .custom instance void [original]RequiredMemberAttribute::.ctor() = ( - 01 00 00 00 - ) - // Fields - .field private initonly int32 '

k__BackingField' - .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( - 01 00 00 00 - ) - .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( - 01 00 00 00 00 00 00 00 - ) - - // Methods - .method family hidebysig specialname virtual - instance class [mscorlib]System.Type get_EqualityContract () cil managed - { - .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( - 01 00 00 00 - ) - ldnull - throw - } // end of method Derived::get_EqualityContract - - .method public hidebysig specialname - instance int32 get_P () cil managed - { - .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( - 01 00 00 00 - ) - ldnull - throw - } // end of method Derived::get_P - - .method public hidebysig specialname - instance void modreq([mscorlib]System.Runtime.CompilerServices.IsExternalInit) set_P ( - int32 'value' - ) cil managed - { - .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( - 01 00 00 00 - ) - ldnull - throw - } // end of method Derived::set_P - - .method public hidebysig virtual - instance string ToString () cil managed - { - ldnull - throw - } // end of method Derived::ToString - - .method family hidebysig virtual - instance bool PrintMembers ( - class [mscorlib]System.Text.StringBuilder builder - ) cil managed - { - ldnull - throw - } // end of method Derived::PrintMembers - - .method public hidebysig specialname static - bool op_Inequality ( - class Derived left, - class Derived right - ) cil managed - { - ldnull - throw - } // end of method Derived::op_Inequality - - .method public hidebysig specialname static - bool op_Equality ( - class Derived left, - class Derived right - ) cil managed - { - ldnull - throw - } // end of method Derived::op_Equality - - .method public hidebysig virtual - instance int32 GetHashCode () cil managed - { - ldnull - throw - } // end of method Derived::GetHashCode - - .method public hidebysig virtual - instance bool Equals ( - object obj - ) cil managed - { - ldnull - throw - } // end of method Derived::Equals - - .method public final hidebysig virtual - instance bool Equals ( - class [original]Base other - ) cil managed - { - ldnull - throw - } // end of method Derived::Equals - - .method public hidebysig newslot virtual - instance bool Equals ( - class Derived other - ) cil managed - { - ldnull - throw - } // end of method Derived::Equals - - .method public hidebysig newslot virtual - instance class Derived '$' () cil managed - { - .custom instance void [mscorlib]System.Runtime.CompilerServices.PreserveBaseOverridesAttribute::.ctor() = ( - 01 00 00 00 - ) - .override method instance class [original]Base [original]Base::'$'() - ldnull - throw - } // end of method Derived::'$' + var ilSource = GetShadowingRecordIl(propertyIsRequired: false); + + var il = CompileIL(ilSource); + + var c = @" +using System.Diagnostics.CodeAnalysis; + +_ = new DerivedDerived1(); - .method family hidebysig specialname rtspecialname - instance void .ctor ( - class Derived original - ) cil managed +record DerivedDerived1 : Derived +{ + [SetsRequiredMembers] + public DerivedDerived1() { - ldnull - throw - } // end of method Derived::.ctor + } +} +"; - .method public hidebysig specialname rtspecialname - instance void .ctor () cil managed + var comp = CreateCompilation(c, new[] { il, originalComp.EmitToImageReference() }); + comp.VerifyDiagnostics(); + } + + [Fact] + public void EnforcedRequiredMembers_ShadowedInSource_04_HasSetsRequiredMembers_ManualBaseCall() { - ldnull - throw - } // end of method Derived::.ctor + var original = @" +public record Base +{ + public required int P { get; init; } +} +"; - // Properties - .property instance class [mscorlib]System.Type EqualityContract() + var originalComp = CreateCompilationWithRequiredMembers(new[] { original, IsExternalInitTypeDefinition }, assemblyName: "original"); + CompileAndVerify(originalComp, verify: ExecutionConditionUtil.IsCoreClr ? Verification.Passes : Verification.Skipped).VerifyDiagnostics(); + + var ilSource = GetShadowingRecordIl(propertyIsRequired: false); + + var il = CompileIL(ilSource); + + var c = @" +using System.Diagnostics.CodeAnalysis; + +_ = new DerivedDerived1(); + +record DerivedDerived1 : Derived +{ + [SetsRequiredMembers] + public DerivedDerived1() { - .get instance class [mscorlib]System.Type Derived::get_EqualityContract() } - .property instance int32 P() + + [SetsRequiredMembers] + public DerivedDerived1(int unused) : base(null) { - .custom instance void [original]RequiredMemberAttribute::.ctor() = ( - 01 00 00 00 - ) - .get instance int32 Derived::get_P() - .set instance void modreq([mscorlib]System.Runtime.CompilerServices.IsExternalInit) Derived::set_P(int32) } -} // end of class Derived + public DerivedDerived1(bool unused) : base(null) + { + } +} "; + var comp = CreateCompilation(c, new[] { il, originalComp.EmitToImageReference() }); + comp.VerifyDiagnostics( + // (18,12): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. + // public DerivedDerived1(bool unused) : base(null) + Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived1").WithArguments("Derived").WithLocation(18, 12) + ); + } + + [Fact] + public void EnforcedRequiredMembers_ShadowedInSource_06() + { + var original = @" +public record Base +{ + public required int P { get; init; } +} +"; + + var originalComp = CreateCompilationWithRequiredMembers(new[] { original, IsExternalInitTypeDefinition }, assemblyName: "original"); + CompileAndVerify(originalComp, verify: ExecutionConditionUtil.IsCoreClr ? Verification.Passes : Verification.Skipped).VerifyDiagnostics(); + + var ilSource = GetShadowingRecordIl(propertyIsRequired: true); var il = CompileIL(ilSource); var c = @" @@ -2455,42 +2637,58 @@ record DerivedDerived3() : Derived; "; var comp = CreateCompilation(c, new[] { il, originalComp.EmitToImageReference() }); - // PROTOTYPE(req): do we want to take the effort to remove some of these duplicate errors? comp.VerifyDiagnostics( - // (6,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. - // record DerivedDerived1 : Derived - Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived1").WithArguments("Derived").WithLocation(6, 8), - // (6,8): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. - // record DerivedDerived1 : Derived - Diagnostic(ErrorCode.ERR_RequiredMembersInvalid, "DerivedDerived1").WithArguments("Derived").WithLocation(6, 8), // (8,12): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. // public DerivedDerived1() Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived1").WithArguments("Derived").WithLocation(8, 12), // (12,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. // record DerivedDerived2 : Derived Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived2").WithArguments("Derived").WithLocation(12, 8), - // (12,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. - // record DerivedDerived2 : Derived - Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived2").WithArguments("Derived").WithLocation(12, 8), - // (12,8): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. - // record DerivedDerived2 : Derived - Diagnostic(ErrorCode.ERR_RequiredMembersInvalid, "DerivedDerived2").WithArguments("Derived").WithLocation(12, 8), - // (15,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. - // record DerivedDerived3() : Derived; - Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived3").WithArguments("Derived").WithLocation(15, 8), // (15,8): error CS9509: The required members list for the base type 'Derived' is malformed and cannot be interpreted. To use this constructor, apply the 'SetsRequiredMembers' attribute. // record DerivedDerived3() : Derived; - Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived3").WithArguments("Derived").WithLocation(15, 8), - // (15,8): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. - // record DerivedDerived3() : Derived; - Diagnostic(ErrorCode.ERR_RequiredMembersInvalid, "DerivedDerived3").WithArguments("Derived").WithLocation(15, 8) + Diagnostic(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, "DerivedDerived3").WithArguments("Derived").WithLocation(15, 8) ); } + [Fact] + public void EnforcedRequiredMembers_ShadowedInSource_06_HasSetsRequiredMembers() + { + var original = @" +public record Base +{ + public required int P { get; init; } +} +"; + + var originalComp = CreateCompilationWithRequiredMembers(new[] { original, IsExternalInitTypeDefinition }, assemblyName: "original"); + CompileAndVerify(originalComp, verify: ExecutionConditionUtil.IsCoreClr ? Verification.Passes : Verification.Skipped).VerifyDiagnostics(); + + var ilSource = GetShadowingRecordIl(propertyIsRequired: true); + var il = CompileIL(ilSource); + + var c = @" +using System.Diagnostics.CodeAnalysis; + +_ = new DerivedDerived1(); + +record DerivedDerived1 : Derived +{ + [SetsRequiredMembers] + public DerivedDerived1() + { + } +} +"; + + var comp = CreateCompilation(c, new[] { il, originalComp.EmitToImageReference() }); + comp.VerifyDiagnostics(); + } + [Fact] public void EnforcedRequiredMembers_ShadowedFromMetadata_01() { var vb = @" +Imports System.Diagnostics.CodeAnalysis Imports System.Runtime.CompilerServices Public Class Base @@ -2503,13 +2701,23 @@ Public Class Derived Inherits Base Public Shadows Property P As Integer + + Public Sub New() + End Sub + + + Public Sub New(unused As Integer) + End Sub End Class "; var vbComp = CreateVisualBasicCompilationWithRequiredMembers(vb); CompileAndVerify(vbComp).VerifyDiagnostics(); - var c = @"_ = new Derived();"; + var c = """ + _ = new Derived(); + _ = new Derived(1); + """; var comp = CreateCompilation(c, new[] { vbComp.EmitToImageReference() }); comp.VerifyDiagnostics( // (1,9): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. @@ -2522,6 +2730,7 @@ End Class public void EnforcedRequiredMembers_ShadowedFromMetadata_02() { var vb = @" +Imports System.Diagnostics.CodeAnalysis Imports System.Runtime.CompilerServices Public Class Base @@ -2533,13 +2742,23 @@ End Class Public Class Derived Inherits Base Public Shadows Property P As Integer + + Public Sub New() + End Sub + + + Public Sub New(unused As Integer) + End Sub End Class "; var vbComp = CreateVisualBasicCompilationWithRequiredMembers(vb); CompileAndVerify(vbComp).VerifyDiagnostics(); - var c = @"_ = new Derived();"; + var c = """ + _ = new Derived(); + _ = new Derived(1); + """; var comp = CreateCompilation(c, new[] { vbComp.EmitToImageReference() }); comp.VerifyDiagnostics( // (1,9): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. @@ -2552,6 +2771,7 @@ End Class public void EnforcedRequiredMembers_ShadowedFromMetadata_03() { var vb = @" +Imports System.Diagnostics.CodeAnalysis Imports System.Runtime.CompilerServices Public Class Base @@ -2562,13 +2782,23 @@ End Class Public Class Derived Inherits Base Public Shadows Property P As Integer + + Public Sub New() + End Sub + + + Public Sub New(unused As Integer) + End Sub End Class "; var vbComp = CreateVisualBasicCompilationWithRequiredMembers(vb); CompileAndVerify(vbComp).VerifyDiagnostics(); - var c = @"_ = new Derived();"; + var c = """ + _ = new Derived(); + _ = new Derived(1); + """; var comp = CreateCompilation(c, new[] { vbComp.EmitToImageReference() }); comp.VerifyDiagnostics( // (1,9): error CS9508: The required members list for 'Derived' is malformed and cannot be interpreted. @@ -2851,6 +3081,42 @@ public class RequiredMemberAttribute : Attribute ); } + [Fact] + public void SetsRequiredMemberInAttribute_Recursive() + { + var code = @" +namespace System.Diagnostics.CodeAnalysis; + +public class SetsRequiredMembersAttribute : Attribute +{ + public required int P { get; set; } + + [SetsRequiredMembers] + public SetsRequiredMembersAttribute() + { + } +} + +public class C +{ + public required int Prop { get; set; } + + [SetsRequiredMembers] + public C() + { + } + + static void M() + { + _ = new C(); + } +} +"; + + var comp = CreateCompilation(new[] { code, RequiredMemberAttribute }); + comp.VerifyDiagnostics(); + } + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] public void RequiredMemberSuppressesNullabilityWarnings_01() { @@ -3033,21 +3299,26 @@ public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_01() public class C { public required string Prop { get; set; } + public required string Field; public C(bool unused) { } public C() : this(true) { Prop.ToString(); + Field.ToString(); } } "; var comp = CreateCompilationWithRequiredMembers(code); comp.VerifyDiagnostics( - // (11,9): warning CS8602: Dereference of a possibly null reference. + // (12,9): warning CS8602: Dereference of a possibly null reference. // Prop.ToString(); - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop").WithLocation(11, 9) + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop").WithLocation(12, 9), + // (13,9): warning CS8602: Dereference of a possibly null reference. + // Field.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Field").WithLocation(13, 9) ); } @@ -3059,12 +3330,14 @@ public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_02() public struct C { public required string Prop { get; set; } + public required string Field; public C(bool unused) { } public C() : this(true) { Prop.ToString(); + Field.ToString(); } } "; @@ -3072,12 +3345,18 @@ public C() : this(true) var comp = CreateCompilationWithRequiredMembers(code); // PROTOTYPE(req): These errors should change with Rikki's DA changes. comp.VerifyDiagnostics( - // (7,12): error CS0843: Auto-implemented property 'C.Prop' must be fully assigned before control is returned to the caller. + // (8,12): error CS0171: Field 'C.Field' must be fully assigned before control is returned to the caller + // public C(bool unused) { } + Diagnostic(ErrorCode.ERR_UnassignedThis, "C").WithArguments("C.Field").WithLocation(8, 12), + // (8,12): error CS0843: Auto-implemented property 'C.Prop' must be fully assigned before control is returned to the caller. // public C(bool unused) { } - Diagnostic(ErrorCode.ERR_UnassignedThisAutoProperty, "C").WithArguments("C.Prop").WithLocation(7, 12), - // (11,9): warning CS8602: Dereference of a possibly null reference. + Diagnostic(ErrorCode.ERR_UnassignedThisAutoProperty, "C").WithArguments("C.Prop").WithLocation(8, 12), + // (12,9): warning CS8602: Dereference of a possibly null reference. // Prop.ToString(); - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop").WithLocation(11, 9) + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop").WithLocation(12, 9), + // (13,9): warning CS8602: Dereference of a possibly null reference. + // Field.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Field").WithLocation(13, 9) ); } @@ -3110,29 +3389,36 @@ public C(bool unused) : this() [InlineData("")] public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_04(string baseSyntax) { - var code = $@" + var code = $$""" #nullable enable public class Base -{{ - public required string Prop1 {{ get; set; }} - public string Prop2 {{ get; set; }} = null!; -}} +{ + public required string Prop1 { get; set; } + public string Prop2 { get; set; } = null!; + public required string Field1; + public string Field2 = null!; +} public class Derived : Base -{{ - public Derived() {baseSyntax} - {{ +{ + public Derived() {{baseSyntax}} + { Prop1.ToString(); Prop2.ToString(); - }} -}} -"; + Field1.ToString(); + Field2.ToString(); + } +} +"""; var comp = CreateCompilationWithRequiredMembers(code); comp.VerifyDiagnostics( - // (13,9): warning CS8602: Dereference of a possibly null reference. + // (14,9): warning CS8602: Dereference of a possibly null reference. // Prop1.ToString(); - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop1").WithLocation(13, 9) + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop1").WithLocation(14, 9), + // (16,9): warning CS8602: Dereference of a possibly null reference. + // Field1.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Field1").WithLocation(16, 9) ); } @@ -3141,37 +3427,51 @@ public Derived() {baseSyntax} [InlineData("")] public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_05(string baseSyntax) { - var code = @$" + var code = $$""" #nullable enable public class Base -{{ - public required string Prop1 {{ get; set; }} - public string Prop2 {{ get; set; }} = null!; -}} +{ + public required string Prop1 { get; set; } + public string Prop2 { get; set; } = null!; + public required string Field1; + public string Field2 = null!; +} public class Derived : Base -{{ - public required string Prop3 {{ get; set; }} - public string Prop4 {{ get; set; }} = null!; +{ + public required string Prop3 { get; set; } + public string Prop4 { get; set; } = null!; + public required string Field3; + public string Field4 = null!; - public Derived() {baseSyntax} - {{ + public Derived() {{baseSyntax}} + { Prop1.ToString(); // 1 Prop2.ToString(); Prop3.ToString(); // 2 Prop4.ToString(); - }} -}} -"; + Field1.ToString(); // 1 + Field2.ToString(); + Field3.ToString(); // 2 + Field4.ToString(); + } +} +"""; var comp = CreateCompilationWithRequiredMembers(code); comp.VerifyDiagnostics( - // (16,9): warning CS8602: Dereference of a possibly null reference. + // (19,9): warning CS8602: Dereference of a possibly null reference. // Prop1.ToString(); // 1 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop1").WithLocation(16, 9), - // (18,9): warning CS8602: Dereference of a possibly null reference. + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop1").WithLocation(19, 9), + // (21,9): warning CS8602: Dereference of a possibly null reference. // Prop3.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop3").WithLocation(18, 9) + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop3").WithLocation(21, 9), + // (23,9): warning CS8602: Dereference of a possibly null reference. + // Field1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Field1").WithLocation(23, 9), + // (25,9): warning CS8602: Dereference of a possibly null reference. + // Field3.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Field3").WithLocation(25, 9) ); } @@ -3255,4 +3555,77 @@ public Derived() : this(true) Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop3").WithLocation(20, 9) ); } + + [Fact] + public void SetsRequiredMembersAppliedToRecordCopyConstructor_DeclaredInType() + { + var code = """ + public record C + { + public required string Prop1 { get; set; } + public required int Field1; + } + """; + + var comp = CreateCompilationWithRequiredMembers(code); + CompileAndVerify(comp, sourceSymbolValidator: validate, symbolValidator: validate); + + void validate(ModuleSymbol module) + { + var c = module.GlobalNamespace.GetTypeMember("C"); + var copyCtor = c.GetMembers(".ctor").Cast().Single(m => m.ParameterCount == 1); + + if (copyCtor is SynthesizedRecordCopyCtor) + { + Assert.Empty(copyCtor.GetAttributes()); + } + else + { + AssertEx.Equal("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute..ctor()", + copyCtor.GetAttributes().Single(a => a.AttributeClass!.IsWellKnownSetsRequiredMembersAttribute()).AttributeConstructor.ToTestDisplayString()); + } + } + } + + [Theory] + [CombinatorialData] + public void SetsRequiredMembersAppliedToRecordCopyConstructor_DeclaredInBase(bool useMetadataReference) + { + var @base = """ + public record Base + { + public required string Prop1 { get; set; } + public required int Field1; + } + """; + + var code = """ + public record Derived : Base; + """; + + var comp = CreateCompilationWithRequiredMembers(new[] { @base, code }); + CompileAndVerify(comp, sourceSymbolValidator: validate, symbolValidator: validate); + + var baseComp = CreateCompilationWithRequiredMembers(@base); + CompileAndVerify(baseComp).VerifyDiagnostics(); + + comp = CreateCompilation(code, references: new[] { useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference() }); + CompileAndVerify(comp, sourceSymbolValidator: validate, symbolValidator: validate); + + void validate(ModuleSymbol module) + { + var c = module.GlobalNamespace.GetTypeMember("Derived"); + var copyCtor = c.GetMembers(".ctor").Cast().Single(m => m.ParameterCount == 1); + + if (copyCtor is SynthesizedRecordCopyCtor) + { + Assert.Empty(copyCtor.GetAttributes()); + } + else + { + AssertEx.Equal("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute..ctor()", + copyCtor.GetAttributes().Single(a => a.AttributeClass!.IsWellKnownSetsRequiredMembersAttribute()).AttributeConstructor.ToTestDisplayString()); + } + } + } } diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/AttributeDescription.cs b/src/Compilers/Core/Portable/Symbols/Attributes/AttributeDescription.cs index 0cc374d095026..c472cfd8df66f 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/AttributeDescription.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/AttributeDescription.cs @@ -479,5 +479,6 @@ static AttributeDescription() internal static readonly AttributeDescription InterpolatedStringHandlerAttribute = new AttributeDescription("System.Runtime.CompilerServices", "InterpolatedStringHandlerAttribute", s_signatures_HasThis_Void_Only); internal static readonly AttributeDescription InterpolatedStringHandlerArgumentAttribute = new AttributeDescription("System.Runtime.CompilerServices", "InterpolatedStringHandlerArgumentAttribute", s_signaturesOfInterpolatedStringArgumentAttribute); internal static readonly AttributeDescription RequiredMemberAttribute = new AttributeDescription("System.Runtime.CompilerServices", "RequiredMemberAttribute", s_signatures_HasThis_Void_Only); + internal static readonly AttributeDescription SetsRequiredMembersAttribute = new AttributeDescription("System.Diagnostics.CodeAnalysis", "SetsRequiredMembersAttribute", s_signatures_HasThis_Void_Only); } } diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonMethodEarlyWellKnownAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonMethodEarlyWellKnownAttributeData.cs index 94ee22f574fa9..d41640ca4e082 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonMethodEarlyWellKnownAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonMethodEarlyWellKnownAttributeData.cs @@ -52,5 +52,23 @@ public ObsoleteAttributeData? ObsoleteAttributeData } } #endregion + + #region SetsRequiredMembers + private bool _hasSetsRequiredMembers = false; + public bool HasSetsRequiredMembersAttribute + { + get + { + VerifySealed(expected: true); + return _hasSetsRequiredMembers; + } + set + { + VerifySealed(false); + _hasSetsRequiredMembers = value; + SetDataStored(); + } + } + #endregion } } diff --git a/src/Compilers/Core/Portable/WellKnownMember.cs b/src/Compilers/Core/Portable/WellKnownMember.cs index b43227dd5bd20..2a3a3c34cb98e 100644 --- a/src/Compilers/Core/Portable/WellKnownMember.cs +++ b/src/Compilers/Core/Portable/WellKnownMember.cs @@ -518,6 +518,7 @@ internal enum WellKnownMember System_Runtime_CompilerServices_DefaultInterpolatedStringHandler__ToStringAndClear, System_Runtime_CompilerServices_RequiredMemberAttribute__ctor, + System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor, Count diff --git a/src/Compilers/Core/Portable/WellKnownMembers.cs b/src/Compilers/Core/Portable/WellKnownMembers.cs index 74577d8c0ab6a..1fe3638459746 100644 --- a/src/Compilers/Core/Portable/WellKnownMembers.cs +++ b/src/Compilers/Core/Portable/WellKnownMembers.cs @@ -3542,6 +3542,13 @@ static WellKnownMembers() 0, // Method Signature (byte)SignatureTypeCode.TypeHandle, (byte)SpecialType.System_Void, // Return Type + // System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor + (byte)MemberFlags.Constructor, // Flags + (byte)WellKnownType.ExtSentinel, (byte)(WellKnownType.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute - WellKnownType.ExtSentinel), // DeclaringTypeId + 0, // Arity + 0, // Method Signature + (byte)SignatureTypeCode.TypeHandle, (byte)SpecialType.System_Void, // Return Type + }; string[] allNames = new string[(int)WellKnownMember.Count] @@ -3987,6 +3994,7 @@ static WellKnownMembers() ".ctor", // System_Text_StringBuilder__ctor "ToStringAndClear", // System_Runtime_CompilerServices_DefaultInterpolatedStringHandler__ToStringAndClear ".ctor", // System_Runtime_CompilerServices_RequiredMemberAttribute__ctor + ".ctor", // System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor }; s_descriptors = MemberDescriptor.InitializeFromStream(new System.IO.MemoryStream(initializationBytes, writable: false), allNames); diff --git a/src/Compilers/Core/Portable/WellKnownTypes.cs b/src/Compilers/Core/Portable/WellKnownTypes.cs index fd3366365333f..91064608d6142 100644 --- a/src/Compilers/Core/Portable/WellKnownTypes.cs +++ b/src/Compilers/Core/Portable/WellKnownTypes.cs @@ -318,6 +318,7 @@ internal enum WellKnownType System_ArgumentNullException, System_Runtime_CompilerServices_RequiredMemberAttribute, + System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute, NextAvailable, // Remember to update the AllWellKnownTypes tests when making changes here @@ -628,7 +629,8 @@ internal static class WellKnownTypes "System.Runtime.CompilerServices.DefaultInterpolatedStringHandler", "System.ArgumentNullException", - "System.Runtime.CompilerServices.RequiredMemberAttribute" + "System.Runtime.CompilerServices.RequiredMemberAttribute", + "System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute", }; private static readonly Dictionary s_nameToTypeIdMap = new Dictionary((int)Count); diff --git a/src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs b/src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs index 953e494b360f7..8241509bd0d1f 100644 --- a/src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs +++ b/src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs @@ -636,6 +636,19 @@ public RequiredMemberAttribute() } } } +"; + + protected const string SetsRequiredMembersAttribute = @" +namespace System.Diagnostics.CodeAnalysis +{ + [AttributeUsage(AttributeTargets.Constructor, Inherited = false, AllowMultiple = false)] + public class SetsRequiredMembersAttribute : Attribute + { + public SetsRequiredMembersAttribute() + { + } + } +} "; protected static CSharpCompilationOptions WithNullableEnable(CSharpCompilationOptions options = null) From a4dd570f4d77dc961fc409028a087035a9b3d8ce Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Mon, 28 Mar 2022 14:23:40 -0700 Subject: [PATCH 02/12] Correct test issues. --- .../Compiler/AnonymousTypeMethodBodySynthesizer.cs | 2 ++ .../AnonymousType.SynthesizedMethodBase.cs | 2 +- .../Test/Symbol/Symbols/MissingSpecialMember.cs | 2 ++ .../SymbolsTests/WellKnownTypeValidationTests.vb | 12 ++++++++---- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compiler/AnonymousTypeMethodBodySynthesizer.cs b/src/Compilers/CSharp/Portable/Compiler/AnonymousTypeMethodBodySynthesizer.cs index 9d9fac748c464..86eb7b3a6f942 100644 --- a/src/Compilers/CSharp/Portable/Compiler/AnonymousTypeMethodBodySynthesizer.cs +++ b/src/Compilers/CSharp/Portable/Compiler/AnonymousTypeMethodBodySynthesizer.cs @@ -69,6 +69,8 @@ internal override bool HasSpecialName { get { return true; } } + + protected override bool HasSetsRequiredMembersImpl => false; } private sealed partial class AnonymousTypePropertyGetAccessorSymbol : SynthesizedMethodBase diff --git a/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.SynthesizedMethodBase.cs b/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.SynthesizedMethodBase.cs index 41e9f8a26619e..b9fcf514dd8f9 100644 --- a/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.SynthesizedMethodBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.SynthesizedMethodBase.cs @@ -232,7 +232,7 @@ internal sealed override int CalculateLocalSyntaxOffset(int localPosition, Synta throw ExceptionUtilities.Unreachable; } - protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; + protected override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/MissingSpecialMember.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/MissingSpecialMember.cs index 1b7aff577023d..44f46608967e5 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/MissingSpecialMember.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/MissingSpecialMember.cs @@ -611,6 +611,7 @@ public void AllWellKnownTypes() case WellKnownType.System_Runtime_CompilerServices_IsExternalInit: case WellKnownType.System_Runtime_CompilerServices_DefaultInterpolatedStringHandler: case WellKnownType.System_Runtime_CompilerServices_RequiredMemberAttribute: + case WellKnownType.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute: // Not yet in the platform. continue; case WellKnownType.Microsoft_CodeAnalysis_Runtime_Instrumentation: @@ -966,6 +967,7 @@ public void AllWellKnownTypeMembers() case WellKnownMember.System_Runtime_CompilerServices_NativeIntegerAttribute__ctorTransformFlags: case WellKnownMember.System_Runtime_CompilerServices_DefaultInterpolatedStringHandler__ToStringAndClear: case WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor: + case WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor: // Not yet in the platform. continue; case WellKnownMember.Microsoft_CodeAnalysis_Runtime_Instrumentation__CreatePayloadForMethodsSpanningSingleFile: diff --git a/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb b/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb index 4c32b28a689e8..24617a76c8c71 100644 --- a/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb +++ b/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb @@ -545,7 +545,8 @@ End Namespace WellKnownType.System_Runtime_CompilerServices_NativeIntegerAttribute, WellKnownType.System_Runtime_CompilerServices_IsExternalInit, WellKnownType.System_Runtime_CompilerServices_DefaultInterpolatedStringHandler, - WellKnownType.System_Runtime_CompilerServices_RequiredMemberAttribute + WellKnownType.System_Runtime_CompilerServices_RequiredMemberAttribute, + WellKnownType.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute ' Not available on all platforms. Continue For @@ -613,7 +614,8 @@ End Namespace WellKnownType.System_Runtime_CompilerServices_NativeIntegerAttribute, WellKnownType.System_Runtime_CompilerServices_IsExternalInit, WellKnownType.System_Runtime_CompilerServices_DefaultInterpolatedStringHandler, - WellKnownType.System_Runtime_CompilerServices_RequiredMemberAttribute + WellKnownType.System_Runtime_CompilerServices_RequiredMemberAttribute, + WellKnownType.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute ' Not available on all platforms. Continue For Case WellKnownType.ExtSentinel @@ -700,7 +702,8 @@ End Namespace WellKnownMember.System_Runtime_CompilerServices_NativeIntegerAttribute__ctor, WellKnownMember.System_Runtime_CompilerServices_NativeIntegerAttribute__ctorTransformFlags, WellKnownMember.System_Runtime_CompilerServices_DefaultInterpolatedStringHandler__ToStringAndClear, - WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor + WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor, + WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor: ' Not available yet, but will be in upcoming release. Continue For Case WellKnownMember.Microsoft_CodeAnalysis_Runtime_Instrumentation__CreatePayloadForMethodsSpanningSingleFile, @@ -843,7 +846,8 @@ End Namespace WellKnownMember.System_Runtime_CompilerServices_NativeIntegerAttribute__ctor, WellKnownMember.System_Runtime_CompilerServices_NativeIntegerAttribute__ctorTransformFlags, WellKnownMember.System_Runtime_CompilerServices_DefaultInterpolatedStringHandler__ToStringAndClear, - WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor + WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor, + WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor: ' Not available yet, but will be in upcoming release. Continue For Case WellKnownMember.Microsoft_CodeAnalysis_Runtime_Instrumentation__CreatePayloadForMethodsSpanningSingleFile, From a9aef94e5715688a343738fd77483bc70cc181fd Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Mon, 28 Mar 2022 15:35:01 -0700 Subject: [PATCH 03/12] Fix EE compile. --- .../CSharp/Source/ExpressionCompiler/Symbols/EEMethodSymbol.cs | 2 ++ .../ExpressionCompiler/Symbols/PlaceholderMethodSymbol.cs | 2 ++ .../Symbols/SynthesizedContextMethodSymbol.cs | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/EEMethodSymbol.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/EEMethodSymbol.cs index 0038351ba2599..0c45d02ea88f1 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/EEMethodSymbol.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/EEMethodSymbol.cs @@ -722,5 +722,7 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l } internal override bool IsNullableAnalysisEnabled() => false; + + protected override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/PlaceholderMethodSymbol.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/PlaceholderMethodSymbol.cs index bafe42bc6c090..bed4b97588505 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/PlaceholderMethodSymbol.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/PlaceholderMethodSymbol.cs @@ -273,6 +273,8 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l internal override bool IsNullableAnalysisEnabled() => false; + protected override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; + #if DEBUG protected override MethodSymbolAdapter CreateCciAdapter() { diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/SynthesizedContextMethodSymbol.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/SynthesizedContextMethodSymbol.cs index 8d4af67a7dd76..5121e63935310 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/SynthesizedContextMethodSymbol.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/SynthesizedContextMethodSymbol.cs @@ -218,5 +218,7 @@ internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChang { throw ExceptionUtilities.Unreachable; } + + protected override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable; } } From 95d08c1e44e14e70e91fc6d0d3a0d2becd5a88e5 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 29 Mar 2022 14:41:51 -0700 Subject: [PATCH 04/12] Fix formatting, add additional tests. --- .../Symbol/Symbols/RequiredMembersTests.cs | 28 +++++++++++-------- .../WellKnownTypeValidationTests.vb | 4 +-- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 5463fe8577d55..15ee32e7fe07f 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -1275,19 +1275,21 @@ public class C [Theory] [CombinatorialData] - public void EnforcedRequiredMembers_NoInheritance_NoneSet_HasSetsRequiredMembers(bool useMetadataReference, [CombinatorialValues("", " C")] string constructor) + public void EnforcedRequiredMembers_NoInheritance_NoneSet_HasSetsRequiredMembers(bool useMetadataReference, [CombinatorialValues("", " C")] string constructor, [CombinatorialValues("", "method: ")] string target) { - var c = @" -using System.Diagnostics.CodeAnalysis; -public class C -{ - public required int Prop { get; set; } - public required int Field; +#pragma warning disable IDE0055 // Fix formatting + var c = $$""" + using System.Diagnostics.CodeAnalysis; + public class C + { + public required int Prop { get; set; } + public required int Field; - [SetsRequiredMembers] - public C() {} -} -"; + [{{target}}SetsRequiredMembers] + public C() {} + } + """; +#pragma warning restore IDE0055 // Fix formatting var creation = $@"C c = new{constructor}();"; var comp = CreateCompilationWithRequiredMembers(new[] { c, creation }); @@ -2307,6 +2309,7 @@ private static string GetShadowingRecordIl(bool propertyIsRequired) 01 00 00 00 ) """ : ""; +#pragma warning disable IDE0055 // Fix formatting return $$""" .assembly extern original {} @@ -2470,6 +2473,7 @@ .set instance void modreq([mscorlib]System.Runtime.CompilerServices.IsExternalIn } // end of class Derived """; +#pragma warning restore IDE0055 // Fix formatting } [Fact] @@ -3389,6 +3393,7 @@ public C(bool unused) : this() [InlineData("")] public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_04(string baseSyntax) { +#pragma warning disable IDE0055 // Fix formatting var code = $$""" #nullable enable public class Base @@ -3410,6 +3415,7 @@ public Derived() {{baseSyntax}} } } """; +#pragma warning restore IDE0055 // Fix formatting var comp = CreateCompilationWithRequiredMembers(code); comp.VerifyDiagnostics( diff --git a/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb b/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb index 24617a76c8c71..6596294ce252e 100644 --- a/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb +++ b/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb @@ -703,7 +703,7 @@ End Namespace WellKnownMember.System_Runtime_CompilerServices_NativeIntegerAttribute__ctorTransformFlags, WellKnownMember.System_Runtime_CompilerServices_DefaultInterpolatedStringHandler__ToStringAndClear, WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor, - WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor: + WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor ' Not available yet, but will be in upcoming release. Continue For Case WellKnownMember.Microsoft_CodeAnalysis_Runtime_Instrumentation__CreatePayloadForMethodsSpanningSingleFile, @@ -847,7 +847,7 @@ End Namespace WellKnownMember.System_Runtime_CompilerServices_NativeIntegerAttribute__ctorTransformFlags, WellKnownMember.System_Runtime_CompilerServices_DefaultInterpolatedStringHandler__ToStringAndClear, WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor, - WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor: + WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor ' Not available yet, but will be in upcoming release. Continue For Case WellKnownMember.Microsoft_CodeAnalysis_Runtime_Instrumentation__CreatePayloadForMethodsSpanningSingleFile, From d9da29b4f864eb1cad79474e663a2b6f8dad83a1 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 29 Mar 2022 15:12:23 -0700 Subject: [PATCH 05/12] Fix used assembly references leg. --- src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index fa62c3d18bfb0..dad5cf02dfd9a 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -2142,7 +2142,7 @@ private static BoundCall GenerateBaseCopyConstructorInitializer(SynthesizedRecor return null; } - var constructorUseSiteInfo = CompoundUseSiteInfo.DiscardedDependencies; + var constructorUseSiteInfo = new CompoundUseSiteInfo(diagnostics, constructor.ContainingAssembly); constructorUseSiteInfo.Add(baseConstructor.GetUseSiteInfo()); if (Binder.ReportConstructorUseSiteDiagnostics(diagnosticsLocation, diagnostics, suppressUnsupportedRequiredMembersError: constructor.HasSetsRequiredMembers, constructorUseSiteInfo)) { From 72c3927363d7f90b06dda1ac7ccf0836fac995ee Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 29 Mar 2022 15:19:38 -0700 Subject: [PATCH 06/12] PR feedback. --- src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs index f5d9ea7070f6b..265eb3b805ae4 100644 --- a/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs @@ -843,6 +843,6 @@ internal static bool HasAsyncMethodBuilderAttribute(this Symbol symbol, [NotNull internal static bool IsRequired(this Symbol symbol) => symbol is FieldSymbol { IsRequired: true } or PropertySymbol { IsRequired: true }; internal static bool ShouldCheckRequiredMembers(this MethodSymbol method) - => method.MethodKind == MethodKind.Constructor && !method.HasSetsRequiredMembers; + => !method.HasSetsRequiredMembers; } } From 49a53a5a0e0c7ddbdc435c16e66d5b628bbd6f3b Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 29 Mar 2022 16:34:11 -0700 Subject: [PATCH 07/12] Readd condition. --- src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs index 265eb3b805ae4..7392e9e350ffd 100644 --- a/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs @@ -843,6 +843,6 @@ internal static bool HasAsyncMethodBuilderAttribute(this Symbol symbol, [NotNull internal static bool IsRequired(this Symbol symbol) => symbol is FieldSymbol { IsRequired: true } or PropertySymbol { IsRequired: true }; internal static bool ShouldCheckRequiredMembers(this MethodSymbol method) - => !method.HasSetsRequiredMembers; + => method is { MethodKind: MethodKind.Constructor, HasSetsRequiredMembers: false }; } } From 39848208dbffa38c6c44a9309f0c404d039ea12c Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 5 Apr 2022 17:20:24 -0700 Subject: [PATCH 08/12] Use SuppressRequiredMembers to inform nullable analysis of whether properties should be considered initialized. --- .../Portable/Binder/Binder_Initializers.cs | 1 - .../Compilation/MethodBodySemanticModel.cs | 2 +- .../Portable/Compiler/MethodCompiler.cs | 84 +++++---- .../Portable/FlowAnalysis/NullableWalker.cs | 170 +++++++++++++----- .../Symbol/Symbols/RequiredMembersTests.cs | 81 +++++++++ .../Portable/Binding/BindingDiagnosticBag.cs | 2 + 6 files changed, 267 insertions(+), 73 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Initializers.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Initializers.cs index 3cf2cf7bc0d82..ee8a2c9913f0b 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Initializers.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Initializers.cs @@ -18,7 +18,6 @@ internal struct ProcessedFieldInitializers { internal ImmutableArray BoundInitializers { get; set; } internal BoundStatement? LoweredInitializers { get; set; } - internal NullableWalker.VariableState AfterInitializersState; internal bool HasErrors { get; set; } internal ImportChain? FirstImportChain { get; set; } } diff --git a/src/Compilers/CSharp/Portable/Compilation/MethodBodySemanticModel.cs b/src/Compilers/CSharp/Portable/Compilation/MethodBodySemanticModel.cs index 7b425fcee19c8..eba9d23ee4424 100644 --- a/src/Compilers/CSharp/Portable/Compilation/MethodBodySemanticModel.cs +++ b/src/Compilers/CSharp/Portable/Compilation/MethodBodySemanticModel.cs @@ -304,7 +304,7 @@ protected override BoundNode RewriteNullableBoundNodesWithSnapshots( out NullableWalker.SnapshotManager snapshotManager, ref ImmutableDictionary remappedSymbols) { - var afterInitializersState = NullableWalker.GetAfterInitializersState(Compilation, MemberSymbol); + var afterInitializersState = NullableWalker.GetAfterInitializersState(Compilation, MemberSymbol, boundRoot); return NullableWalker.AnalyzeAndRewrite(Compilation, MemberSymbol, boundRoot, binder, afterInitializersState, diagnostics, createSnapshots, out snapshotManager, ref remappedSymbols); } diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index dad5cf02dfd9a..a57cb24a93689 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -625,6 +625,7 @@ private void CompileNamedType(NamedTypeSymbol containingType) useConstructorExitWarnings: true, initialNullableState: null, getFinalNullableState: false, + baseOrThisInitializer: null, finalNullableState: out _); } } @@ -1011,7 +1012,8 @@ private void CompileMethod( useConstructorExitWarnings: false, initialNullableState: null, getFinalNullableState: true, - out processedInitializers.AfterInitializersState); + baseOrThisInitializer: null, + out _); } var unusedDiagnostics = DiagnosticBag.GetInstance(); @@ -1031,28 +1033,12 @@ private void CompileMethod( processedInitializers.HasErrors = processedInitializers.HasErrors || analyzedInitializers.HasAnyErrors; } - if (includeInitializersInBody && - processedInitializers.AfterInitializersState is null && - ReportNullableDiagnostics) - { - NullableWalker.AnalyzeIfNeeded( - _compilation, - methodSymbol, - // we analyze to produce an AfterInitializersState even if there are no initializers - // because it conveniently allows us to capture all the 'default' states for applicable members - analyzedInitializers ?? GetSynthesizedEmptyBody(methodSymbol), - diagsForCurrentMethod.DiagnosticBag, - useConstructorExitWarnings: false, - initialNullableState: null, - getFinalNullableState: true, - out processedInitializers.AfterInitializersState); - } - body = BindMethodBody( methodSymbol, compilationState, diagsForCurrentMethod, - processedInitializers.AfterInitializersState, + includeInitializersInBody, + analyzedInitializers, ReportNullableDiagnostics, includesFieldInitializers: includeInitializersInBody && !processedInitializers.BoundInitializers.IsEmpty, out importChain, @@ -1702,20 +1688,32 @@ private static void GetStateMachineSlotDebugInfo( } // NOTE: can return null if the method has no body. - internal static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationState compilationState, BindingDiagnosticBag diagnostics) +#nullable enable + internal static BoundBlock? BindMethodBody(MethodSymbol method, TypeCompilationState compilationState, BindingDiagnosticBag diagnostics) { - return BindMethodBody(method, compilationState, diagnostics, nullableInitialState: null, reportNullableDiagnostics: true, includesFieldInitializers: false, out _, out _, out _); + return BindMethodBody( + method, + compilationState, + diagnostics, + includeInitializersInBody: false, + initializersBody: null, + reportNullableDiagnostics: true, + includesFieldInitializers: false, + importChain: out _, + originalBodyNested: out _, + forSemanticModel: out _); } // NOTE: can return null if the method has no body. - private static BoundBlock BindMethodBody( + private static BoundBlock? BindMethodBody( MethodSymbol method, TypeCompilationState compilationState, BindingDiagnosticBag diagnostics, - NullableWalker.VariableState nullableInitialState, + bool includeInitializersInBody, + BoundNode? initializersBody, bool reportNullableDiagnostics, bool includesFieldInitializers, - out ImportChain importChain, + out ImportChain? importChain, out bool originalBodyNested, out MethodBodySemanticModel.InitialState forSemanticModel) { @@ -1723,11 +1721,15 @@ private static BoundBlock BindMethodBody( importChain = null; forSemanticModel = default; - BoundBlock body; + BoundBlock? body; + NullableWalker.VariableState? nullableInitialState = null; + + initializersBody ??= GetSynthesizedEmptyBody(method); if (method is SynthesizedRecordConstructor recordStructPrimaryCtor && method.ContainingType.IsRecordStruct) { body = BoundBlock.SynthesizedNoLocals(recordStructPrimaryCtor.GetSyntax()); + nullableInitialState = getInitializerState(body); } else if (method is SourceMemberMethodSymbol sourceMethod) { @@ -1756,12 +1758,15 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && importChain = bodyBinder.ImportChain; BoundNode methodBody = bodyBinder.BindMethodBody(syntaxNode, diagnostics, includesFieldInitializers); BoundNode methodBodyForSemanticModel = methodBody; - NullableWalker.SnapshotManager snapshotManager = null; - ImmutableDictionary remappedSymbols = null; + NullableWalker.SnapshotManager? snapshotManager = null; + ImmutableDictionary? remappedSymbols = null; var compilation = bodyBinder.Compilation; + nullableInitialState = getInitializerState(methodBody); + if (reportNullableDiagnostics) { + Debug.Assert(diagnostics.DiagnosticBag != null); if (compilation.IsNullableAnalysisEnabledIn(method)) { var isSufficientLangVersion = compilation.LanguageVersion >= MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion(); @@ -1789,17 +1794,18 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && useConstructorExitWarnings: true, nullableInitialState, getFinalNullableState: false, + baseOrThisInitializer: null, finalNullableState: out _); } } - forSemanticModel = new MethodBodySemanticModel.InitialState(syntaxNode, methodBodyForSemanticModel, bodyBinder, snapshotManager, remappedSymbols); switch (methodBody.Kind) { case BoundKind.ConstructorMethodBody: var constructor = (BoundConstructorMethodBody)methodBody; - body = constructor.BlockBody ?? constructor.ExpressionBody; + body = constructor.BlockBody ?? constructor.ExpressionBody!; + Debug.Assert(body != null); if (constructor.Initializer is BoundNoOpStatement) { @@ -1832,7 +1838,8 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && case BoundKind.NonConstructorMethodBody: var nonConstructor = (BoundNonConstructorMethodBody)methodBody; - body = nonConstructor.BlockBody ?? nonConstructor.ExpressionBody; + body = nonConstructor.BlockBody ?? nonConstructor.ExpressionBody!; + Debug.Assert(body != null); break; case BoundKind.Block: @@ -1845,7 +1852,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && else { var property = sourceMethod.AssociatedSymbol as SourcePropertySymbolBase; - if ((object)property != null && property.IsAutoPropertyWithGetAccessor) + if (property is not null && property.IsAutoPropertyWithGetAccessor) { return MethodBodySynthesizer.ConstructAutoPropertyAccessorBody(sourceMethod); } @@ -1862,15 +1869,18 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && var stmts = ArrayBuilder.GetInstance(); ctor.GenerateMethodBodyStatements(factory, stmts, diagnostics); body = BoundBlock.SynthesizedNoLocals(node, stmts.ToImmutableAndFree()); + nullableInitialState = getInitializerState(body); } else { // synthesized methods should return their bound bodies body = null; + nullableInitialState = getInitializerState(null); } if (reportNullableDiagnostics && method.IsConstructor() && method.IsImplicitlyDeclared && nullableInitialState is object) { + Debug.Assert(diagnostics.AccumulatesDiagnostics); NullableWalker.AnalyzeIfNeeded( compilationState.Compilation, method, @@ -1879,6 +1889,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && useConstructorExitWarnings: true, nullableInitialState, getFinalNullableState: false, + baseOrThisInitializer: null, finalNullableState: out _); } @@ -1910,7 +1921,18 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && } return BoundBlock.SynthesizedNoLocals(method.GetNonNullSyntaxNode(), statements); + + NullableWalker.VariableState? getInitializerState(BoundNode? body) + { + if (reportNullableDiagnostics && includeInitializersInBody) + { + return NullableWalker.GetAfterInitializersState(compilationState.Compilation, method, initializersBody, body, diagnostics); + } + + return null; + } } +#nullable disable private static BoundBlock GetSynthesizedEmptyBody(Symbol symbol) { diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 10b7730eefdb7..46d27bd41f6c8 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -11,6 +11,7 @@ using System.Linq; using System.Runtime.CompilerServices; using System.Text; +using Microsoft.CodeAnalysis.Collections; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; @@ -235,6 +236,8 @@ private PooledDictionary private readonly bool _hasInitialState; + private readonly MethodSymbol? _baseOrThisInitializer; + #if DEBUG ///

/// Contains the expressions that should not be inserted into . @@ -418,6 +421,7 @@ private NullableWalker( Binder binder, Conversions conversions, Variables? variables, + MethodSymbol? baseOrThisInitializer, ArrayBuilder<(BoundReturnStatement, TypeWithAnnotations)>? returnTypesOpt, ImmutableDictionary.Builder? analyzedNullabilityMapOpt, SnapshotManager.Builder? snapshotBuilderOpt, @@ -425,6 +429,7 @@ private NullableWalker( : base(compilation, symbol, node, EmptyStructTypeCache.CreatePrecise(), trackUnassignments: true) { Debug.Assert(!useDelegateInvokeParameterTypes || delegateInvokeMethodOpt is object); + Debug.Assert(baseOrThisInitializer is null or { MethodKind: MethodKind.Constructor }); _variables = variables ?? Variables.Create(symbol); _binder = binder; @@ -438,6 +443,7 @@ private NullableWalker( _snapshotBuilderOpt = snapshotBuilderOpt; _isSpeculative = isSpeculative; _hasInitialState = variables is { }; + _baseOrThisInitializer = baseOrThisInitializer; } public string GetDebuggerDisplay() @@ -880,40 +886,83 @@ ImmutableArray getMembersNeedingDefaultInitialState() return ImmutableArray.Empty; } + bool includeRequiredMembers = true; + bool includeBaseRequiredMembers = true; + bool hasThisConstructorInitializer = false; + + if (method is SourceMemberMethodSymbol { SyntaxNode: ConstructorDeclarationSyntax { Initializer: { RawKind: var initializerKind } } }) + { + var baseOrThisInitializer = (_baseOrThisInitializer ?? GetConstructorThisOrBaseSymbol(this.methodMainNode)); + // If there's an error in the base or this initializer, presume that we should set all required members to default. + includeBaseRequiredMembers = baseOrThisInitializer?.ShouldCheckRequiredMembers() ?? true; + if (initializerKind == (int)SyntaxKind.ThisConstructorInitializer) + { + hasThisConstructorInitializer = true; + includeRequiredMembers = includeBaseRequiredMembers; + } + else if (initializerKind == (int)SyntaxKind.BaseConstructorInitializer) + { + includeRequiredMembers = true; + } + } + // We don't use a default initial state for value type instance constructors without `: this()` because // any usages of uninitialized fields will get definite assignment errors anyway. - if (!method.HasThisConstructorInitializer(out _) && (!method.ContainingType.IsValueType || method.IsStatic)) + if (!hasThisConstructorInitializer && (!method.ContainingType.IsValueType || method.IsStatic)) { - return membersToBeInitialized(method.ContainingType, includeAllMembers: true); + return membersToBeInitialized(method.ContainingType, includeAllMembers: true, includeRequiredMembers, includeBaseRequiredMembers); } // We want to presume all required members of the type are uninitialized, and in addition we want to set all fields to // default if we can get to this constructor by doing so (ie, : this() in a value type). - return membersToBeInitialized(method.ContainingType, includeAllMembers: method.IncludeFieldInitializersInBody()); + return membersToBeInitialized(method.ContainingType, includeAllMembers: method.IncludeFieldInitializersInBody(), includeRequiredMembers, includeBaseRequiredMembers); - static ImmutableArray membersToBeInitialized(NamedTypeSymbol containingType, bool includeAllMembers) + static ImmutableArray membersToBeInitialized(NamedTypeSymbol containingType, bool includeAllMembers, bool includeRequiredMembers, bool includeBaseRequiredMembers) { - var requiredMembers = containingType.AllRequiredMembers; - var members = includeAllMembers ? containingType.GetMembersUnordered() : ImmutableArray.Empty; - if (requiredMembers.IsEmpty) + return (includeAllMembers, includeRequiredMembers, includeBaseRequiredMembers) switch { - return members; - } + (includeAllMembers: false, includeRequiredMembers: false, includeBaseRequiredMembers: false) + => ImmutableArray.Empty, + + (includeAllMembers: false, includeRequiredMembers: true, includeBaseRequiredMembers: false) + => containingType.GetMembersUnordered().SelectAsArray(predicate: SymbolExtensions.IsRequired, selector: getFieldSymbolToBeInitialized), + + (includeAllMembers: false, includeRequiredMembers: true, includeBaseRequiredMembers: true) + => containingType.AllRequiredMembers.SelectAsArray(static kvp => getFieldSymbolToBeInitialized(kvp.Value)), - var builder = ArrayBuilder.GetInstance(members.Length + requiredMembers.Count); - builder.AddRange(members); - foreach (var (_, requiredMember) in requiredMembers) + (includeAllMembers: true, includeRequiredMembers: _, includeBaseRequiredMembers: false) + => containingType.GetMembersUnordered().SelectAsArray(getFieldSymbolToBeInitialized), + + (includeAllMembers: true, includeRequiredMembers: true, includeBaseRequiredMembers: true) + => getAllTypeAndRequiredMembers(), + + (includeAllMembers: _, includeRequiredMembers: false, includeBaseRequiredMembers: true) + => throw ExceptionUtilities.Unreachable, + }; + + ImmutableArray getAllTypeAndRequiredMembers() { - if (!members.IsEmpty && requiredMember.ContainingType.Equals(containingType, TypeCompareKind.ConsiderEverything)) + var members = containingType.GetMembersUnordered(); + var requiredMembers = containingType.BaseTypeNoUseSiteDiagnostics?.AllRequiredMembers ?? ImmutableSegmentedDictionary.Empty; + + if (requiredMembers.IsEmpty) { - continue; + return members; } - // We want to assume that all required members were _not_ set by the chained constructor - builder.Add(requiredMember is SourcePropertySymbol { IsAutoPropertyWithGetAccessor: true } prop ? prop.BackingField : requiredMember); + var builder = ArrayBuilder.GetInstance(members.Length + requiredMembers.Count); + builder.AddRange(members); + foreach (var (_, requiredMember) in requiredMembers) + { + // We want to assume that all required members were _not_ set by the chained constructor + builder.Add(getFieldSymbolToBeInitialized(requiredMember)); + } + + return builder.ToImmutableAndFree(); } - return builder.ToImmutableAndFree(); + static Symbol getFieldSymbolToBeInitialized(Symbol requiredMember) + => requiredMember is SourcePropertySymbol { IsAutoPropertyWithGetAccessor: true } prop ? prop.BackingField : requiredMember; } } } @@ -1157,6 +1206,7 @@ internal static void AnalyzeIfNeeded( bool useConstructorExitWarnings, VariableState? initialNullableState, bool getFinalNullableState, + MethodSymbol? baseOrThisInitializer, out VariableState? finalNullableState) { if (!HasRequiredLanguageVersion(compilation) || !compilation.IsNullableAnalysisEnabledIn(method)) @@ -1166,13 +1216,13 @@ internal static void AnalyzeIfNeeded( // Once we address https://github.com/dotnet/roslyn/issues/46579 we should also always pass `getFinalNullableState: true` in debug mode. // We will likely always need to write a 'null' out for the out parameter in this code path, though, because // we don't want to introduce behavior differences between debug and release builds - Analyze(compilation, method, node, new DiagnosticBag(), useConstructorExitWarnings: false, initialNullableState: null, getFinalNullableState: false, out _, requiresAnalysis: false); + Analyze(compilation, method, node, new DiagnosticBag(), useConstructorExitWarnings: false, initialNullableState: null, getFinalNullableState: false, baseOrThisInitializer, out _, requiresAnalysis: false); } finalNullableState = null; return; } - Analyze(compilation, method, node, diagnostics, useConstructorExitWarnings, initialNullableState, getFinalNullableState, out finalNullableState); + Analyze(compilation, method, node, diagnostics, useConstructorExitWarnings, initialNullableState, getFinalNullableState, baseOrThisInitializer, out finalNullableState); } private static void Analyze( @@ -1183,6 +1233,7 @@ private static void Analyze( bool useConstructorExitWarnings, VariableState? initialNullableState, bool getFinalNullableState, + MethodSymbol? baseOrThisInitializer, out VariableState? finalNullableState, bool requiresAnalysis = true) { @@ -1207,6 +1258,7 @@ private static void Analyze( useDelegateInvokeReturnType: false, delegateInvokeMethodOpt: null, initialState: initialNullableState, + baseOrThisInitializer, analyzedNullabilityMapOpt: null, snapshotBuilderOpt: null, returnTypesOpt: null, @@ -1215,36 +1267,67 @@ private static void Analyze( requiresAnalysis); } - /// - /// Gets the "after initializers state" which should be used at the beginning of nullable analysis - /// of certain constructors. Only used for semantic model and debug verification. - /// - internal static VariableState? GetAfterInitializersState(CSharpCompilation compilation, Symbol? symbol) + internal static VariableState? GetAfterInitializersState(CSharpCompilation compilation, Symbol? symbol, BoundNode constructorBody) { if (symbol is MethodSymbol method && method.IncludeFieldInitializersInBody() && method.ContainingType is SourceMemberContainerTypeSymbol containingType) { - var unusedDiagnostics = DiagnosticBag.GetInstance(); + Binder.ProcessedFieldInitializers discardedInitializers = default; + Binder.BindFieldInitializers(compilation, null, method.IsStatic ? containingType.StaticInitializers : containingType.InstanceInitializers, BindingDiagnosticBag.Discarded, ref discardedInitializers); + return GetAfterInitializersState(compilation, method, InitializerRewriter.RewriteConstructor(discardedInitializers.BoundInitializers, method), constructorBody, diagnostics: BindingDiagnosticBag.Discarded); + } - Binder.ProcessedFieldInitializers initializers = default; - Binder.BindFieldInitializers(compilation, null, method.IsStatic ? containingType.StaticInitializers : containingType.InstanceInitializers, BindingDiagnosticBag.Discarded, ref initializers); - NullableWalker.AnalyzeIfNeeded( - compilation, - method, - InitializerRewriter.RewriteConstructor(initializers.BoundInitializers, method), - unusedDiagnostics, - useConstructorExitWarnings: false, - initialNullableState: null, - getFinalNullableState: true, - out var afterInitializersState); + return null; + } - unusedDiagnostics.Free(); + /// + /// Gets the "after initializers state" which should be used at the beginning of nullable analysis + /// of certain constructors. + /// + internal static VariableState? GetAfterInitializersState(CSharpCompilation compilation, MethodSymbol method, BoundNode nodeToAnalyze, BoundNode? constructorBody, BindingDiagnosticBag diagnostics) + { + Debug.Assert(method.IsConstructor()); + bool ownsDiagnostics; + DiagnosticBag diagnosticsBag; + if (diagnostics.DiagnosticBag == null) + { + diagnostics = BindingDiagnosticBag.Discarded; + diagnosticsBag = DiagnosticBag.GetInstance(); + ownsDiagnostics = true; + } + else + { + diagnosticsBag = diagnostics.DiagnosticBag; + ownsDiagnostics = false; + } + + MethodSymbol? baseOrThisInitializer = GetConstructorThisOrBaseSymbol(constructorBody); + + NullableWalker.AnalyzeIfNeeded( + compilation, + method, + nodeToAnalyze, + diagnosticsBag, + useConstructorExitWarnings: false, + initialNullableState: null, + getFinalNullableState: true, + baseOrThisInitializer, + out var afterInitializersState); - return afterInitializersState; + if (ownsDiagnostics) + { + diagnosticsBag.Free(); } - return null; + return afterInitializersState; + } + + private static MethodSymbol? GetConstructorThisOrBaseSymbol(BoundNode? constructorBody) + { + return constructorBody is BoundConstructorMethodBody { Initializer: BoundExpressionStatement { Expression: BoundCall { Method: { MethodKind: MethodKind.Constructor } initializerMethod } } } + ? initializerMethod + : null; } /// @@ -1260,7 +1343,7 @@ internal static void AnalyzeWithoutRewrite( DiagnosticBag diagnostics, bool createSnapshots) { - _ = AnalyzeWithSemanticInfo(compilation, symbol, node, binder, initialState: GetAfterInitializersState(compilation, symbol), diagnostics, createSnapshots, requiresAnalysis: false); + _ = AnalyzeWithSemanticInfo(compilation, symbol, node, binder, initialState: GetAfterInitializersState(compilation, symbol, node), diagnostics, createSnapshots, requiresAnalysis: false); } /// @@ -1311,6 +1394,7 @@ private static (SnapshotManager?, ImmutableDictionary.Builder? analyzedNullabilityMapOpt, SnapshotManager.Builder? snapshotBuilderOpt, ArrayBuilder<(BoundReturnStatement, TypeWithAnnotations)>? returnTypesOpt, @@ -1524,6 +1612,7 @@ private static void Analyze( binder, conversions, initialState is null ? null : Variables.Create(initialState.Variables), + baseOrThisInitializer, returnTypesOpt, analyzedNullabilityMapOpt, snapshotBuilderOpt); @@ -4044,6 +4133,7 @@ internal static TypeWithAnnotations BestTypeForLambdaReturns( binder, conversions: conversions, variables: null, + baseOrThisInitializer: null, returnTypesOpt: null, analyzedNullabilityMapOpt: null, snapshotBuilderOpt: null); diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 15ee32e7fe07f..733bc62060cad 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -3562,6 +3562,87 @@ public Derived() : this(true) ); } + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_08() + { + var code = """ + using System.Diagnostics.CodeAnalysis; + #nullable enable + public class Base + { + public required string Prop1 { get; set; } + public string Prop2 { get; set; } = null!; + + [SetsRequiredMembers] + protected Base() {} + } + + public class Derived : Base + { + public required string Prop3 { get; set; } + public string Prop4 { get; set; } + + public Derived() : base() + { + Prop1.ToString(); + Prop2.ToString(); + Prop3.ToString(); // 1 + Prop4.ToString(); // 2 + } + } + """; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (21,9): warning CS8602: Dereference of a possibly null reference. + // Prop3.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop3").WithLocation(21, 9), + // (22,9): warning CS8602: Dereference of a possibly null reference. + // Prop4.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop4").WithLocation(22, 9) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_10() + { + var code = """ + using System.Diagnostics.CodeAnalysis; + #nullable enable + public class Base + { + public required string Prop1 { get; set; } + public string Prop2 { get; set; } = null!; + + protected Base() {} + } + + public class Derived : Base + { + public required string Prop3 { get; set; } + public string Prop4 { get; set; } + + [SetsRequiredMembers] + public Derived(int unused) : base() + { + Prop4 = null!; + } + + public Derived() : this(0) + { + Prop1.ToString(); + Prop2.ToString(); + Prop3.ToString(); + Prop4.ToString(); + } + } + """; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + ); + } + [Fact] public void SetsRequiredMembersAppliedToRecordCopyConstructor_DeclaredInType() { diff --git a/src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs b/src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs index 7fc154a1614f0..dd6f574547594 100644 --- a/src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs +++ b/src/Compilers/Core/Portable/Binding/BindingDiagnosticBag.cs @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Symbols; @@ -26,6 +27,7 @@ protected BindingDiagnosticBag(DiagnosticBag? diagnosticBag) DiagnosticBag = diagnosticBag; } + [MemberNotNullWhen(true, nameof(DiagnosticBag))] internal bool AccumulatesDiagnostics => DiagnosticBag is object; internal void AddRange(ImmutableArray diagnostics) where T : Diagnostic From e69ef155335047eb5bcbcbe7ca5dbb7899395fab Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 5 Apr 2022 17:36:07 -0700 Subject: [PATCH 09/12] Add missing member test for SetsRequiredMembers on records. --- .../Source/SourceMemberContainerSymbol.cs | 5 +++++ .../Symbol/Symbols/RequiredMembersTests.cs | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs index a4f847759b390..75814424f6450 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs @@ -2399,6 +2399,11 @@ private void CheckForRequiredMemberAttribute(BindingDiagnosticBag diagnostics) { // Ensure that an error is reported if the required constructor isn't present. _ = Binder.GetWellKnownTypeMember(DeclaringCompilation, WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor, diagnostics, Locations[0]); + if (this.IsRecord) + { + // Copy constructors need to emit SetsRequiredMembers on the ctor + _ = Binder.GetWellKnownTypeMember(DeclaringCompilation, WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor, diagnostics, Locations[0]); + } } if (BaseTypeNoUseSiteDiagnostics is (not SourceMemberContainerTypeSymbol) and { HasRequiredMembersError: true }) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 733bc62060cad..2eb1bec81256a 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -3674,6 +3674,26 @@ void validate(ModuleSymbol module) } } + [Fact] + public void SetsRequiredMembersAppliedToRecordCopyConstructor_DeclaredInType_SetsRequiredMembersMissing() + { + var code = """ + public record C + { + public required string Prop1 { get; set; } + public required int Field1; + } + """; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.MakeMemberMissing(WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor); + comp.VerifyDiagnostics( + // (1,15): error CS0656: Missing compiler required member 'System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute..ctor' + // public record C + Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "C").WithArguments("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute", ".ctor").WithLocation(1, 15) + ); + } + [Theory] [CombinatorialData] public void SetsRequiredMembersAppliedToRecordCopyConstructor_DeclaredInBase(bool useMetadataReference) From 4cfdf578299b4b9b5b59fa579b32840515d5707f Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 13 Apr 2022 18:14:10 -0700 Subject: [PATCH 10/12] Add restriction that SetsRequiredMembersAttribute must be applied when chaining to a constructor with the attribute. Add more tests. --- .../Portable/Binder/Binder_Expressions.cs | 7 + .../CSharp/Portable/CSharpResources.resx | 3 + .../CSharp/Portable/Errors/ErrorCode.cs | 1 + .../Portable/FlowAnalysis/NullableWalker.cs | 2 - .../Portable/xlf/CSharpResources.cs.xlf | 5 + .../Portable/xlf/CSharpResources.de.xlf | 5 + .../Portable/xlf/CSharpResources.es.xlf | 5 + .../Portable/xlf/CSharpResources.fr.xlf | 5 + .../Portable/xlf/CSharpResources.it.xlf | 5 + .../Portable/xlf/CSharpResources.ja.xlf | 5 + .../Portable/xlf/CSharpResources.ko.xlf | 5 + .../Portable/xlf/CSharpResources.pl.xlf | 5 + .../Portable/xlf/CSharpResources.pt-BR.xlf | 5 + .../Portable/xlf/CSharpResources.ru.xlf | 5 + .../Portable/xlf/CSharpResources.tr.xlf | 5 + .../Portable/xlf/CSharpResources.zh-Hans.xlf | 5 + .../Portable/xlf/CSharpResources.zh-Hant.xlf | 5 + .../Symbol/Symbols/RequiredMembersTests.cs | 341 +++++++++++++++++- 18 files changed, 416 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs index 4bae526c1af20..7c2ee0cfb69a4 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs @@ -4123,6 +4123,13 @@ private BoundExpression BindConstructorInitializerCore( diagnostics); } + if (resultMember.HasSetsRequiredMembers && !constructor.HasSetsRequiredMembers) + { + hasErrors = true; + // This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + diagnostics.Add(ErrorCode.ERR_ChainingToSetsRequiredMembersRequiresSetsRequiredMembers, errorLocation); + } + return new BoundCall( nonNullSyntax, receiver, diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index af0bf6c4debfa..010fe6bec2a74 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -7121,4 +7121,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ An expression tree may not contain UTF8 string conversion or literal. + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index a5a973ee103ec..e65f2f40628e0 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -2085,5 +2085,6 @@ internal enum ErrorCode ERR_RequiredMembersMustBeAssignedValue = 9507, ERR_RequiredMembersInvalid = 9508, ERR_RequiredMembersBaseTypeInvalid = 9509, + ERR_ChainingToSetsRequiredMembersRequiresSetsRequiredMembers = 9510, } } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index da4d8d6e70c8c..44d4583d06a10 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -906,8 +906,6 @@ ImmutableArray getMembersNeedingDefaultInitialState() } } - // We don't use a default initial state for value type instance constructors without `: this()` because - // any usages of uninitialized fields will get definite assignment errors anyway. // Pre-C# 11, we don't use a default initial state for value type instance constructors without `: this()` // because any usages of uninitialized fields will get definite assignment errors anyway. if (!hasThisConstructorInitializer diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf index 4dc2237083b38..9c90d576dc56b 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf @@ -292,6 +292,11 @@ __arglist nemůže mít argument předávaný pomocí in nebo out + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf index 622e763692d46..e6cd08232c967 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf @@ -292,6 +292,11 @@ "__arglist" darf kein über "in" oder "out" übergebenes Argument umfassen. + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf index 8d934caafbd2e..1c17530e8be9e 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf @@ -292,6 +292,11 @@ __arglist no puede tener un argumento que se ha pasado con "in" o "out" + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf index 98b2f6e20fae0..bbaa27f5fcfea 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf @@ -292,6 +292,11 @@ __arglist ne peut pas avoir un argument passé par 'in' ou 'out' + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf index 2c8c2761090ba..4911ca24e3103 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf @@ -292,6 +292,11 @@ __arglist non può contenere un argomento passato da 'in' o 'out' + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf index 32a7a60bd1362..aafa154754957 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf @@ -292,6 +292,11 @@ __arglist では、'in' や 'out' で引数を渡すことができません + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf index 1ed166125436a..976ce92ee668b 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf @@ -292,6 +292,11 @@ __arglist는 'in' 또는 'out'으로 전달되는 인수를 가질 수 없습니다. + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf index 767d1fb51d589..3e4aedaaffc2a 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf @@ -292,6 +292,11 @@ Element __arglist nie może mieć argumentu przekazywanego przez parametr „in” ani „out” + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf index ce7d393dc6431..2fc3abc72bf76 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf @@ -292,6 +292,11 @@ __arglist não pode ter um argumento passado por 'in' ou 'out' + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf index 9f1a531e3298a..52bd688037d98 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf @@ -292,6 +292,11 @@ В __arglist невозможно передать аргумент с помощью in или out + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf index 40d679220061b..9e63442d2deb5 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf @@ -292,6 +292,11 @@ __arglist, 'in' veya 'out' tarafından geçirilen bir bağımsız değişkene sahip olamaz + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf index 51691787bde4f..a46207fe33caf 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf @@ -292,6 +292,11 @@ __arglist 不能有 "in" 或 "out" 传递的参数 + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf index 46ec0a5a34435..e848417ea76b4 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf @@ -292,6 +292,11 @@ __arglist 不得包含 'in' 或 'out' 傳遞的引數 + + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + + The operator '{0}' requires a matching non-checked version of the operator to also be defined The operator '{0}' requires a matching non-checked version of the operator to also be defined diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 0885ba1d8548b..460798e345062 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -3574,6 +3574,9 @@ public Derived() : base() var comp = CreateCompilationWithRequiredMembers(code); comp.VerifyDiagnostics( + // (17,24): error CS9510: This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + // public Derived() : base() + Diagnostic(ErrorCode.ERR_ChainingToSetsRequiredMembersRequiresSetsRequiredMembers, "base").WithLocation(17, 24), // (21,9): warning CS8602: Dereference of a possibly null reference. // Prop3.ToString(); // 1 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop3").WithLocation(21, 9), @@ -3584,7 +3587,7 @@ public Derived() : base() } [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] - public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_10() + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_09() { var code = """ using System.Diagnostics.CodeAnalysis; @@ -3620,6 +3623,313 @@ public Derived() : this(0) var comp = CreateCompilationWithRequiredMembers(code); comp.VerifyDiagnostics( + // (22,24): error CS9510: This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + // public Derived() : this(0) + Diagnostic(ErrorCode.ERR_ChainingToSetsRequiredMembersRequiresSetsRequiredMembers, "this").WithLocation(22, 24) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_10() + { + // This IL is equivalent to: + // #nullable enable + // [constructor: SetsRequiredMembers] + // public record Base(bool unused) { public required string Prop { get; init; } } + var il = """ + .assembly extern attr {} + + .class public auto ansi beforefieldinit Base + extends [mscorlib]System.Object + implements class [mscorlib]System.IEquatable`1 + { + .custom instance void System.Runtime.CompilerServices.NullableContextAttribute::.ctor(uint8) = ( + 01 00 01 00 00 + ) + .custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( + 01 00 00 00 00 + ) + // Fields + .field public string Field1 + .custom instance void [attr]RequiredMemberAttribute::.ctor() = ( + 01 00 00 00 + ) + .field public string Field2 + .custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( + 01 00 02 00 00 + ) + .custom instance void [attr]RequiredMemberAttribute::.ctor() = ( + 01 00 00 00 + ) + + // Methods + .method public hidebysig specialname rtspecialname + instance void .ctor ( + bool 'unused' + ) cil managed + { + .custom instance void [attr]System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::.ctor + + .method family hidebysig specialname newslot virtual + instance class [mscorlib]System.Type get_EqualityContract () cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::get_EqualityContract + + .method public hidebysig specialname + instance bool get_unused () cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::get_unused + + .method public hidebysig specialname + instance void modreq([mscorlib]mscorlib.CompilerServices.IsExternalInit) set_unused ( + bool 'value' + ) cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::set_unused + + .method public hidebysig virtual + instance string ToString () cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::ToString + + .method family hidebysig newslot virtual + instance bool PrintMembers ( + class [mscorlib]System.Text.StringBuilder builder + ) cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::PrintMembers + + .method public hidebysig specialname static + bool op_Inequality ( + class Base left, + class Base right + ) cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::op_Inequality + + .method public hidebysig specialname static + bool op_Equality ( + class Base left, + class Base right + ) cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::op_Equality + + .method public hidebysig virtual + instance int32 GetHashCode () cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::GetHashCode + + .method public hidebysig virtual + instance bool Equals ( + object obj + ) cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::Equals + + .method public hidebysig newslot virtual + instance bool Equals ( + class Base other + ) cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::Equals + + .method public hidebysig newslot virtual + instance class Base '$' () cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::'$' + + .method family hidebysig specialname rtspecialname + instance void .ctor ( + class Base original + ) cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::.ctor + + .method public hidebysig + instance void Deconstruct ( + [out] bool& 'unused' + ) cil managed + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + ldnull + throw + } // end of method Base::Deconstruct + + // Properties + .property instance class [mscorlib]System.Type EqualityContract() + { + .get instance class [mscorlib]System.Type Base::get_EqualityContract() + } + .property instance bool 'unused'() + { + .get instance bool Base::get_unused() + .set instance void modreq([mscorlib]mscorlib.CompilerServices.IsExternalInit) Base::set_unused(bool) + } + } // end of class Base + + .class private auto ansi sealed beforefieldinit System.Runtime.CompilerServices.NullableAttribute + extends [mscorlib]System.Attribute + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + .custom instance void Microsoft.CodeAnalysis.EmbeddedAttribute::.ctor() = ( + 01 00 00 00 + ) + .custom instance void [mscorlib]System.AttributeUsageAttribute::.ctor(valuetype [mscorlib]System.AttributeTargets) = ( + 01 00 84 6b 00 00 02 00 54 02 0d 41 6c 6c 6f 77 + 4d 75 6c 74 69 70 6c 65 00 54 02 09 49 6e 68 65 + 72 69 74 65 64 00 + ) + // Fields + .field public initonly uint8[] NullableFlags + + // Methods + .method public hidebysig specialname rtspecialname + instance void .ctor ( + uint8 '' + ) cil managed + { + ldnull + throw + } // end of method NullableAttribute::.ctor + + .method public hidebysig specialname rtspecialname + instance void .ctor ( + uint8[] '' + ) cil managed + { + ldnull + throw + } // end of method NullableAttribute::.ctor + } // end of class mscorlib.CompilerServices.NullableAttribute + + .class private auto ansi sealed beforefieldinit System.Runtime.CompilerServices.NullableContextAttribute + extends [mscorlib]System.Attribute + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + .custom instance void Microsoft.CodeAnalysis.EmbeddedAttribute::.ctor() = ( + 01 00 00 00 + ) + .custom instance void [mscorlib]System.AttributeUsageAttribute::.ctor(valuetype [mscorlib]System.AttributeTargets) = ( + 01 00 4c 14 00 00 02 00 54 02 0d 41 6c 6c 6f 77 + 4d 75 6c 74 69 70 6c 65 00 54 02 09 49 6e 68 65 + 72 69 74 65 64 00 + ) + // Fields + .field public initonly uint8 Flag + + // Methods + .method public hidebysig specialname rtspecialname + instance void .ctor ( + uint8 '' + ) cil managed + { + ldnull + throw + } // end of method NullableContextAttribute::.ctor + } + .class private auto ansi sealed beforefieldinit Microsoft.CodeAnalysis.EmbeddedAttribute + extends [mscorlib]System.Attribute + { + .custom instance void [mscorlib]mscorlib.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + .custom instance void Microsoft.CodeAnalysis.EmbeddedAttribute::.ctor() = ( + 01 00 00 00 + ) + // Methods + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + ldnull + throw + } // end of method EmbeddedAttribute::.ctor + } // end of class Microsoft.CodeAnalysis.EmbeddedAttribute + """; + + var attrComp = CreateCompilationWithRequiredMembers("", assemblyName: "attr"); + + var code = """ + #nullable enable + public record Derived(bool unused) : Base(unused); + """; + + var comp = CreateCompilationWithIL(code, ilSource: il, references: new[] { attrComp.EmitToImageReference() }); + comp.VerifyDiagnostics( + // (2,42): error CS9510: This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute. + // public record Derived(bool unused) : Base(unused); + Diagnostic(ErrorCode.ERR_ChainingToSetsRequiredMembersRequiresSetsRequiredMembers, "(unused)").WithLocation(2, 42) ); } @@ -3674,6 +3984,35 @@ public record C ); } + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_11() + { + var code = """ + #nullable enable + public class Base + { + public required string Prop1 { get; set; } + public string Prop2 { get; set; } = null!; + } + + public class Derived : Base + { + public required string Prop3 { get; set; } = Prop1.ToString(); + public string Prop4 { get; set; } = Prop2.ToString(); + } + """; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (10,50): error CS0236: A field initializer cannot reference the non-static field, method, or property 'Base.Prop1' + // public required string Prop3 { get; set; } = Prop1.ToString(); + Diagnostic(ErrorCode.ERR_FieldInitRefNonstatic, "Prop1").WithArguments("Base.Prop1").WithLocation(10, 50), + // (11,41): error CS0236: A field initializer cannot reference the non-static field, method, or property 'Base.Prop2' + // public string Prop4 { get; set; } = Prop2.ToString(); + Diagnostic(ErrorCode.ERR_FieldInitRefNonstatic, "Prop2").WithArguments("Base.Prop2").WithLocation(11, 41) + ); + } + [Theory] [CombinatorialData] public void SetsRequiredMembersAppliedToRecordCopyConstructor_DeclaredInBase(bool useMetadataReference) From e7e4947cc382bbc607ebb9b6de6fc6c238edac9b Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 14 Apr 2022 15:53:40 -0700 Subject: [PATCH 11/12] Remove erroneous assert. --- src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index e8e363ec19884..6a5e71600bcd5 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -1815,7 +1815,6 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && case BoundKind.ConstructorMethodBody: var constructor = (BoundConstructorMethodBody)methodBody; body = constructor.BlockBody ?? constructor.ExpressionBody!; - Debug.Assert(body != null); if (constructor.Initializer is BoundExpressionStatement expressionStatement) { From 106378e780addab1e27bfe176c7f8e7fa925d9eb Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 14 Apr 2022 18:14:40 -0700 Subject: [PATCH 12/12] PR Feedback. --- .../Portable/FlowAnalysis/NullableWalker.cs | 35 +++++++++++-------- .../Symbol/Symbols/RequiredMembersTests.cs | 6 ---- .../Test/Utilities/CSharp/CSharpTestBase.cs | 4 +-- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 44d4583d06a10..b0c58cadf4247 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -886,23 +886,28 @@ ImmutableArray getMembersNeedingDefaultInitialState() return ImmutableArray.Empty; } - bool includeRequiredMembers = true; + bool includeCurrentTypeRequiredMembers = true; bool includeBaseRequiredMembers = true; bool hasThisConstructorInitializer = false; if (method is SourceMemberMethodSymbol { SyntaxNode: ConstructorDeclarationSyntax { Initializer: { RawKind: var initializerKind } } }) { + // We have multiple ways of entering the nullable walker: we could be just analyzing the initializers, with a BoundStatementList body and _baseOrThisInitializer + // having been provided, or we could be analyzing the body of a constructor, with a BoundConstructorBody body and _baseOrThisInitializer being null. var baseOrThisInitializer = (_baseOrThisInitializer ?? GetConstructorThisOrBaseSymbol(this.methodMainNode)); // If there's an error in the base or this initializer, presume that we should set all required members to default. includeBaseRequiredMembers = baseOrThisInitializer?.ShouldCheckRequiredMembers() ?? true; if (initializerKind == (int)SyntaxKind.ThisConstructorInitializer) { hasThisConstructorInitializer = true; - includeRequiredMembers = includeBaseRequiredMembers; + // If we chained to a `this` constructor, a SetsRequiredMembers attribute applies to both the current type's required members and the base type's required members. + includeCurrentTypeRequiredMembers = includeBaseRequiredMembers; } else if (initializerKind == (int)SyntaxKind.BaseConstructorInitializer) { - includeRequiredMembers = true; + // If we chained to a `base` constructor, a SetsRequiredMembers attribute applies to the base type's required members only, and the current type's required members + // are not assumed to be initialized. + includeCurrentTypeRequiredMembers = true; } } @@ -913,37 +918,37 @@ ImmutableArray getMembersNeedingDefaultInitialState() || method.IsStatic || compilation.IsFeatureEnabled(MessageID.IDS_FeatureAutoDefaultStructs))) { - return membersToBeInitialized(method.ContainingType, includeAllMembers: true, includeRequiredMembers, includeBaseRequiredMembers); + return membersToBeInitialized(method.ContainingType, includeAllMembers: true, includeCurrentTypeRequiredMembers, includeBaseRequiredMembers); } // We want to presume all required members of the type are uninitialized, and in addition we want to set all fields to // default if we can get to this constructor by doing so (ie, : this() in a value type). - return membersToBeInitialized(method.ContainingType, includeAllMembers: method.IncludeFieldInitializersInBody(), includeRequiredMembers, includeBaseRequiredMembers); + return membersToBeInitialized(method.ContainingType, includeAllMembers: method.IncludeFieldInitializersInBody(), includeCurrentTypeRequiredMembers, includeBaseRequiredMembers); - static ImmutableArray membersToBeInitialized(NamedTypeSymbol containingType, bool includeAllMembers, bool includeRequiredMembers, bool includeBaseRequiredMembers) + static ImmutableArray membersToBeInitialized(NamedTypeSymbol containingType, bool includeAllMembers, bool includeCurrentTypeRequiredMembers, bool includeBaseRequiredMembers) { - return (includeAllMembers, includeRequiredMembers, includeBaseRequiredMembers) switch + return (includeAllMembers, includeCurrentTypeRequiredMembers, includeBaseRequiredMembers) switch { - (includeAllMembers: false, includeRequiredMembers: false, includeBaseRequiredMembers: false) + (includeAllMembers: false, includeCurrentTypeRequiredMembers: false, includeBaseRequiredMembers: false) => ImmutableArray.Empty, - (includeAllMembers: false, includeRequiredMembers: true, includeBaseRequiredMembers: false) + (includeAllMembers: false, includeCurrentTypeRequiredMembers: true, includeBaseRequiredMembers: false) => containingType.GetMembersUnordered().SelectAsArray(predicate: SymbolExtensions.IsRequired, selector: getFieldSymbolToBeInitialized), - (includeAllMembers: false, includeRequiredMembers: true, includeBaseRequiredMembers: true) + (includeAllMembers: false, includeCurrentTypeRequiredMembers: true, includeBaseRequiredMembers: true) => containingType.AllRequiredMembers.SelectAsArray(static kvp => getFieldSymbolToBeInitialized(kvp.Value)), - (includeAllMembers: true, includeRequiredMembers: _, includeBaseRequiredMembers: false) + (includeAllMembers: true, includeCurrentTypeRequiredMembers: _, includeBaseRequiredMembers: false) => containingType.GetMembersUnordered().SelectAsArray(getFieldSymbolToBeInitialized), - (includeAllMembers: true, includeRequiredMembers: true, includeBaseRequiredMembers: true) - => getAllTypeAndRequiredMembers(), + (includeAllMembers: true, includeCurrentTypeRequiredMembers: true, includeBaseRequiredMembers: true) + => getAllTypeAndRequiredMembers(containingType), - (includeAllMembers: _, includeRequiredMembers: false, includeBaseRequiredMembers: true) + (includeAllMembers: _, includeCurrentTypeRequiredMembers: false, includeBaseRequiredMembers: true) => throw ExceptionUtilities.Unreachable, }; - ImmutableArray getAllTypeAndRequiredMembers() + static ImmutableArray getAllTypeAndRequiredMembers(TypeSymbol containingType) { var members = containingType.GetMembersUnordered(); var requiredMembers = containingType.BaseTypeNoUseSiteDiagnostics?.AllRequiredMembers ?? ImmutableSegmentedDictionary.Empty; diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 460798e345062..045d657d9565b 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -1277,7 +1277,6 @@ public class C [CombinatorialData] public void EnforcedRequiredMembers_NoInheritance_NoneSet_HasSetsRequiredMembers(bool useMetadataReference, [CombinatorialValues("", " C")] string constructor, [CombinatorialValues("", "method: ")] string target) { -#pragma warning disable IDE0055 // Fix formatting var c = $$""" using System.Diagnostics.CodeAnalysis; public class C @@ -1289,7 +1288,6 @@ public class C public C() {} } """; -#pragma warning restore IDE0055 // Fix formatting var creation = $@"C c = new{constructor}();"; var comp = CreateCompilationWithRequiredMembers(new[] { c, creation }); @@ -2309,7 +2307,6 @@ private static string GetShadowingRecordIl(bool propertyIsRequired) 01 00 00 00 ) """ : ""; -#pragma warning disable IDE0055 // Fix formatting return $$""" .assembly extern original {} @@ -2473,7 +2470,6 @@ .set instance void modreq([mscorlib]System.Runtime.CompilerServices.IsExternalIn } // end of class Derived """; -#pragma warning restore IDE0055 // Fix formatting } [Fact] @@ -3373,7 +3369,6 @@ public C(bool unused) : this() [InlineData("")] public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_04(string baseSyntax) { -#pragma warning disable IDE0055 // Fix formatting var code = $$""" #nullable enable public class Base @@ -3395,7 +3390,6 @@ public Derived() {{baseSyntax}} } } """; -#pragma warning restore IDE0055 // Fix formatting var comp = CreateCompilationWithRequiredMembers(code); comp.VerifyDiagnostics( diff --git a/src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs b/src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs index 98118c5eef03a..9b9c1c9200af6 100644 --- a/src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs +++ b/src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs @@ -629,7 +629,7 @@ public UnmanagedCallersOnlyAttribute() { } namespace System.Runtime.CompilerServices { [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Field | AttributeTargets.Property, Inherited = false, AllowMultiple = false)] - public class RequiredMemberAttribute : Attribute + public sealed class RequiredMemberAttribute : Attribute { public RequiredMemberAttribute() { @@ -642,7 +642,7 @@ public RequiredMemberAttribute() namespace System.Diagnostics.CodeAnalysis { [AttributeUsage(AttributeTargets.Constructor, Inherited = false, AllowMultiple = false)] - public class SetsRequiredMembersAttribute : Attribute + public sealed class SetsRequiredMembersAttribute : Attribute { public SetsRequiredMembersAttribute() {