From 465fbf63798a012ff0111ff34d4f0a6528c4910c Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 17 Jun 2021 11:02:43 -0700 Subject: [PATCH 1/4] Use ArrayBuilder for clarity --- .../AnonymousType.ConstructorSymbol.cs | 7 ++++--- .../Portable/Symbols/ReducedExtensionMethodSymbol.cs | 6 +++--- .../Symbols/Retargeting/RetargetingMethodSymbol.cs | 6 +++--- .../Symbols/Synthesized/SynthesizedDelegateSymbol.cs | 10 ++++++---- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.ConstructorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.ConstructorSymbol.cs index 8abc613b9ccdc..abd52b0b285c8 100644 --- a/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.ConstructorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.ConstructorSymbol.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Immutable; +using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.CodeAnalysis.CSharp.Symbols { @@ -25,13 +26,13 @@ internal AnonymousTypeConstructorSymbol(NamedTypeSymbol container, ImmutableArra int fieldsCount = properties.Length; if (fieldsCount > 0) { - ParameterSymbol[] paramsArr = new ParameterSymbol[fieldsCount]; + var paramsArr = ArrayBuilder.GetInstance(fieldsCount); for (int index = 0; index < fieldsCount; index++) { PropertySymbol property = properties[index]; - paramsArr[index] = SynthesizedParameterSymbol.Create(this, property.TypeWithAnnotations, index, RefKind.None, property.Name); + paramsArr.Add(SynthesizedParameterSymbol.Create(this, property.TypeWithAnnotations, index, RefKind.None, property.Name)); } - _parameters = paramsArr.AsImmutableOrNull(); + _parameters = paramsArr.ToImmutableAndFree(); } else { diff --git a/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs index 3d3e9c4236c67..021af1134ff3c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs @@ -557,13 +557,13 @@ private ImmutableArray MakeParameters() } else { - var parameters = new ParameterSymbol[count - 1]; + var parameters = ArrayBuilder.GetInstance(count - 1); for (int i = 0; i < count - 1; i++) { - parameters[i] = new ReducedExtensionMethodParameterSymbol(this, reducedFromParameters[i + 1]); + parameters.Add(new ReducedExtensionMethodParameterSymbol(this, reducedFromParameters[i + 1])); } - return parameters.AsImmutableOrNull(); + return parameters.ToImmutableAndFree(); } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs index f51b06545c792..b569a4e285e8b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs @@ -172,14 +172,14 @@ private ImmutableArray RetargetParameters() } else { - ParameterSymbol[] parameters = new ParameterSymbol[count]; + var parameters = ArrayBuilder.GetInstance(count); for (int i = 0; i < count; i++) { - parameters[i] = new RetargetingMethodParameterSymbol(this, list[i]); + parameters.Add(new RetargetingMethodParameterSymbol(this, list[i])); } - return parameters.AsImmutableOrNull(); + return parameters.ToImmutableAndFree(); } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs index 6a428d3b1d7cf..c91d8f3f3136c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.Symbols @@ -125,16 +126,17 @@ internal InvokeMethod(SynthesizedDelegateSymbol containingType, BitVector byRefP // if we are given Void type the method returns Void, otherwise its return type is the last type parameter of the delegate: _returnType = voidReturnTypeOpt ?? typeParams.Last(); - var parameters = new ParameterSymbol[typeParams.Length - ((object)voidReturnTypeOpt != null ? 0 : 1)]; - for (int i = 0; i < parameters.Length; i++) + int parameterCount = typeParams.Length - ((object)voidReturnTypeOpt != null ? 0 : 1); + var parameters = ArrayBuilder.GetInstance(parameterCount); + for (int i = 0; i < parameterCount; i++) { // we don't need to distinguish between out and ref since this is an internal synthesized symbol: var refKind = !byRefParameters.IsNull && byRefParameters[i] ? RefKind.Ref : RefKind.None; - parameters[i] = SynthesizedParameterSymbol.Create(this, TypeWithAnnotations.Create(typeParams[i]), i, refKind); + parameters.Add(SynthesizedParameterSymbol.Create(this, TypeWithAnnotations.Create(typeParams[i]), i, refKind)); } - _parameters = parameters.AsImmutableOrNull(); + _parameters = parameters.ToImmutableAndFree(); } public override string Name From 721613a3dc3caa8405a4d9884d05b7a58ebb9ccd Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 17 Jun 2021 11:03:26 -0700 Subject: [PATCH 2/4] Use InterlockedInitialize instead of InterlockedCompareExchange for clarity --- .../CSharp/Portable/Lowering/SynthesizedMethodBaseSymbol.cs | 2 +- .../CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs | 2 +- .../Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs | 2 +- .../CSharp/Portable/Symbols/SubstitutedMethodSymbol.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/SynthesizedMethodBaseSymbol.cs b/src/Compilers/CSharp/Portable/Lowering/SynthesizedMethodBaseSymbol.cs index 6774c46d4f87d..7d213532146fa 100644 --- a/src/Compilers/CSharp/Portable/Lowering/SynthesizedMethodBaseSymbol.cs +++ b/src/Compilers/CSharp/Portable/Lowering/SynthesizedMethodBaseSymbol.cs @@ -106,7 +106,7 @@ public sealed override ImmutableArray Parameters { if (_parameters.IsDefault) { - ImmutableInterlocked.InterlockedCompareExchange(ref _parameters, MakeParameters(), default(ImmutableArray)); + ImmutableInterlocked.InterlockedInitialize(ref _parameters, MakeParameters()); } return _parameters; } diff --git a/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs index 021af1134ff3c..e21bad8622851 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs @@ -513,7 +513,7 @@ public override ImmutableArray Parameters { if (_lazyParameters.IsDefault) { - ImmutableInterlocked.InterlockedCompareExchange(ref _lazyParameters, this.MakeParameters(), default(ImmutableArray)); + ImmutableInterlocked.InterlockedInitialize(ref _lazyParameters, this.MakeParameters()); } return _lazyParameters; } diff --git a/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs index b569a4e285e8b..cde90e9074875 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs @@ -154,7 +154,7 @@ public override ImmutableArray Parameters { if (_lazyParameters.IsDefault) { - ImmutableInterlocked.InterlockedCompareExchange(ref _lazyParameters, this.RetargetParameters(), default(ImmutableArray)); + ImmutableInterlocked.InterlockedInitialize(ref _lazyParameters, this.RetargetParameters()); } return _lazyParameters; diff --git a/src/Compilers/CSharp/Portable/Symbols/SubstitutedMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/SubstitutedMethodSymbol.cs index ffbb3ca75a716..de2b9d597ad0a 100644 --- a/src/Compilers/CSharp/Portable/Symbols/SubstitutedMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/SubstitutedMethodSymbol.cs @@ -252,7 +252,7 @@ public sealed override ImmutableArray Parameters { if (_lazyParameters.IsDefault) { - ImmutableInterlocked.InterlockedCompareExchange(ref _lazyParameters, SubstituteParameters(), default(ImmutableArray)); + ImmutableInterlocked.InterlockedInitialize(ref _lazyParameters, SubstituteParameters()); } return _lazyParameters; From b17b2193bb51a36bee1ab7bdd617e7e5266a8b92 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 17 Jun 2021 11:03:46 -0700 Subject: [PATCH 3/4] Use ImmutableArray.Create for clarity --- .../Synthesized/SynthesizedIntrinsicOperatorSymbol.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs index e97ac78ab7771..047fcb25a065d 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs @@ -42,8 +42,8 @@ public SynthesizedIntrinsicOperatorSymbol(TypeSymbol leftType, string name, Type Debug.Assert((leftType.IsDynamic() || rightType.IsDynamic()) == returnType.IsDynamic()); Debug.Assert(_containingType.IsDynamic() == returnType.IsDynamic()); - _parameters = (new ParameterSymbol[] {new SynthesizedOperatorParameterSymbol(this, leftType, 0, "left"), - new SynthesizedOperatorParameterSymbol(this, rightType, 1, "right")}).AsImmutableOrNull(); + _parameters = ImmutableArray.Create(new SynthesizedOperatorParameterSymbol(this, leftType, 0, "left"), + new SynthesizedOperatorParameterSymbol(this, rightType, 1, "right")); _isCheckedBuiltin = isCheckedBuiltin; } @@ -52,7 +52,7 @@ public SynthesizedIntrinsicOperatorSymbol(TypeSymbol container, string name, Typ _containingType = container; _name = name; _returnType = returnType; - _parameters = (new ParameterSymbol[] { new SynthesizedOperatorParameterSymbol(this, container, 0, "value") }).AsImmutableOrNull(); + _parameters = ImmutableArray.Create(new SynthesizedOperatorParameterSymbol(this, container, 0, "value")); _isCheckedBuiltin = isCheckedBuiltin; } From 856df7f74517aa9afcc1db69988bb5999edae7fa Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 17 Jun 2021 11:06:44 -0700 Subject: [PATCH 4/4] Assert non-null return values from MethodSymbol.Parameters --- .../Portable/Symbols/Source/SourceDelegateMethodSymbol.cs | 3 ++- .../Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceDelegateMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceDelegateMethodSymbol.cs index b4eab31db8f74..d1282fe90152f 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceDelegateMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceDelegateMethodSymbol.cs @@ -126,7 +126,8 @@ public sealed override ImmutableArray Parameters { get { - return _parameters; + Debug.Assert(!_parameters.IsDefault, $"Expected {nameof(InitializeParameters)} prior to accessing this property."); + return _parameters.NullToEmpty(); } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs index 8f7155d60dadd..4b66d9d66ebfe 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs @@ -167,7 +167,8 @@ public override ImmutableArray Parameters { get { - if (_parameters.IsEmpty) + Debug.Assert(!_parameters.IsDefault, $"Expected {nameof(SetParameters)} prior to accessing this property."); + if (_parameters.IsDefault) { return ImmutableArray.Empty; }