Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collection expressions: synthesize single-element list type for IEnumerable<T>, IReadOnlyCollection<T>, IReadOnlyList<T> #71147

Merged
merged 29 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ private void CompileSynthesizedMethods(PrivateImplementationDetails privateImplC

var compilationState = new TypeCompilationState(null, _compilation, _moduleBeingBuiltOpt);
var context = new EmitContext(_moduleBeingBuiltOpt, null, diagnostics.DiagnosticBag, metadataOnly: false, includePrivateMembers: true);
foreach (Cci.IMethodDefinition definition in privateImplClass.GetMethods(context).Concat(privateImplClass.GetTopLevelTypeMethods(context)))
foreach (Cci.IMethodDefinition definition in privateImplClass.GetMethods(context).Concat(privateImplClass.GetTopLevelAndNestedTypeMethods(context)))
{
var method = (MethodSymbol)definition.GetInternalSymbol();
Debug.Assert(method.SynthesizesLoweredBoundBody);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1916,19 +1916,19 @@ internal NamedTypeSymbol EnsureInlineArrayTypeExists(SyntaxNode syntaxNode, Synt
return (NamedTypeSymbol)typeAdapter.GetInternalSymbol()!;
}

internal NamedTypeSymbol EnsureReadOnlyListTypeExists(SyntaxNode syntaxNode, bool hasKnownLength, DiagnosticBag diagnostics)
internal NamedTypeSymbol EnsureReadOnlyListTypeExists(SyntaxNode syntaxNode, SynthesizedReadOnlyListKind kind, DiagnosticBag diagnostics)
{
Debug.Assert(syntaxNode is { });
Debug.Assert(diagnostics is { });

string typeName = GeneratedNames.MakeSynthesizedReadOnlyListName(hasKnownLength, CurrentGenerationOrdinal);
string typeName = GeneratedNames.MakeSynthesizedReadOnlyListName(kind, CurrentGenerationOrdinal);
var privateImplClass = GetPrivateImplClass(syntaxNode, diagnostics).PrivateImplementationDetails;
var typeAdapter = privateImplClass.GetSynthesizedType(typeName);
NamedTypeSymbol typeSymbol;

if (typeAdapter is null)
{
typeSymbol = SynthesizedReadOnlyListTypeSymbol.Create(SourceModule, typeName, hasKnownLength);
typeSymbol = SynthesizedReadOnlyListTypeSymbol.Create(SourceModule, typeName, kind);
privateImplClass.TryAddSynthesizedType(typeSymbol.GetCciAdapter());
typeAdapter = privateImplClass.GetSynthesizedType(typeName)!;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,32 +312,36 @@ SpecialType.System_Collections_Generic_IReadOnlyCollection_T or
int numberIncludingLastSpread;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you didn't name this variable, but... number of what? We need to rename this for clarity. If you want to do it now, go for it, if not we can do it in a follow up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried lastSpreadIndex here 57dc299 but that's more than a rename. I'll leave it out of this PR.

bool useKnownLength = ShouldUseKnownLength(node, out numberIncludingLastSpread);

if (numberIncludingLastSpread == 0 && elements.Length == 0)
if (elements.Length == 0)
{
Debug.Assert(numberIncludingLastSpread == 0);
// arrayOrList = Array.Empty<ElementType>();
arrayOrList = CreateEmptyArray(syntax, ArrayTypeSymbol.CreateSZArray(_compilation.Assembly, elementType));
}
else
{
var typeArgs = ImmutableArray.Create(elementType);
var synthesizedType = _factory.ModuleBuilderOpt.EnsureReadOnlyListTypeExists(syntax, hasKnownLength: useKnownLength, _diagnostics.DiagnosticBag).Construct(typeArgs);
var kind = useKnownLength
? numberIncludingLastSpread == 0 && elements.Length == 1 && SynthesizedReadOnlyListTypeSymbol.CanCreateSingleElement(_compilation)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CanCreateSingleElement

This runs for every single-element collection expr unlike SynthesizedReadOnlyListTypeSymbol.Create. Not sure if this matters but I think we can detect this in EnsureReadOnlyListTypeExists and downgrade instead of checking each time, although that would only optimize the happy path.

? SynthesizedReadOnlyListKind.SingleElement
: SynthesizedReadOnlyListKind.Array
: SynthesizedReadOnlyListKind.List;
var synthesizedType = _factory.ModuleBuilderOpt.EnsureReadOnlyListTypeExists(syntax, kind: kind, _diagnostics.DiagnosticBag).Construct(typeArgs);
if (synthesizedType.IsErrorType())
{
return BadExpression(node);
}

BoundExpression fieldValue;
if (useKnownLength)
BoundExpression fieldValue = kind switch
{
// fieldValue = e1;
SynthesizedReadOnlyListKind.SingleElement => this.VisitExpression((BoundExpression)elements.Single()),
// fieldValue = new ElementType[] { e1, ..., eN };
var arrayType = ArrayTypeSymbol.CreateSZArray(_compilation.Assembly, elementType);
fieldValue = CreateAndPopulateArray(node, arrayType);
}
else
{
SynthesizedReadOnlyListKind.Array => CreateAndPopulateArray(node, ArrayTypeSymbol.CreateSZArray(_compilation.Assembly, elementType)),
// fieldValue = new List<ElementType> { e1, ..., eN };
fieldValue = CreateAndPopulateList(node, elementType, elements);
}
SynthesizedReadOnlyListKind.List => CreateAndPopulateList(node, elementType, elements),
var v => throw ExceptionUtilities.UnexpectedValue(v)
};

// arrayOrList = new <>z__ReadOnlyList<ElementType>(fieldValue);
arrayOrList = new BoundObjectCreationExpression(syntax, synthesizedType.Constructors.Single(), fieldValue) { WasCompilerGenerated = true };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,16 @@ internal static string MakeSynthesizedInlineArrayName(int arrayLength, int gener
return (generation > 0) ? name + GeneratedNameConstants.GenerationSeparator + generation : name;
}

internal static string MakeSynthesizedReadOnlyListName(bool hasKnownLength, int generation)
internal static string MakeSynthesizedReadOnlyListName(SynthesizedReadOnlyListKind kind, int generation)
{
Debug.Assert((char)GeneratedNameKind.ReadOnlyListType == 'z');
string name = hasKnownLength ? "<>z__ReadOnlyArray" : "<>z__ReadOnlyList";
string name = kind switch
{
SynthesizedReadOnlyListKind.Array => "<>z__ReadOnlyArray",
SynthesizedReadOnlyListKind.List => "<>z__ReadOnlyList",
SynthesizedReadOnlyListKind.SingleElement => "<>z__ReadOnlySingleElementList",
var v => throw ExceptionUtilities.UnexpectedValue(v)
};

// Synthesized list types need to have unique name across generations because they are not reused.
return (generation > 0) ? name + CommonGeneratedNames.GenerationSeparator + generation : name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal sealed class SynthesizedReadOnlyListConstructor : SynthesizedInstanceConstructor
{
internal SynthesizedReadOnlyListConstructor(SynthesizedReadOnlyListTypeSymbol containingType, TypeSymbol parameterType) : base(containingType)
internal SynthesizedReadOnlyListConstructor(SynthesizedReadOnlyListTypeSymbol containingType, TypeSymbol parameterType, string parameterName) : base(containingType)
{
Parameters = ImmutableArray.Create(
SynthesizedParameterSymbol.Create(this, TypeWithAnnotations.Create(parameterType), ordinal: 0, RefKind.None, "items"));
SynthesizedParameterSymbol.Create(this, TypeWithAnnotations.Create(parameterType), ordinal: 0, RefKind.None, parameterName));
}

public override ImmutableArray<ParameterSymbol> Parameters { get; }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Linq;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal sealed class SynthesizedReadOnlyListEnumeratorConstructor : SynthesizedInstanceConstructor
{
internal SynthesizedReadOnlyListEnumeratorConstructor(SynthesizedReadOnlyListEnumeratorTypeSymbol containingType, TypeSymbol parameterType) : base(containingType)
{
Parameters = ImmutableArray.Create(
SynthesizedParameterSymbol.Create(this, TypeWithAnnotations.Create(parameterType), ordinal: 0, RefKind.None, "item"));
}

public override ImmutableArray<ParameterSymbol> Parameters { get; }

internal override bool SynthesizesLoweredBoundBody => true;

internal override void GenerateMethodBody(TypeCompilationState compilationState, BindingDiagnosticBag diagnostics)
{
SyntheticBoundNodeFactory f = new SyntheticBoundNodeFactory(this, this.GetNonNullSyntaxNode(), compilationState, diagnostics);
f.CurrentFunction = this;

try
{
var baseConstructor = ContainingType.BaseTypeNoUseSiteDiagnostics.InstanceConstructors.Single();
var field = ContainingType.GetFieldsToEmit().First();
var parameter = Parameters.Single();

var block = f.Block(
// object..ctor();
f.ExpressionStatement(f.Call(f.This(), baseConstructor)),
// _item = item;
f.Assignment(f.Field(f.This(), field), f.Parameter(parameter)),
// return;
f.Return());
f.CloseMethod(block);
}
catch (SyntheticBoundNodeFactory.MissingPredefinedMember ex)
{
diagnostics.Add(ex.Diagnostic);
f.CloseMethod(f.ThrowNull());
}
}
}
}
Loading
Loading