From 7d3fe09012e2126df5d7ac7fd1a290bad4b0ad25 Mon Sep 17 00:00:00 2001 From: AlekseyTs Date: Fri, 13 Nov 2020 10:28:50 -0800 Subject: [PATCH 1/7] Simplify and reduce amaunt of work performed by SourcePropertySymbolBase constructor. Closes #45489. --- .../Portable/Binder/Binder_Statements.cs | 2 +- .../Portable/Compiler/MethodCompiler.cs | 2 +- .../LocalRewriter_AssignmentOperator.cs | 2 +- .../CSharp/Portable/Symbols/CompletionPart.cs | 15 +- .../Source/SourceMemberContainerSymbol.cs | 6 +- .../Symbols/Source/SourcePropertySymbol.cs | 202 +++-- .../Source/SourcePropertySymbolBase.cs | 691 ++++++++++-------- ...nthesizedRecordEqualityContractProperty.cs | 68 +- .../SynthesizedRecordPropertySymbol.cs | 69 +- .../SynthesizedBackingFieldSymbol.cs | 31 + .../Test/Emit/Emit/EmitMetadataTests.cs | 2 +- 11 files changed, 614 insertions(+), 476 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 70e56c5a322d8..279641294175e 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1628,7 +1628,7 @@ private static bool AccessingAutoPropertyFromConstructor(BoundExpression receive var propertyIsStatic = propertySymbol.IsStatic; return (object)sourceProperty != null && - sourceProperty.IsAutoProperty && + sourceProperty.IsAutoPropertyWithGetAccessor && TypeSymbol.Equals(sourceProperty.ContainingType, fromMember.ContainingType, TypeCompareKind.ConsiderEverything2) && IsConstructorOrField(fromMember, isStatic: propertyIsStatic) && (propertyIsStatic || receiver.Kind == BoundKind.ThisReference); diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index d1566cf8956af..888935f7b03e2 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -1754,7 +1754,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax && else { var property = sourceMethod.AssociatedSymbol as SourcePropertySymbolBase; - if ((object)property != null && property.IsAutoProperty) + if ((object)property != null && property.IsAutoPropertyWithGetAccessor) { return MethodBodySynthesizer.ConstructAutoPropertyAccessorBody(sourceMethod); } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_AssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_AssignmentOperator.cs index 90ea473a0085f..2ffd6adf61f3f 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_AssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_AssignmentOperator.cs @@ -299,7 +299,7 @@ private BoundExpression MakePropertyAssignment( if (setMethod is null) { var autoProp = (SourcePropertySymbolBase)property; - Debug.Assert(autoProp.IsAutoProperty, + Debug.Assert(autoProp.IsAutoPropertyWithGetAccessor, "only autoproperties can be assignable without having setters"); var backingField = autoProp.BackingField; diff --git a/src/Compilers/CSharp/Portable/Symbols/CompletionPart.cs b/src/Compilers/CSharp/Portable/Symbols/CompletionPart.cs index fd0189840d81c..7ba170de6fe01 100644 --- a/src/Compilers/CSharp/Portable/Symbols/CompletionPart.cs +++ b/src/Compilers/CSharp/Portable/Symbols/CompletionPart.cs @@ -85,11 +85,16 @@ internal enum CompletionPart TypeParameterSymbolAll = Attributes | TypeParameterConstraints, // For property symbols - StartPropertyParameters = 1 << 4, - FinishPropertyParameters = 1 << 5, - StartPropertyType = 1 << 6, - FinishPropertyType = 1 << 7, - PropertySymbolAll = Attributes | StartPropertyParameters | FinishPropertyParameters | StartPropertyType | FinishPropertyType, + StartPropertyEnsureSignature = 1 << 4, + FinishPropertyEnsureSignature = 1 << 5, + StartPropertyParameters = 1 << 6, + FinishPropertyParameters = 1 << 7, + StartPropertyType = 1 << 8, + FinishPropertyType = 1 << 9, + StartPropertyFinalCompletion = 1 << 10, + FinishPropertyFinalCompletion = 1 << 11, + PropertySymbolAll = Attributes | StartPropertyEnsureSignature | FinishPropertyEnsureSignature | StartPropertyParameters | FinishPropertyParameters | + StartPropertyType | FinishPropertyType | StartPropertyFinalCompletion | FinishPropertyFinalCompletion, // For alias symbols AliasTarget = 1 << 4, diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs index ef90e51f51770..44dfc15e0e7a6 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs @@ -3291,7 +3291,7 @@ ImmutableArray addProperties(ImmutableArray rec } if (existingMember is null) { - addProperty(new SynthesizedRecordPropertySymbol(this, syntax, param, isOverride: false, diagnostics)); + addProperty(new SynthesizedRecordPropertySymbol(this, syntax, param, isOverride: false)); } else if (existingMember is PropertySymbol { IsStatic: false, GetMethod: { } } prop && prop.TypeWithAnnotations.Equals(param.TypeWithAnnotations, TypeCompareKind.AllIgnoreOptions)) @@ -3299,7 +3299,7 @@ ImmutableArray addProperties(ImmutableArray rec // There already exists a member corresponding to the candidate synthesized property. if (isInherited && prop.IsAbstract) { - addProperty(new SynthesizedRecordPropertySymbol(this, syntax, param, isOverride: true, diagnostics)); + addProperty(new SynthesizedRecordPropertySymbol(this, syntax, param, isOverride: true)); } else { @@ -3393,7 +3393,7 @@ PropertySymbol addEqualityContract() if (!memberSignatures.TryGetValue(targetProperty, out Symbol? existingEqualityContractProperty)) { - equalityContract = new SynthesizedRecordEqualityContractProperty(this, diagnostics); + equalityContract = new SynthesizedRecordEqualityContractProperty(this); members.Add(equalityContract); members.Add(equalityContract.GetMethod); } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index 695b7c6b73247..4ca8ef4726c75 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -30,7 +30,7 @@ internal static SourcePropertySymbol Create(SourceMemberContainerTypeSymbol cont private static SourcePropertySymbol Create( SourceMemberContainerTypeSymbol containingType, - Binder bodyBinder, + Binder binder, BasePropertyDeclarationSyntax syntax, string name, Location location, @@ -45,71 +45,91 @@ private static SourcePropertySymbol Create( out bool isInitOnly, out var getSyntax, out var setSyntax); - // This has the value that IsIndexer will ultimately have, once we've populated the fields of this object. - bool isIndexer = syntax.Kind() == SyntaxKind.IndexerDeclaration; + var explicitInterfaceSpecifier = GetExplicitInterfaceSpecifier(syntax); + SyntaxTokenList modifiersTokenList = GetModifierTokensSyntax(syntax); + bool isExplicitInterfaceImplementation = explicitInterfaceSpecifier is object; var modifiers = MakeModifiers( containingType, - GetModifierTokensSyntax(syntax), - isExplicitInterfaceImplementation: explicitInterfaceSpecifier is object, - isIndexer: isIndexer, + modifiersTokenList, + isExplicitInterfaceImplementation, + isIndexer: syntax.Kind() == SyntaxKind.IndexerDeclaration, accessorsHaveImplementation: accessorsHaveImplementation, location, diagnostics, out _); + + bool isExpressionBodied = !hasAccessorList && GetArrowExpression(syntax) != null; + + binder = binder.WithUnsafeRegionIfNecessary(modifiersTokenList); + TypeSymbol? explicitInterfaceType; + string? aliasQualifierOpt; + string memberName = ExplicitInterfaceHelpers.GetMemberNameAndInterfaceSymbol(binder, explicitInterfaceSpecifier, name, diagnostics, out explicitInterfaceType, out aliasQualifierOpt); + return new SourcePropertySymbol( containingType, - bodyBinder, syntax, - getSyntax: getSyntax, - setSyntax: setSyntax, - explicitInterfaceSpecifier, + hasGetAccessor: getSyntax != null || isExpressionBodied, + hasSetAccessor: setSyntax != null, + isExplicitInterfaceImplementation, + explicitInterfaceType, + aliasQualifierOpt, modifiers, - isIndexer: isIndexer, isAutoProperty: isAutoProperty, - hasAccessorList: hasAccessorList, + isExpressionBodied: isExpressionBodied, isInitOnly: isInitOnly, - name, + memberName, location, diagnostics); } private SourcePropertySymbol( - SourceMemberContainerTypeSymbol containingType, - Binder bodyBinder, - BasePropertyDeclarationSyntax syntax, - CSharpSyntaxNode? getSyntax, - CSharpSyntaxNode? setSyntax, - ExplicitInterfaceSpecifierSyntax? explicitInterfaceSpecifier, + SourceMemberContainerTypeSymbol containingType, + BasePropertyDeclarationSyntax syntax, + bool hasGetAccessor, + bool hasSetAccessor, + bool isExplicitInterfaceImplementation, + TypeSymbol? explicitInterfaceType, + string? aliasQualifierOpt, DeclarationModifiers modifiers, - bool isIndexer, bool isAutoProperty, - bool hasAccessorList, + bool isExpressionBodied, bool isInitOnly, - string name, - Location location, - DiagnosticBag diagnostics) - : base( + string memberName, + Location location, + DiagnosticBag diagnostics) + : base( containingType, - bodyBinder, syntax, - getSyntax: getSyntax, - setSyntax: setSyntax, - arrowExpression: GetArrowExpression(syntax), - explicitInterfaceSpecifier, + hasGetAccessor, + hasSetAccessor, + isExplicitInterfaceImplementation, + explicitInterfaceType, + aliasQualifierOpt, modifiers, - isIndexer: isIndexer, hasInitializer: HasInitializer(syntax), isAutoProperty: isAutoProperty, - hasAccessorList: hasAccessorList, + isExpressionBodied: isExpressionBodied, isInitOnly: isInitOnly, syntax.Type.GetRefKind(), - name, - location, - typeOpt: default, - hasParameters: GetParameterListSyntax(syntax) is object, - diagnostics) + memberName, + syntax.AttributeLists, + location) { + if (IsAutoProperty) + { + Binder.CheckFeatureAvailability( + syntax, + (hasGetAccessor && !hasSetAccessor) ? MessageID.IDS_FeatureReadonlyAutoImplementedProperties : MessageID.IDS_FeatureAutoImplementedProperties, + diagnostics, + location); + } + + CheckForBlockAndExpressionBody( + syntax.AccessorList, + syntax.GetExpressionBodySyntax(), + syntax, + diagnostics); } private TypeSyntax GetTypeSyntax(SyntaxNode syntax) => ((BasePropertyDeclarationSyntax)syntax).Type; @@ -117,9 +137,6 @@ private SourcePropertySymbol( protected override Location TypeLocation => GetTypeSyntax(CSharpSyntaxNode).Location; - protected override SyntaxTokenList GetModifierTokens(SyntaxNode syntax) - => GetModifierTokensSyntax(syntax); - private static SyntaxTokenList GetModifierTokensSyntax(SyntaxNode syntax) => ((BasePropertyDeclarationSyntax)syntax).Modifiers; @@ -214,14 +231,33 @@ private static void GetAccessorDeclarations( } } - protected override void CheckForBlockAndExpressionBody(CSharpSyntaxNode syntax, DiagnosticBag diagnostics) + private static AccessorDeclarationSyntax GetGetAccessorDeclaration(BasePropertyDeclarationSyntax syntax) { - var prop = (BasePropertyDeclarationSyntax)syntax; - CheckForBlockAndExpressionBody( - prop.AccessorList, - prop.GetExpressionBodySyntax(), - prop, - diagnostics); + foreach (var accessor in syntax.AccessorList!.Accessors) + { + switch (accessor.Kind()) + { + case SyntaxKind.GetAccessorDeclaration: + return accessor; + } + } + + throw ExceptionUtilities.Unreachable; + } + + private static AccessorDeclarationSyntax GetSetAccessorDeclaration(BasePropertyDeclarationSyntax syntax) + { + foreach (var accessor in syntax.AccessorList!.Accessors) + { + switch (accessor.Kind()) + { + case SyntaxKind.SetAccessorDeclaration: + case SyntaxKind.InitAccessorDeclaration: + return accessor; + } + } + + throw ExceptionUtilities.Unreachable; } private static DeclarationModifiers MakeModifiers( @@ -308,25 +344,51 @@ private static DeclarationModifiers MakeModifiers( return mods; } - protected override SourcePropertyAccessorSymbol? CreateAccessorSymbol( - bool isGet, - CSharpSyntaxNode? syntaxOpt, + protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, bool isExplicitInterfaceImplementation, PropertySymbol? explicitlyImplementedPropertyOpt, DiagnosticBag diagnostics) + { + var syntax = (BasePropertyDeclarationSyntax)CSharpSyntaxNode; + ArrowExpressionClauseSyntax? arrowExpression = GetArrowExpression(syntax); + string? aliasQualifierOpt = GetExplicitInterfaceSpecifier(syntax)?.Name.GetAliasQualifierOpt(); + + if (syntax.AccessorList is null && arrowExpression != null) + { + return CreateExpressionBodiedAccessor( + arrowExpression, + explicitlyImplementedPropertyOpt, + aliasQualifierOpt, + isExplicitInterfaceImplementation, + diagnostics); + } + else + { + return CreateAccessorSymbol(GetGetAccessorDeclaration(syntax), explicitlyImplementedPropertyOpt, aliasQualifierOpt, isAutoPropertyAccessor, isExplicitInterfaceImplementation, diagnostics); + } + } + + protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(bool isAutoPropertyAccessor, bool isExplicitInterfaceImplementation, PropertySymbol? explicitlyImplementedPropertyOpt, DiagnosticBag diagnostics) + { + var syntax = (BasePropertyDeclarationSyntax)CSharpSyntaxNode; + string? aliasQualifierOpt = GetExplicitInterfaceSpecifier(syntax)?.Name.GetAliasQualifierOpt(); + + Debug.Assert(!(syntax.AccessorList is null && GetArrowExpression(syntax) != null)); + + return CreateAccessorSymbol(GetSetAccessorDeclaration(syntax), explicitlyImplementedPropertyOpt, aliasQualifierOpt, isAutoPropertyAccessor, isExplicitInterfaceImplementation, diagnostics); + } + + private SourcePropertyAccessorSymbol CreateAccessorSymbol( + AccessorDeclarationSyntax syntax, PropertySymbol? explicitlyImplementedPropertyOpt, string? aliasQualifierOpt, bool isAutoPropertyAccessor, bool isExplicitInterfaceImplementation, DiagnosticBag diagnostics) { - if (syntaxOpt is null) - { - return null; - } return SourcePropertyAccessorSymbol.CreateAccessorSymbol( ContainingType, this, _modifiers, _sourceName, - (AccessorDeclarationSyntax)syntaxOpt, + syntax, explicitlyImplementedPropertyOpt, aliasQualifierOpt, isAutoPropertyAccessor, @@ -334,7 +396,7 @@ private static DeclarationModifiers MakeModifiers( diagnostics); } - protected override SourcePropertyAccessorSymbol CreateExpressionBodiedAccessor( + private SourcePropertyAccessorSymbol CreateExpressionBodiedAccessor( ArrowExpressionClauseSyntax syntax, PropertySymbol? explicitlyImplementedPropertyOpt, string? aliasQualifierOpt, @@ -360,15 +422,21 @@ private Binder CreateBinderForTypeAndParameters() var syntax = CSharpSyntaxNode; var binderFactory = compilation.GetBinderFactory(syntaxTree); var binder = binderFactory.GetBinder(syntax, syntax, this); - SyntaxTokenList modifiers = GetModifierTokens(syntax); + SyntaxTokenList modifiers = GetModifierTokensSyntax(syntax); binder = binder.WithUnsafeRegionIfNecessary(modifiers); return binder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks, this); } - protected override TypeWithAnnotations ComputeType(Binder? binder, SyntaxNode syntax, DiagnosticBag diagnostics) + protected override (TypeWithAnnotations Type, ImmutableArray Parameters) MakeParametersAndBindType(DiagnosticBag diagnostics) { - binder ??= CreateBinderForTypeAndParameters(); + Binder binder = CreateBinderForTypeAndParameters(); + var syntax = CSharpSyntaxNode; + + return (ComputeType(binder, syntax, diagnostics), ComputeParameters(binder, syntax, diagnostics)); + } + private TypeWithAnnotations ComputeType(Binder binder, SyntaxNode syntax, DiagnosticBag diagnostics) + { RefKind refKind; var typeSyntax = GetTypeSyntax(syntax).SkipRef(out refKind); var type = binder.BindType(typeSyntax, diagnostics); @@ -433,17 +501,22 @@ private static ImmutableArray MakeParameters( return parameters; } - protected override ImmutableArray ComputeParameters(Binder? binder, CSharpSyntaxNode syntax, DiagnosticBag diagnostics) + private ImmutableArray ComputeParameters(Binder binder, CSharpSyntaxNode syntax, DiagnosticBag diagnostics) { - binder ??= CreateBinderForTypeAndParameters(); - var parameterSyntaxOpt = GetParameterListSyntax(syntax); var parameters = MakeParameters(binder, this, parameterSyntaxOpt, diagnostics, addRefReadOnlyModifier: IsVirtual || IsAbstract); + return parameters; + } + + internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, DiagnosticBag diagnostics) + { + base.AfterAddingTypeMembersChecks(conversions, diagnostics); + HashSet? useSiteDiagnostics = null; - foreach (ParameterSymbol param in parameters) + foreach (ParameterSymbol param in Parameters) { - if (GetExplicitInterfaceSpecifier(syntax) == null && !this.IsNoMoreVisibleThan(param.Type, ref useSiteDiagnostics)) + if (!IsExplicitInterfaceImplementation && !this.IsNoMoreVisibleThan(param.Type, ref useSiteDiagnostics)) { diagnostics.Add(ErrorCode.ERR_BadVisIndexerParam, Location, this, param.Type); } @@ -454,7 +527,6 @@ protected override ImmutableArray ComputeParameters(Binder? bin } diagnostics.Add(Location, useSiteDiagnostics); - return parameters; } protected override bool HasPointerTypeSyntactically diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs index 97a6e528bb402..c377a44813ff7 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs @@ -30,6 +30,10 @@ private enum Flags : byte IsExpressionBodied = 1 << 0, IsAutoProperty = 1 << 1, IsExplicitInterfaceImplementation = 1 << 2, + HasGetAccessor = 1 << 3, + HasSetAccessor = 1 << 4, + HasInitializer = 1 << 5, + IsAutoPropertyWithGetAccessor = IsAutoProperty | HasGetAccessor, } // TODO (tomat): consider splitting into multiple subclasses/rare data. @@ -38,11 +42,14 @@ private enum Flags : byte private readonly string _name; private readonly SyntaxReference _syntaxRef; protected readonly DeclarationModifiers _modifiers; - private readonly ImmutableArray _refCustomModifiers; - private readonly SourcePropertyAccessorSymbol _getMethod; - private readonly SourcePropertyAccessorSymbol _setMethod; + private ImmutableArray _lazyRefCustomModifiers; +#nullable enable + private SourcePropertyAccessorSymbol? _lazyGetMethod; + private SourcePropertyAccessorSymbol? _lazySetMethod; + private DiagnosticBag? _lazyDiagnosticBag; +#nullable disable private readonly TypeSymbol _explicitInterfaceType; - private readonly ImmutableArray _explicitInterfaceImplementations; + private ImmutableArray _lazyExplicitInterfaceImplementations; private readonly Flags _propertyFlags; private readonly RefKind _refKind; @@ -66,58 +73,72 @@ private enum Flags : byte public Location Location { get; } #nullable enable - /// - /// The is passed for explicit interface implementations or - /// overrides to . If a binder is not required in this situation - /// the parameter can be null. - /// protected SourcePropertySymbolBase( SourceMemberContainerTypeSymbol containingType, - Binder? binder, CSharpSyntaxNode syntax, - CSharpSyntaxNode? getSyntax, - CSharpSyntaxNode? setSyntax, - ArrowExpressionClauseSyntax? arrowExpression, - ExplicitInterfaceSpecifierSyntax? interfaceSpecifier, + bool hasGetAccessor, + bool hasSetAccessor, + bool isExplicitInterfaceImplementation, + TypeSymbol? explicitInterfaceType, + string? aliasQualifierOpt, DeclarationModifiers modifiers, - bool isIndexer, bool hasInitializer, bool isAutoProperty, - bool hasAccessorList, + bool isExpressionBodied, bool isInitOnly, RefKind refKind, - string name, - Location location, - TypeWithAnnotations typeOpt, - bool hasParameters, - DiagnosticBag diagnostics) + string memberName, + SyntaxList indexerNameAttributeLists, + Location location) { + Debug.Assert(!isExpressionBodied || !isAutoProperty); + Debug.Assert(!isExpressionBodied || !hasInitializer); + _syntaxRef = syntax.GetReference(); Location = location; + _containingType = containingType; + _refKind = refKind; + _modifiers = modifiers; + _explicitInterfaceType = explicitInterfaceType; - bool isExplicitInterfaceImplementation = interfaceSpecifier != null; if (isExplicitInterfaceImplementation) { _propertyFlags |= Flags.IsExplicitInterfaceImplementation; } + else + { + _lazyExplicitInterfaceImplementations = ImmutableArray.Empty; + } - _containingType = containingType; - _refKind = refKind; + bool isIndexer = IsIndexer; + isAutoProperty = isAutoProperty && !(containingType.IsInterface && !IsStatic) && !IsAbstract && !IsExtern && !isIndexer; - if (binder is object) + if (isAutoProperty) { - binder = binder.WithUnsafeRegionIfNecessary(GetModifierTokens(syntax)); - binder = binder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks, this); + _propertyFlags |= Flags.IsAutoProperty; } - _modifiers = modifiers; - this.CheckAccessibility(location, diagnostics, isExplicitInterfaceImplementation); + if (hasGetAccessor) + { + _propertyFlags |= Flags.HasGetAccessor; + } - this.CheckModifiers(isExplicitInterfaceImplementation, location, isIndexer, diagnostics); + if (hasSetAccessor) + { + _propertyFlags |= Flags.HasSetAccessor; + } - isAutoProperty = isAutoProperty && !(containingType.IsInterface && !IsStatic) && !IsAbstract && !IsExtern && !isIndexer; + if (hasInitializer) + { + _propertyFlags |= Flags.HasInitializer; + } - if (isIndexer && !isExplicitInterfaceImplementation) + if (isExpressionBodied) + { + _propertyFlags |= Flags.IsExpressionBodied; + } + + if (isIndexer && indexerNameAttributeLists.Count != 0 && !isExplicitInterfaceImplementation) { // Evaluate the attributes immediately in case the IndexerNameAttribute has been applied. // NOTE: we want IsExplicitInterfaceImplementation, IsOverride, Locations, and the syntax reference @@ -128,7 +149,7 @@ protected SourcePropertySymbolBase( // always use the real attribute bag of this symbol and modify LoadAndValidateAttributes to // handle partially filled bags. CustomAttributesBag? temp = null; - LoadAndValidateAttributes(OneOrMany.Create(AttributeDeclarationSyntaxList), ref temp, earlyDecodingOnly: true); + LoadAndValidateAttributes(OneOrMany.Create(indexerNameAttributeLists), ref temp, earlyDecodingOnly: true); if (temp != null) { Debug.Assert(temp.IsEarlyDecodedWellKnownAttributeDataComputed); @@ -140,75 +161,28 @@ protected SourcePropertySymbolBase( } } - string aliasQualifierOpt; - string memberName = ExplicitInterfaceHelpers.GetMemberNameAndInterfaceSymbol(binder, interfaceSpecifier, name, diagnostics, out _explicitInterfaceType, out aliasQualifierOpt); _sourceName = _sourceName ?? memberName; //sourceName may have been set while loading attributes _name = isIndexer ? ExplicitInterfaceHelpers.GetMemberName(WellKnownMemberNames.Indexer, _explicitInterfaceType, aliasQualifierOpt) : _sourceName; - if (hasInitializer) + if ((isAutoProperty && hasGetAccessor) || hasInitializer) { - CheckInitializer(isAutoProperty, containingType.IsInterface, IsStatic, location, diagnostics); - } - - var hasGetAccessor = getSyntax is object; - var hasSetAccessor = setSyntax is object; - - if (isAutoProperty || hasInitializer) - { - var isAutoPropertyWithGetSyntax = isAutoProperty && hasGetAccessor; - if (isAutoPropertyWithGetSyntax) - { - _propertyFlags |= Flags.IsAutoProperty; - } - - bool isGetterOnly = hasGetAccessor && !hasSetAccessor; - - if (isAutoPropertyWithGetSyntax && !IsStatic && !isGetterOnly && !isInitOnly) - { - if (ContainingType.IsReadOnly) - { - diagnostics.Add(ErrorCode.ERR_AutoPropsInRoStruct, location); - } - else if (HasReadOnlyModifier) - { - diagnostics.Add(ErrorCode.ERR_AutoPropertyWithSetterCantBeReadOnly, location, this); - } - } - - if (isAutoPropertyWithGetSyntax || hasInitializer) - { - if (isAutoPropertyWithGetSyntax) - { - //issue a diagnostic if the compiler generated attribute ctor is not found. - Binder.ReportUseSiteDiagnosticForSynthesizedAttribute(DeclaringCompilation, - WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor, diagnostics, syntax: syntax); - - if (this._refKind != RefKind.None && !_containingType.IsInterface) - { - diagnostics.Add(ErrorCode.ERR_AutoPropertyCannotBeRefReturning, location, this); - } - } - - string fieldName = GeneratedNames.MakeBackingFieldName(_sourceName); - BackingField = new SynthesizedBackingFieldSymbol(this, - fieldName, - isReadOnly: isGetterOnly || isInitOnly, - this.IsStatic, - hasInitializer); - } - - if (isAutoProperty) - { - Binder.CheckFeatureAvailability( - syntax, - isGetterOnly ? MessageID.IDS_FeatureReadonlyAutoImplementedProperties : MessageID.IDS_FeatureAutoImplementedProperties, - diagnostics, - location); - } + string fieldName = GeneratedNames.MakeBackingFieldName(_sourceName); + BackingField = new SynthesizedBackingFieldSymbol(this, + fieldName, + isReadOnly: (hasGetAccessor && !hasSetAccessor) || isInitOnly, + this.IsStatic, + hasInitializer); } + } + private void EnsureSignature(DiagnosticBag diagnostics) + { PropertySymbol? explicitlyImplementedProperty = null; - _refCustomModifiers = ImmutableArray.Empty; + _lazyRefCustomModifiers = ImmutableArray.Empty; + + TypeWithAnnotations type; + (type, _lazyParameters) = MakeParametersAndBindType(diagnostics); + _lazyType = new TypeWithAnnotations.Boxed(type); // The runtime will not treat the accessors of this property as overrides or implementations // of those of another property unless both the signatures and the custom modifiers match. @@ -223,16 +197,9 @@ protected SourcePropertySymbolBase( // Note: we're checking if the syntax indicates explicit implementation rather, // than if explicitInterfaceType is null because we don't want to look for an // overridden property if this is supposed to be an explicit implementation. + bool isExplicitInterfaceImplementation = IsExplicitInterfaceImplementation; if (isExplicitInterfaceImplementation || this.IsOverride) { - // Type and parameters for overrides and explicit implementations cannot be bound - // lazily since the property name depends on the metadata name of the base property, - // and the property name is required to add the property to the containing type, and - // the type and parameters are required to determine the override or implementation. - var type = typeOpt.HasType ? typeOpt : this.ComputeType(binder, syntax, diagnostics); - _lazyType = new TypeWithAnnotations.Boxed(type); - _lazyParameters = !hasParameters ? ImmutableArray.Empty : this.ComputeParameters(binder, syntax, diagnostics); - bool isOverride = false; PropertySymbol? overriddenOrImplementedProperty; @@ -247,15 +214,16 @@ protected SourcePropertySymbolBase( } else { - string interfacePropertyName = isIndexer ? WellKnownMemberNames.Indexer : name; - explicitlyImplementedProperty = this.FindExplicitlyImplementedProperty(_explicitInterfaceType, interfacePropertyName, interfaceSpecifier, diagnostics); + CSharpSyntaxNode syntax = CSharpSyntaxNode; + string interfacePropertyName = IsIndexer ? WellKnownMemberNames.Indexer : ((PropertyDeclarationSyntax)syntax).Identifier.ValueText; + explicitlyImplementedProperty = this.FindExplicitlyImplementedProperty(_explicitInterfaceType, interfacePropertyName, GetExplicitInterfaceSpecifier(syntax), diagnostics); this.FindExplicitlyImplementedMemberVerification(explicitlyImplementedProperty, diagnostics); overriddenOrImplementedProperty = explicitlyImplementedProperty; } if ((object)overriddenOrImplementedProperty != null) { - _refCustomModifiers = _refKind != RefKind.None ? overriddenOrImplementedProperty.RefCustomModifiers : ImmutableArray.Empty; + _lazyRefCustomModifiers = _refKind != RefKind.None ? overriddenOrImplementedProperty.RefCustomModifiers : ImmutableArray.Empty; TypeWithAnnotations overriddenPropertyType = overriddenOrImplementedProperty.TypeWithAnnotations; @@ -278,151 +246,18 @@ protected SourcePropertySymbolBase( { var modifierType = Binder.GetWellKnownType(DeclaringCompilation, WellKnownType.System_Runtime_InteropServices_InAttribute, diagnostics, TypeLocation); - _refCustomModifiers = ImmutableArray.Create(CSharpCustomModifier.CreateRequired(modifierType)); - } - - if (!hasAccessorList && arrowExpression != null) - { - Debug.Assert(arrowExpression is object); - _propertyFlags |= Flags.IsExpressionBodied; - _getMethod = CreateExpressionBodiedAccessor( - arrowExpression, - explicitlyImplementedProperty, - aliasQualifierOpt, - isExplicitInterfaceImplementation, - diagnostics); - _setMethod = null; + _lazyRefCustomModifiers = ImmutableArray.Create(CSharpCustomModifier.CreateRequired(modifierType)); } - else - { - _getMethod = CreateAccessorSymbol(isGet: true, getSyntax, explicitlyImplementedProperty, aliasQualifierOpt, isAutoProperty, isExplicitInterfaceImplementation, diagnostics); - _setMethod = CreateAccessorSymbol(isGet: false, setSyntax, explicitlyImplementedProperty, aliasQualifierOpt, isAutoProperty, isExplicitInterfaceImplementation, diagnostics); - if (!hasGetAccessor || !hasSetAccessor) - { - if (!hasGetAccessor && !hasSetAccessor) - { - diagnostics.Add(ErrorCode.ERR_PropertyWithNoAccessors, location, this); - } - else if (_refKind != RefKind.None) - { - if (!hasGetAccessor) - { - diagnostics.Add(ErrorCode.ERR_RefPropertyMustHaveGetAccessor, location, this); - } - } - else if (isAutoProperty) - { - var accessor = _getMethod ?? _setMethod; - if (!hasGetAccessor) - { - diagnostics.Add(ErrorCode.ERR_AutoPropertyMustHaveGetAccessor, accessor!.Locations[0], accessor); - } - } - } - - // Check accessor accessibility is more restrictive than property accessibility. - CheckAccessibilityMoreRestrictive(_getMethod, diagnostics); - CheckAccessibilityMoreRestrictive(_setMethod, diagnostics); - - if ((_getMethod is object) && (_setMethod is object)) - { - if (_refKind != RefKind.None) - { - diagnostics.Add(ErrorCode.ERR_RefPropertyCannotHaveSetAccessor, _setMethod.Locations[0], _setMethod); - } - else if ((_getMethod.LocalAccessibility != Accessibility.NotApplicable) && - (_setMethod.LocalAccessibility != Accessibility.NotApplicable)) - { - // Check accessibility is set on at most one accessor. - diagnostics.Add(ErrorCode.ERR_DuplicatePropertyAccessMods, location, this); - } - else if (_getMethod.LocalDeclaredReadOnly && _setMethod.LocalDeclaredReadOnly) - { - diagnostics.Add(ErrorCode.ERR_DuplicatePropertyReadOnlyMods, location, this); - } - else if (this.IsAbstract) - { - // Check abstract property accessors are not private. - CheckAbstractPropertyAccessorNotPrivate(_getMethod, diagnostics); - CheckAbstractPropertyAccessorNotPrivate(_setMethod, diagnostics); - } - } - else - { - if (!this.IsOverride) - { - var accessor = _getMethod ?? _setMethod; - if (accessor is object) - { - // Check accessibility is not set on the one accessor. - if (accessor.LocalAccessibility != Accessibility.NotApplicable) - { - diagnostics.Add(ErrorCode.ERR_AccessModMissingAccessor, location, this); - } - - // Check that 'readonly' is not set on the one accessor. - if (accessor.LocalDeclaredReadOnly) - { - diagnostics.Add(ErrorCode.ERR_ReadOnlyModMissingAccessor, location, this); - } - } - } - } - } - - if (explicitlyImplementedProperty is object) - { - CheckExplicitImplementationAccessor(_getMethod, explicitlyImplementedProperty.GetMethod, explicitlyImplementedProperty, diagnostics); - CheckExplicitImplementationAccessor(_setMethod, explicitlyImplementedProperty.SetMethod, explicitlyImplementedProperty, diagnostics); - } - - _explicitInterfaceImplementations = + Debug.Assert(isExplicitInterfaceImplementation || _lazyExplicitInterfaceImplementations.IsEmpty); + _lazyExplicitInterfaceImplementations = explicitlyImplementedProperty is null ? ImmutableArray.Empty : ImmutableArray.Create(explicitlyImplementedProperty); - - // get-only auto property should not override settable properties - if ((_propertyFlags & Flags.IsAutoProperty) != 0) - { - if (_setMethod is null && !this.IsReadOnly) - { - diagnostics.Add(ErrorCode.ERR_AutoPropertyMustOverrideSet, location, this); - } - - CheckForFieldTargetedAttribute(diagnostics); - } - - CheckForBlockAndExpressionBody(syntax, diagnostics); } protected abstract Location TypeLocation { get; } - protected abstract SyntaxTokenList GetModifierTokens(SyntaxNode syntax); - - private void CheckForFieldTargetedAttribute(DiagnosticBag diagnostics) - { - var languageVersion = this.DeclaringCompilation.LanguageVersion; - if (languageVersion.AllowAttributesOnBackingFields()) - { - return; - } - - foreach (var attribute in AttributeDeclarationSyntaxList) - { - if (attribute.Target?.GetAttributeLocation() == AttributeLocation.Field) - { - diagnostics.Add( - new CSDiagnosticInfo(ErrorCode.WRN_AttributesOnBackingFieldsNotAvailable, - languageVersion.ToDisplayString(), - new CSharpRequiredLanguageVersion(MessageID.IDS_FeatureAttributesOnBackingFields.RequiredVersion())), - attribute.Target.Location); - } - } - } - - protected abstract void CheckForBlockAndExpressionBody(CSharpSyntaxNode syntax, DiagnosticBag diagnostics); - #nullable disable internal sealed override ImmutableArray NotNullMembers => @@ -454,7 +289,7 @@ private void CheckInitializer( } } - public override RefKind RefKind + public sealed override RefKind RefKind { get { @@ -462,25 +297,85 @@ public override RefKind RefKind } } - public override TypeWithAnnotations TypeWithAnnotations + public sealed override TypeWithAnnotations TypeWithAnnotations { get { - if (_lazyType == null) + EnsureSignature(); + return _lazyType.Value; + } + } + +#nullable enable + + private void EnsureSignature() + { + if (!_state.HasComplete(CompletionPart.FinishPropertyEnsureSignature)) + { + // If this lock ever encloses a potential call to Debugger.NotifyOfCrossThreadDependency, + // then we should call DebuggerUtilities.CallBeforeAcquiringLock() (see method comment for more + // details). + + lock (_syntaxRef) { - var diagnostics = DiagnosticBag.GetInstance(); - var result = this.ComputeType(binder: null, CSharpSyntaxNode, diagnostics); - if (Interlocked.CompareExchange(ref _lazyType, new TypeWithAnnotations.Boxed(result), null) == null) + if (_state.NotePartComplete(CompletionPart.StartPropertyEnsureSignature)) + { + // By setting StartPropertyEnsureSignature, we've committed to doing the work and setting + // FinishPropertyEnsureSignature. So there is no cancellation supported between one and the other. + var diagnostics = DiagnosticBag.GetInstance(); + try + { + EnsureSignature(diagnostics); + AddDiagnostics(diagnostics); + } + finally + { + _state.NotePartComplete(CompletionPart.FinishPropertyEnsureSignature); + diagnostics.Free(); + } + } + else { - this.AddDeclarationDiagnostics(diagnostics); + // Either (1) this thread is in the process of completing the method, + // or (2) some other thread has beat us to the punch and completed the method. + // We can distinguish the two cases here by checking for the FinishPropertyEnsureSignature + // part to be complete, which would only occur if another thread completed this + // method. + // + // The other case, in which this thread is in the process of completing the method, + // requires that we return here even though the work is not complete. That's because + // signature is processed by first populating the return type and parameters by binding + // the syntax from source. Those values are visible to the same thread for the purpose + // of computing which methods are implemented and overridden. But then those values + // may be rewritten (by the same thread) to copy down custom modifiers. In order to + // allow the same thread to see the return type and parameters from the syntax (though + // they do not yet take on their final values), we return here. + + // Due to the fact that this method is potentially reentrant, we must use a + // reentrant lock to avoid deadlock and cannot assert that at this point the work + // has completed (_state.HasComplete(CompletionPart.FinishPropertyEnsureSignature)). } - diagnostics.Free(); } + } + } - return _lazyType.Value; + private void AddDiagnostics(DiagnosticBag diagnostics) + { + if (!diagnostics.IsEmptyWithoutResolution) + { + DiagnosticBag destination = _lazyDiagnosticBag; + if (destination is null) + { + var newBag = new DiagnosticBag(); + destination = Interlocked.CompareExchange(ref _lazyDiagnosticBag, newBag, null!) ?? newBag; + } + + destination.AddRange(diagnostics); } } +#nullable disable + internal bool HasPointerType { get @@ -613,36 +508,79 @@ internal bool IsNew internal bool HasReadOnlyModifier => (_modifiers & DeclarationModifiers.ReadOnly) != 0; - public override MethodSymbol GetMethod - { - get { return _getMethod; } - } +#nullable enable + protected abstract SourcePropertyAccessorSymbol CreateGetAccessorSymbol( + bool isAutoPropertyAccessor, + bool isExplicitInterfaceImplementation, + PropertySymbol? explicitlyImplementedPropertyOpt, + DiagnosticBag diagnostics); - public override MethodSymbol SetMethod - { - get { return _setMethod; } - } + protected abstract SourcePropertyAccessorSymbol CreateSetAccessorSymbol( + bool isAutoPropertyAccessor, + bool isExplicitInterfaceImplementation, + PropertySymbol? explicitlyImplementedPropertyOpt, + DiagnosticBag diagnostics); - internal override Microsoft.Cci.CallingConvention CallingConvention + public sealed override MethodSymbol GetMethod { - get { return (IsStatic ? 0 : Microsoft.Cci.CallingConvention.HasThis); } + get + { + if (_lazyGetMethod is null && (_propertyFlags & Flags.HasGetAccessor) != 0) + { + var diagnostics = DiagnosticBag.GetInstance(); + bool isExplicitInterfaceImplementation = IsExplicitInterfaceImplementation; + var result = CreateGetAccessorSymbol(isAutoPropertyAccessor: IsAutoProperty, isExplicitInterfaceImplementation, + explicitlyImplementedPropertyOpt: isExplicitInterfaceImplementation ? ExplicitInterfaceImplementations.FirstOrDefault() : null, + diagnostics); + if (Interlocked.CompareExchange(ref _lazyGetMethod, result, null) == null) + { + AddDiagnostics(diagnostics); + } + + diagnostics.Free(); + } + + Debug.Assert((_lazyGetMethod is object) == ((_propertyFlags & Flags.HasGetAccessor) != 0)); + return _lazyGetMethod; + } } - public override ImmutableArray Parameters + public sealed override MethodSymbol SetMethod { get { - if (_lazyParameters.IsDefault) + if (_lazySetMethod is null && (_propertyFlags & Flags.HasSetAccessor) != 0) { var diagnostics = DiagnosticBag.GetInstance(); - var result = this.ComputeParameters(binder: null, CSharpSyntaxNode, diagnostics); - if (ImmutableInterlocked.InterlockedInitialize(ref _lazyParameters, result)) + bool isExplicitInterfaceImplementation = IsExplicitInterfaceImplementation; + var result = CreateSetAccessorSymbol(isAutoPropertyAccessor: IsAutoProperty, isExplicitInterfaceImplementation, + explicitlyImplementedPropertyOpt: isExplicitInterfaceImplementation ? ExplicitInterfaceImplementations.FirstOrDefault() : null, + diagnostics); + if (Interlocked.CompareExchange(ref _lazySetMethod, result, null) == null) { - this.AddDeclarationDiagnostics(diagnostics); + AddDiagnostics(diagnostics); } + diagnostics.Free(); } + Debug.Assert((_lazySetMethod is object) == ((_propertyFlags & Flags.HasSetAccessor) != 0)); + return _lazySetMethod; + } + } + +#nullable disable + + internal override Microsoft.Cci.CallingConvention CallingConvention + { + get { return (IsStatic ? 0 : Microsoft.Cci.CallingConvention.HasThis); } + } + + public sealed override ImmutableArray Parameters + { + get + { + EnsureSignature(); return _lazyParameters; } } @@ -650,14 +588,30 @@ public override ImmutableArray Parameters internal override bool IsExplicitInterfaceImplementation => (_propertyFlags & Flags.IsExplicitInterfaceImplementation) != 0; - public override ImmutableArray ExplicitInterfaceImplementations + public sealed override ImmutableArray ExplicitInterfaceImplementations { - get { return _explicitInterfaceImplementations; } + get + { + if (IsExplicitInterfaceImplementation) + { + EnsureSignature(); + } + else + { + Debug.Assert(_lazyExplicitInterfaceImplementations.IsEmpty); + } + + return _lazyExplicitInterfaceImplementations; + } } - public override ImmutableArray RefCustomModifiers + public sealed override ImmutableArray RefCustomModifiers { - get { return _refCustomModifiers; } + get + { + EnsureSignature(); + return _lazyRefCustomModifiers; + } } public override Accessibility DeclaredAccessibility @@ -677,7 +631,10 @@ public bool HasSkipLocalsInitAttribute } } - internal bool IsAutoProperty + internal bool IsAutoPropertyWithGetAccessor + => (_propertyFlags & Flags.IsAutoPropertyWithGetAccessor) == Flags.IsAutoPropertyWithGetAccessor; + + protected bool IsAutoProperty => (_propertyFlags & Flags.IsAutoProperty) != 0; /// @@ -717,6 +674,130 @@ internal SyntaxTree SyntaxTree internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, DiagnosticBag diagnostics) { +#nullable enable + bool isExplicitInterfaceImplementation = IsExplicitInterfaceImplementation; + this.CheckAccessibility(Location, diagnostics, isExplicitInterfaceImplementation); + this.CheckModifiers(isExplicitInterfaceImplementation, Location, IsIndexer, diagnostics); + + bool hasInitializer = (_propertyFlags & Flags.HasInitializer) != 0; + if (hasInitializer) + { + CheckInitializer(IsAutoProperty, ContainingType.IsInterface, IsStatic, Location, diagnostics); + } + + if (IsAutoPropertyWithGetAccessor) + { + Debug.Assert(GetMethod is object); + + if (!IsStatic && SetMethod is object && !SetMethod.IsInitOnly) + { + if (ContainingType.IsReadOnly) + { + diagnostics.Add(ErrorCode.ERR_AutoPropsInRoStruct, Location); + } + else if (HasReadOnlyModifier) + { + diagnostics.Add(ErrorCode.ERR_AutoPropertyWithSetterCantBeReadOnly, Location, this); + } + } + + //issue a diagnostic if the compiler generated attribute ctor is not found. + Binder.ReportUseSiteDiagnosticForSynthesizedAttribute(DeclaringCompilation, + WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor, diagnostics, location: Location); + + if (this.RefKind != RefKind.None && !ContainingType.IsInterface) + { + diagnostics.Add(ErrorCode.ERR_AutoPropertyCannotBeRefReturning, Location, this); + } + + // get-only auto property should not override settable properties + if (SetMethod is null && !this.IsReadOnly) + { + diagnostics.Add(ErrorCode.ERR_AutoPropertyMustOverrideSet, Location, this); + } + } + + if (!IsExpressionBodied) + { + bool hasGetAccessor = GetMethod is object; + bool hasSetAccessor = SetMethod is object; + + if (hasGetAccessor && hasSetAccessor) + { + if (_refKind != RefKind.None) + { + diagnostics.Add(ErrorCode.ERR_RefPropertyCannotHaveSetAccessor, _lazySetMethod.Locations[0], _lazySetMethod); + } + else if ((_lazyGetMethod.LocalAccessibility != Accessibility.NotApplicable) && + (_lazySetMethod.LocalAccessibility != Accessibility.NotApplicable)) + { + // Check accessibility is set on at most one accessor. + diagnostics.Add(ErrorCode.ERR_DuplicatePropertyAccessMods, Location, this); + } + else if (_lazyGetMethod.LocalDeclaredReadOnly && _lazySetMethod.LocalDeclaredReadOnly) + { + diagnostics.Add(ErrorCode.ERR_DuplicatePropertyReadOnlyMods, Location, this); + } + else if (this.IsAbstract) + { + // Check abstract property accessors are not private. + CheckAbstractPropertyAccessorNotPrivate(_lazyGetMethod, diagnostics); + CheckAbstractPropertyAccessorNotPrivate(_lazySetMethod, diagnostics); + } + } + else + { + if (!hasGetAccessor && !hasSetAccessor) + { + diagnostics.Add(ErrorCode.ERR_PropertyWithNoAccessors, Location, this); + } + else if (RefKind != RefKind.None) + { + if (!hasGetAccessor) + { + diagnostics.Add(ErrorCode.ERR_RefPropertyMustHaveGetAccessor, Location, this); + } + } + else if (!hasGetAccessor && IsAutoProperty) + { + diagnostics.Add(ErrorCode.ERR_AutoPropertyMustHaveGetAccessor, _lazySetMethod!.Locations[0], _lazySetMethod); + } + + if (!this.IsOverride) + { + var accessor = _lazyGetMethod ?? _lazySetMethod; + if (accessor is object) + { + // Check accessibility is not set on the one accessor. + if (accessor.LocalAccessibility != Accessibility.NotApplicable) + { + diagnostics.Add(ErrorCode.ERR_AccessModMissingAccessor, Location, this); + } + + // Check that 'readonly' is not set on the one accessor. + if (accessor.LocalDeclaredReadOnly) + { + diagnostics.Add(ErrorCode.ERR_ReadOnlyModMissingAccessor, Location, this); + } + } + } + } + + // Check accessor accessibility is more restrictive than property accessibility. + CheckAccessibilityMoreRestrictive(_lazyGetMethod, diagnostics); + CheckAccessibilityMoreRestrictive(_lazySetMethod, diagnostics); + } + + PropertySymbol? explicitlyImplementedProperty = ExplicitInterfaceImplementations.FirstOrDefault(); + + if (explicitlyImplementedProperty is object) + { + CheckExplicitImplementationAccessor(GetMethod, explicitlyImplementedProperty.GetMethod, explicitlyImplementedProperty, diagnostics); + CheckExplicitImplementationAccessor(SetMethod, explicitlyImplementedProperty.SetMethod, explicitlyImplementedProperty, diagnostics); + } + +#nullable disable + Location location = TypeLocation; var compilation = DeclaringCompilation; @@ -733,9 +814,9 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, _explicitInterfaceType.CheckAllConstraints(compilation, conversions, new SourceLocation(explicitInterfaceSpecifier.Name), diagnostics); // Note: we delayed nullable-related checks that could pull on NonNullTypes - if (!_explicitInterfaceImplementations.IsEmpty) + if (explicitlyImplementedProperty is object) { - TypeSymbol.CheckNullableReferenceTypeMismatchOnImplementingMember(this.ContainingType, this, _explicitInterfaceImplementations[0], isExplicit: true, diagnostics); + TypeSymbol.CheckNullableReferenceTypeMismatchOnImplementingMember(this.ContainingType, this, explicitlyImplementedProperty, isExplicit: true, diagnostics); } } @@ -832,24 +913,6 @@ private void CheckModifiers(bool isExplicitInterfaceImplementation, Location loc } } -#nullable enable - protected abstract SourcePropertyAccessorSymbol? CreateAccessorSymbol( - bool isGet, - CSharpSyntaxNode? syntaxOpt, - PropertySymbol? explicitlyImplementedPropertyOpt, - string? aliasQualifierOpt, - bool isAutoPropertyAccessor, - bool isExplicitInterfaceImplementation, - DiagnosticBag diagnostics); - - protected abstract SourcePropertyAccessorSymbol CreateExpressionBodiedAccessor( - ArrowExpressionClauseSyntax syntax, - PropertySymbol? explicitlyImplementedPropertyOpt, - string? aliasQualifierOpt, - bool isExplicitInterfaceImplementation, - DiagnosticBag diagnostics); -#nullable disable - private void CheckAccessibilityMoreRestrictive(SourcePropertyAccessorSymbol accessor, DiagnosticBag diagnostics) { if (((object)accessor != null) && @@ -929,8 +992,8 @@ internal SynthesizedSealedPropertyAccessor SynthesizedSealedAccessorOpt { get { - bool hasGetter = (object)_getMethod != null; - bool hasSetter = (object)_setMethod != null; + bool hasGetter = GetMethod is object; + bool hasSetter = SetMethod is object; if (!this.IsSealed || (hasGetter && hasSetter)) { return null; @@ -952,15 +1015,15 @@ internal SynthesizedSealedPropertyAccessor SynthesizedSealedAccessorOpt /// private SynthesizedSealedPropertyAccessor MakeSynthesizedSealedAccessor() { - Debug.Assert(this.IsSealed && ((object)_getMethod == null || (object)_setMethod == null)); + Debug.Assert(this.IsSealed && (GetMethod is null || SetMethod is null)); - if ((object)_getMethod != null) + if (GetMethod is object) { // need to synthesize setter MethodSymbol overriddenAccessor = this.GetOwnOrInheritedSetMethod(); return (object)overriddenAccessor == null ? null : new SynthesizedSealedPropertyAccessor(this, overriddenAccessor); } - else if ((object)_setMethod != null) + else if (SetMethod is object) { // need to synthesize getter MethodSymbol overriddenAccessor = this.GetOwnOrInheritedGetMethod(); @@ -986,7 +1049,7 @@ private SynthesizedSealedPropertyAccessor MakeSynthesizedSealedAccessor() AttributeLocation IAttributeTargetSymbol.DefaultAttributeLocation => AttributeLocation.Property; AttributeLocation IAttributeTargetSymbol.AllowedAttributeLocations - => (_propertyFlags & Flags.IsAutoProperty) != 0 + => IsAutoPropertyWithGetAccessor ? AttributeLocation.Property | AttributeLocation.Field : AttributeLocation.Property; @@ -1349,6 +1412,11 @@ internal override void ForceComplete(SourceLocation locationOpt, CancellationTok GetAttributes(); break; + case CompletionPart.StartPropertyEnsureSignature: + case CompletionPart.FinishPropertyEnsureSignature: + EnsureSignature(); + break; + case CompletionPart.StartPropertyParameters: case CompletionPart.FinishPropertyParameters: { @@ -1405,6 +1473,30 @@ internal override void ForceComplete(SourceLocation locationOpt, CancellationTok } break; + case CompletionPart.StartPropertyFinalCompletion: + case CompletionPart.FinishPropertyFinalCompletion: + { + if (_state.NotePartComplete(CompletionPart.StartPropertyFinalCompletion)) + { + DiagnosticBag diagnostic = _lazyDiagnosticBag; + if (diagnostic is object) + { + Debug.Assert(!diagnostic.IsEmptyWithoutResolution); + this.AddDeclarationDiagnostics(diagnostic); + _lazyDiagnosticBag = null; + } + + var completedOnThisThread = _state.NotePartComplete(CompletionPart.FinishPropertyFinalCompletion); + Debug.Assert(completedOnThisThread); + } + else + { + // StartPropertyFinalCompletion was completed by another thread. Wait for it to finish the work. + _state.SpinWaitComplete(CompletionPart.FinishPropertyFinalCompletion, cancellationToken); + } + } + break; + case CompletionPart.None: return; @@ -1425,7 +1517,7 @@ protected virtual void ValidatePropertyType(DiagnosticBag diagnostics) { diagnostics.Add(ErrorCode.ERR_FieldCantBeRefAny, TypeLocation, type); } - else if (this.IsAutoProperty && type.IsRefLikeType && (this.IsStatic || !this.ContainingType.IsRefLikeType)) + else if (this.IsAutoPropertyWithGetAccessor && type.IsRefLikeType && (this.IsStatic || !this.ContainingType.IsRefLikeType)) { diagnostics.Add(ErrorCode.ERR_FieldAutoPropCantBeByRefLike, TypeLocation, type); } @@ -1434,9 +1526,8 @@ protected virtual void ValidatePropertyType(DiagnosticBag diagnostics) #endregion #nullable enable - protected abstract ImmutableArray ComputeParameters(Binder? binder, CSharpSyntaxNode syntax, DiagnosticBag diagnostics); - protected abstract TypeWithAnnotations ComputeType(Binder? binder, SyntaxNode syntax, DiagnosticBag diagnostics); + protected abstract (TypeWithAnnotations Type, ImmutableArray Parameters) MakeParametersAndBindType(DiagnosticBag diagnostics); protected static ExplicitInterfaceSpecifierSyntax? GetExplicitInterfaceSpecifier(SyntaxNode syntax) => (syntax as BasePropertyDeclarationSyntax)?.ExplicitInterfaceSpecifier; diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs index 805ae55024e5f..c412dde69ec17 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs @@ -17,34 +17,29 @@ internal sealed class SynthesizedRecordEqualityContractProperty : SourceProperty { internal const string PropertyName = "EqualityContract"; - public SynthesizedRecordEqualityContractProperty( - SourceMemberContainerTypeSymbol containingType, - DiagnosticBag diagnostics) + public SynthesizedRecordEqualityContractProperty(SourceMemberContainerTypeSymbol containingType) : base( containingType, - binder: null, syntax: (CSharpSyntaxNode)containingType.SyntaxReferences[0].GetSyntax(), - getSyntax: (CSharpSyntaxNode)containingType.SyntaxReferences[0].GetSyntax(), - setSyntax: null, - arrowExpression: null, - interfaceSpecifier: null, + hasGetAccessor: true, + hasSetAccessor: false, + isExplicitInterfaceImplementation: false, + explicitInterfaceType: null, + aliasQualifierOpt: null, modifiers: (containingType.IsSealed, containingType.BaseTypeNoUseSiteDiagnostics.IsObjectType()) switch { (true, true) => DeclarationModifiers.Private, (false, true) => DeclarationModifiers.Protected | DeclarationModifiers.Virtual, (_, false) => DeclarationModifiers.Protected | DeclarationModifiers.Override }, - isIndexer: false, hasInitializer: false, isAutoProperty: false, - hasAccessorList: false, + isExpressionBodied: false, isInitOnly: false, RefKind.None, PropertyName, - containingType.Locations[0], - typeOpt: TypeWithAnnotations.Create(Binder.GetWellKnownType(containingType.DeclaringCompilation, WellKnownType.System_Type, diagnostics, containingType.Locations[0]), NullableAnnotation.NotAnnotated), - hasParameters: false, - diagnostics) + indexerNameAttributeLists: new SyntaxList(), + containingType.Locations[0]) { } @@ -60,59 +55,26 @@ public override SyntaxList AttributeDeclarationSyntaxList protected override Location TypeLocation => ContainingType.Locations[0]; - protected override SyntaxTokenList GetModifierTokens(SyntaxNode syntax) - => new SyntaxTokenList(); - - protected override void CheckForBlockAndExpressionBody(CSharpSyntaxNode syntax, DiagnosticBag diagnostics) - { - // Nothing to do here - } - - protected override SourcePropertyAccessorSymbol? CreateAccessorSymbol( - bool isGet, - CSharpSyntaxNode? syntax, - PropertySymbol? explicitlyImplementedPropertyOpt, - string? aliasQualifierOpt, - bool isAutoPropertyAccessor, - bool isExplicitInterfaceImplementation, - DiagnosticBag diagnostics) + protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, bool isExplicitInterfaceImplementation, PropertySymbol? explicitlyImplementedPropertyOpt, DiagnosticBag diagnostics) { - if (!isGet) - { - return null; - } - - Debug.Assert(syntax is object); - return SourcePropertyAccessorSymbol.CreateAccessorSymbol( ContainingType, this, _modifiers, ContainingType.Locations[0], - syntax, + (CSharpSyntaxNode)((SourceMemberContainerTypeSymbol)ContainingType).SyntaxReferences[0].GetSyntax(), diagnostics); } - protected override SourcePropertyAccessorSymbol CreateExpressionBodiedAccessor( - ArrowExpressionClauseSyntax syntax, - PropertySymbol? explicitlyImplementedPropertyOpt, - string? aliasQualifierOpt, - bool isExplicitInterfaceImplementation, - DiagnosticBag diagnostics) + protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(bool isAutoPropertyAccessor, bool isExplicitInterfaceImplementation, PropertySymbol? explicitlyImplementedPropertyOpt, DiagnosticBag diagnostics) { - // There should be no expression-bodied synthesized record properties throw ExceptionUtilities.Unreachable; } - protected override ImmutableArray ComputeParameters(Binder? binder, CSharpSyntaxNode syntax, DiagnosticBag diagnostics) - { - return ImmutableArray.Empty; - } - - protected override TypeWithAnnotations ComputeType(Binder? binder, SyntaxNode syntax, DiagnosticBag diagnostics) + protected override (TypeWithAnnotations Type, ImmutableArray Parameters) MakeParametersAndBindType(DiagnosticBag diagnostics) { - // No need to worry about reporting use-site diagnostics, we already did that in the constructor - return TypeWithAnnotations.Create(DeclaringCompilation.GetWellKnownType(WellKnownType.System_Type), NullableAnnotation.NotAnnotated); + return (TypeWithAnnotations.Create(Binder.GetWellKnownType(DeclaringCompilation, WellKnownType.System_Type, diagnostics, Location), NullableAnnotation.NotAnnotated), + ImmutableArray.Empty); } protected override bool HasPointerTypeSyntactically => false; diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs index 983b439df057c..4b41d02033955 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs @@ -17,61 +17,53 @@ public SynthesizedRecordPropertySymbol( SourceMemberContainerTypeSymbol containingType, CSharpSyntaxNode syntax, ParameterSymbol backingParameter, - bool isOverride, - DiagnosticBag diagnostics) + bool isOverride) : base( containingType, - binder: null, syntax: syntax, - getSyntax: syntax, - setSyntax: syntax, - arrowExpression: null, - interfaceSpecifier: null, + hasGetAccessor: true, + hasSetAccessor: true, + isExplicitInterfaceImplementation: false, + explicitInterfaceType: null, + aliasQualifierOpt: null, modifiers: DeclarationModifiers.Public | (isOverride ? DeclarationModifiers.Override : DeclarationModifiers.None), - isIndexer: false, hasInitializer: true, // Synthesized record properties always have a synthesized initializer isAutoProperty: true, - hasAccessorList: false, + isExpressionBodied: false, isInitOnly: true, RefKind.None, backingParameter.Name, - backingParameter.Locations[0], - typeOpt: backingParameter.TypeWithAnnotations, - hasParameters: false, - diagnostics) + indexerNameAttributeLists: new SyntaxList(), + backingParameter.Locations[0]) { BackingParameter = (SourceParameterSymbol)backingParameter; } - public override IAttributeTargetSymbol AttributesOwner => BackingParameter as IAttributeTargetSymbol ?? this; protected override Location TypeLocation => ((ParameterSyntax)CSharpSyntaxNode).Type!.Location; - protected override SyntaxTokenList GetModifierTokens(SyntaxNode syntax) - => new SyntaxTokenList(); - public override SyntaxList AttributeDeclarationSyntaxList => BackingParameter.AttributeDeclarationList; - protected override void CheckForBlockAndExpressionBody(CSharpSyntaxNode syntax, DiagnosticBag diagnostics) + protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, bool isExplicitInterfaceImplementation, PropertySymbol? explicitlyImplementedPropertyOpt, DiagnosticBag diagnostics) { - // Nothing to do here + Debug.Assert(isAutoPropertyAccessor); + return CreateAccessorSymbol(isGet: true, CSharpSyntaxNode, diagnostics); } - protected override SourcePropertyAccessorSymbol CreateAccessorSymbol( - bool isGet, - CSharpSyntaxNode? syntax, - PropertySymbol? explicitlyImplementedPropertyOpt, - string? aliasQualifierOpt, - bool isAutoPropertyAccessor, - bool isExplicitInterfaceImplementation, - DiagnosticBag diagnostics) + protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(bool isAutoPropertyAccessor, bool isExplicitInterfaceImplementation, PropertySymbol? explicitlyImplementedPropertyOpt, DiagnosticBag diagnostics) { - Debug.Assert(syntax is object); Debug.Assert(isAutoPropertyAccessor); + return CreateAccessorSymbol(isGet: false, CSharpSyntaxNode, diagnostics); + } + private SourcePropertyAccessorSymbol CreateAccessorSymbol( + bool isGet, + CSharpSyntaxNode syntax, + DiagnosticBag diagnostics) + { return SourcePropertyAccessorSymbol.CreateAccessorSymbol( isGet, usesInit: !isGet, // the setter is always init-only @@ -83,25 +75,10 @@ protected override SourcePropertyAccessorSymbol CreateAccessorSymbol( diagnostics); } - protected override SourcePropertyAccessorSymbol CreateExpressionBodiedAccessor( - ArrowExpressionClauseSyntax syntax, - PropertySymbol? explicitlyImplementedPropertyOpt, - string? aliasQualifierOpt, - bool isExplicitInterfaceImplementation, - DiagnosticBag diagnostics) - { - // There should be no expression-bodied synthesized record properties - throw ExceptionUtilities.Unreachable; - } - - protected override ImmutableArray ComputeParameters(Binder? binder, CSharpSyntaxNode syntax, DiagnosticBag diagnostics) - { - return ImmutableArray.Empty; - } - - protected override TypeWithAnnotations ComputeType(Binder? binder, SyntaxNode syntax, DiagnosticBag diagnostics) + protected override (TypeWithAnnotations Type, ImmutableArray Parameters) MakeParametersAndBindType(DiagnosticBag diagnostics) { - return BackingParameter.TypeWithAnnotations; + return (BackingParameter.TypeWithAnnotations, + ImmutableArray.Empty); } protected override bool HasPointerTypeSyntactically diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs index 3f87f1e205ee2..a61a3b0842982 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs @@ -120,5 +120,36 @@ internal override bool HasRuntimeSpecialName public override bool IsImplicitlyDeclared => true; + + internal override void PostDecodeWellKnownAttributes(ImmutableArray boundAttributes, ImmutableArray allAttributeSyntaxNodes, DiagnosticBag diagnostics, AttributeLocation symbolPart, WellKnownAttributeData decodedData) + { + base.PostDecodeWellKnownAttributes(boundAttributes, allAttributeSyntaxNodes, diagnostics, symbolPart, decodedData); + + if (!allAttributeSyntaxNodes.IsEmpty && _property.IsAutoPropertyWithGetAccessor) + { + CheckForFieldTargetedAttribute(diagnostics); + } + } + + private void CheckForFieldTargetedAttribute(DiagnosticBag diagnostics) + { + var languageVersion = this.DeclaringCompilation.LanguageVersion; + if (languageVersion.AllowAttributesOnBackingFields()) + { + return; + } + + foreach (var attribute in AttributeDeclarationSyntaxList) + { + if (attribute.Target?.GetAttributeLocation() == AttributeLocation.Field) + { + diagnostics.Add( + new CSDiagnosticInfo(ErrorCode.WRN_AttributesOnBackingFieldsNotAvailable, + languageVersion.ToDisplayString(), + new CSharpRequiredLanguageVersion(MessageID.IDS_FeatureAttributesOnBackingFields.RequiredVersion())), + attribute.Target.Location); + } + } + } } } diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EmitMetadataTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EmitMetadataTests.cs index a38eb81dda40e..1853d88758217 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EmitMetadataTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EmitMetadataTests.cs @@ -1294,7 +1294,7 @@ private static void VerifyAutoProperty(PropertySymbol property, bool isFromSourc { if (property is SourcePropertySymbol sourceProperty) { - Assert.True(sourceProperty.IsAutoProperty); + Assert.True(sourceProperty.IsAutoPropertyWithGetAccessor); } } else From 8748dbfc6ca74f12bc6c5ff5dd61b852d31a6ab4 Mon Sep 17 00:00:00 2001 From: AlekseyTs Date: Fri, 13 Nov 2020 11:21:35 -0800 Subject: [PATCH 2/7] Nullable fixup --- .../Symbols/Source/SourceMemberContainerSymbol.cs | 2 ++ .../Portable/Symbols/Source/SourcePropertySymbolBase.cs | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs index 44dfc15e0e7a6..5be7d7f72f8f9 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs @@ -3320,6 +3320,8 @@ void addProperty(SynthesizedRecordPropertySymbol property) { existingOrAddedMembers.Add(property); members.Add(property); + Debug.Assert(property.GetMethod is object); + Debug.Assert(property.SetMethod is object); members.Add(property.GetMethod); members.Add(property.SetMethod); members.Add(property.BackingField); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs index c377a44813ff7..22fe71cd8e9ed 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs @@ -363,7 +363,7 @@ private void AddDiagnostics(DiagnosticBag diagnostics) { if (!diagnostics.IsEmptyWithoutResolution) { - DiagnosticBag destination = _lazyDiagnosticBag; + DiagnosticBag? destination = _lazyDiagnosticBag; if (destination is null) { var newBag = new DiagnosticBag(); @@ -521,7 +521,7 @@ protected abstract SourcePropertyAccessorSymbol CreateSetAccessorSymbol( PropertySymbol? explicitlyImplementedPropertyOpt, DiagnosticBag diagnostics); - public sealed override MethodSymbol GetMethod + public sealed override MethodSymbol? GetMethod { get { @@ -545,7 +545,7 @@ public sealed override MethodSymbol GetMethod } } - public sealed override MethodSymbol SetMethod + public sealed override MethodSymbol? SetMethod { get { @@ -724,6 +724,9 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, if (hasGetAccessor && hasSetAccessor) { + Debug.Assert(_lazyGetMethod is object); + Debug.Assert(_lazySetMethod is object); + if (_refKind != RefKind.None) { diagnostics.Add(ErrorCode.ERR_RefPropertyCannotHaveSetAccessor, _lazySetMethod.Locations[0], _lazySetMethod); From c0557dcb0095c217b1f3294d1ac876c0f4250ff5 Mon Sep 17 00:00:00 2001 From: AlekseyTs Date: Fri, 13 Nov 2020 12:05:37 -0800 Subject: [PATCH 3/7] Nullable fixup --- .../CSharp/Test/Semantic/Semantics/RecordTests.cs | 6 +++--- .../CSharp/Test/Symbol/Symbols/Source/RecordTests.cs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 4566738df97bb..e5ef840d1d695 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -18159,7 +18159,7 @@ static void Main() var equalityContractGet = equalityContract.GetMethod; Assert.Equal("System.Type B.EqualityContract { get; }", equalityContract.ToTestDisplayString()); - Assert.Equal(Accessibility.Protected, equalityContractGet.DeclaredAccessibility); + Assert.Equal(Accessibility.Protected, equalityContractGet!.DeclaredAccessibility); Assert.False(equalityContractGet.IsAbstract); Assert.False(equalityContractGet.IsVirtual); Assert.True(equalityContractGet.IsOverride); @@ -18275,7 +18275,7 @@ static void Main() var equalityContractGet = equalityContract.GetMethod; Assert.Equal("System.Type B.EqualityContract { get; }", equalityContract.ToTestDisplayString()); - Assert.Equal(Accessibility.Protected, equalityContractGet.DeclaredAccessibility); + Assert.Equal(Accessibility.Protected, equalityContractGet!.DeclaredAccessibility); Assert.False(equalityContractGet.IsAbstract); Assert.False(equalityContractGet.IsVirtual); Assert.True(equalityContractGet.IsOverride); @@ -18348,7 +18348,7 @@ static void Main() var equalityContractGet = equalityContract.GetMethod; Assert.Equal("System.Type B.EqualityContract { get; }", equalityContract.ToTestDisplayString()); - Assert.Equal(modifiers == "sealed " ? Accessibility.Private : Accessibility.Protected, equalityContractGet.DeclaredAccessibility); + Assert.Equal(modifiers == "sealed " ? Accessibility.Private : Accessibility.Protected, equalityContractGet!.DeclaredAccessibility); Assert.False(equalityContractGet.IsAbstract); Assert.Equal(modifiers != "sealed ", equalityContractGet.IsVirtual); Assert.False(equalityContractGet.IsOverride); diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs index a57c3c59e78f4..3c9bfe700fdba 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs @@ -150,7 +150,7 @@ public void GeneratedProperties() var x = (SourcePropertySymbolBase)c.GetProperty("x"); Assert.NotNull(x.GetMethod); - Assert.Equal(MethodKind.PropertyGet, x.GetMethod.MethodKind); + Assert.Equal(MethodKind.PropertyGet, x.GetMethod!.MethodKind); Assert.Equal(SpecialType.System_Int32, x.Type.SpecialType); Assert.False(x.IsReadOnly); Assert.False(x.IsWriteOnly); @@ -175,7 +175,7 @@ public void GeneratedProperties() Assert.Equal(Accessibility.Public, getAccessor.DeclaredAccessibility); var setAccessor = x.SetMethod; - Assert.Equal(x, setAccessor.AssociatedSymbol); + Assert.Equal(x, setAccessor!.AssociatedSymbol); Assert.True(setAccessor.IsImplicitlyDeclared); Assert.Equal(c, setAccessor.ContainingSymbol); Assert.Equal(c, setAccessor.ContainingType); @@ -184,7 +184,7 @@ public void GeneratedProperties() var y = (SourcePropertySymbolBase)c.GetProperty("y"); Assert.NotNull(y.GetMethod); - Assert.Equal(MethodKind.PropertyGet, y.GetMethod.MethodKind); + Assert.Equal(MethodKind.PropertyGet, y.GetMethod!.MethodKind); Assert.Equal(SpecialType.System_Int32, y.Type.SpecialType); Assert.False(y.IsReadOnly); Assert.False(y.IsWriteOnly); @@ -208,7 +208,7 @@ public void GeneratedProperties() Assert.Equal(c, getAccessor.ContainingType); setAccessor = y.SetMethod; - Assert.Equal(y, setAccessor.AssociatedSymbol); + Assert.Equal(y, setAccessor!.AssociatedSymbol); Assert.True(setAccessor.IsImplicitlyDeclared); Assert.Equal(c, setAccessor.ContainingSymbol); Assert.Equal(c, setAccessor.ContainingType); From f3c745619e38068b08dbe13dcfbb7975e088f6d9 Mon Sep 17 00:00:00 2001 From: AlekseyTs Date: Fri, 13 Nov 2020 14:39:25 -0800 Subject: [PATCH 4/7] Formatting fixup --- .../Symbols/Source/SourcePropertySymbolBase.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs index 22fe71cd8e9ed..ab5eff3ea1afd 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs @@ -523,12 +523,12 @@ protected abstract SourcePropertyAccessorSymbol CreateSetAccessorSymbol( public sealed override MethodSymbol? GetMethod { - get + get { if (_lazyGetMethod is null && (_propertyFlags & Flags.HasGetAccessor) != 0) { var diagnostics = DiagnosticBag.GetInstance(); - bool isExplicitInterfaceImplementation = IsExplicitInterfaceImplementation; + bool isExplicitInterfaceImplementation = IsExplicitInterfaceImplementation; var result = CreateGetAccessorSymbol(isAutoPropertyAccessor: IsAutoProperty, isExplicitInterfaceImplementation, explicitlyImplementedPropertyOpt: isExplicitInterfaceImplementation ? ExplicitInterfaceImplementations.FirstOrDefault() : null, diagnostics); @@ -541,7 +541,7 @@ public sealed override MethodSymbol? GetMethod } Debug.Assert((_lazyGetMethod is object) == ((_propertyFlags & Flags.HasGetAccessor) != 0)); - return _lazyGetMethod; + return _lazyGetMethod; } } @@ -565,7 +565,7 @@ public sealed override MethodSymbol? SetMethod } Debug.Assert((_lazySetMethod is object) == ((_propertyFlags & Flags.HasSetAccessor) != 0)); - return _lazySetMethod; + return _lazySetMethod; } } @@ -590,8 +590,8 @@ internal override bool IsExplicitInterfaceImplementation public sealed override ImmutableArray ExplicitInterfaceImplementations { - get - { + get + { if (IsExplicitInterfaceImplementation) { EnsureSignature(); @@ -601,16 +601,16 @@ public sealed override ImmutableArray ExplicitInterfaceImplement Debug.Assert(_lazyExplicitInterfaceImplementations.IsEmpty); } - return _lazyExplicitInterfaceImplementations; + return _lazyExplicitInterfaceImplementations; } } public sealed override ImmutableArray RefCustomModifiers { - get + get { EnsureSignature(); - return _lazyRefCustomModifiers; + return _lazyRefCustomModifiers; } } From 167e2a1edda3efafaa7130e823a26ab4ef97dbb2 Mon Sep 17 00:00:00 2001 From: AlekseyTs Date: Wed, 18 Nov 2020 06:14:40 -0800 Subject: [PATCH 5/7] PR feedback --- .../CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs index ab5eff3ea1afd..ff3b6cc031724 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs @@ -689,7 +689,7 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, { Debug.Assert(GetMethod is object); - if (!IsStatic && SetMethod is object && !SetMethod.IsInitOnly) + if (!IsStatic && SetMethod is { IsInitOnly: false }) { if (ContainingType.IsReadOnly) { @@ -1418,6 +1418,7 @@ internal override void ForceComplete(SourceLocation locationOpt, CancellationTok case CompletionPart.StartPropertyEnsureSignature: case CompletionPart.FinishPropertyEnsureSignature: EnsureSignature(); + Debug.Assert(_state.HasComplete(CompletionPart.FinishPropertyEnsureSignature)); break; case CompletionPart.StartPropertyParameters: From 5f41eed23f04b7af68010cb05eb5fe6dd33afccd Mon Sep 17 00:00:00 2001 From: AlekseyTs Date: Wed, 18 Nov 2020 09:57:06 -0800 Subject: [PATCH 6/7] Adgjust comment as suggested Co-authored-by: Jared Parsons --- .../CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs index ff3b6cc031724..3741dcaff52d2 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs @@ -161,7 +161,7 @@ protected SourcePropertySymbolBase( } } - _sourceName = _sourceName ?? memberName; //sourceName may have been set while loading attributes + _sourceName = _sourceName ?? memberName; // _sourceName may have been set while loading attributes _name = isIndexer ? ExplicitInterfaceHelpers.GetMemberName(WellKnownMemberNames.Indexer, _explicitInterfaceType, aliasQualifierOpt) : _sourceName; if ((isAutoProperty && hasGetAccessor) || hasInitializer) From 24391b743c0cc4fdaaf2e2167c7a1c1b464b301f Mon Sep 17 00:00:00 2001 From: AlekseyTs Date: Thu, 19 Nov 2020 05:21:55 -0800 Subject: [PATCH 7/7] Rename private function --- .../Portable/Symbols/Source/SourcePropertySymbolBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs index 3741dcaff52d2..0b2a75c8661a5 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs @@ -175,7 +175,7 @@ protected SourcePropertySymbolBase( } } - private void EnsureSignature(DiagnosticBag diagnostics) + private void EnsureSignatureGuarded(DiagnosticBag diagnostics) { PropertySymbol? explicitlyImplementedProperty = null; _lazyRefCustomModifiers = ImmutableArray.Empty; @@ -325,7 +325,7 @@ private void EnsureSignature() var diagnostics = DiagnosticBag.GetInstance(); try { - EnsureSignature(diagnostics); + EnsureSignatureGuarded(diagnostics); AddDiagnostics(diagnostics); } finally