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

Integrate Lambda Default Parameters into Synthesized Delegate Types and Lowering Pass #63293

Merged
merged 25 commits into from
Aug 16, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
99210b0
WIP add default parameter information to synthesized delegate types
adamperlin Aug 4, 2022
f846cf5
Update closure codegen tests file to see codegen in current state
adamperlin Aug 5, 2022
4e8dddc
Finish adding default parameters to synthesized delegate types and up…
adamperlin Aug 9, 2022
020b563
Add additional unit tests
adamperlin Aug 9, 2022
ef79ab7
Add additional test case
adamperlin Aug 9, 2022
1374707
Merge remote-tracking branch 'roslyn-origin/features/lambda-default-p…
adamperlin Aug 9, 2022
0f757da
Fix Roslyn build
adamperlin Aug 10, 2022
c1d4482
Address review feedback and fix unit tests
adamperlin Aug 11, 2022
26375e1
Remove default value from SynthesizedParameterSymbol constructor to f…
adamperlin Aug 11, 2022
13d4974
Revert "Remove default value from SynthesizedParameterSymbol construc…
adamperlin Aug 11, 2022
8ba94ee
Use SynthesizedComplexParameterSymbol instead of SynthesizedParameter…
adamperlin Aug 11, 2022
ccaddb8
Remove unused imports
adamperlin Aug 11, 2022
0b88e79
Remove all uses of CheckNames
adamperlin Aug 12, 2022
d484d30
Move default value from base into SourceComplexParameterSymbol and ad…
adamperlin Aug 12, 2022
8254fca
Add PROTOTYPE comment about Caller... attributes
adamperlin Aug 12, 2022
9d113cf
Address review feedback
adamperlin Aug 15, 2022
91f93b4
Add assert in constructor to clarify dependence between default value…
adamperlin Aug 15, 2022
7632698
Remove line in src/Compilers/CSharp/Portable/Lowering/SynthesizedMeth…
adamperlin Aug 15, 2022
ab22426
Put CallerArgumentExpression attribute test under CoreClrOnly to fix …
adamperlin Aug 15, 2022
20a3e12
Merge branch 'main' of https://github.com/adamperlin/roslyn
adamperlin Aug 15, 2022
97fbff9
Switch from ConditionalFact to condition in test body
adamperlin Aug 15, 2022
c45d77c
Add expected output to [CallerArgumentExpression] attribute test
adamperlin Aug 16, 2022
2247ab4
Add prototype comment
adamperlin Aug 16, 2022
fff9e9a
Add prototype comment to add different language version tests
adamperlin Aug 16, 2022
b0b2d65
Add additional prototype comment
adamperlin Aug 16, 2022
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
9 changes: 7 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8939,10 +8939,12 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
TypeWithAnnotations returnType,
ImmutableArray<RefKind> parameterRefKinds,
ImmutableArray<DeclarationScope> parameterScopes,
ImmutableArray<TypeWithAnnotations> parameterTypes)
ImmutableArray<TypeWithAnnotations> parameterTypes,
ImmutableArray<ConstantValue?> parameterDefaultValues = default)
adamperlin marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(ContainingMemberOrLambda is { });
Debug.Assert(parameterRefKinds.IsDefault || parameterRefKinds.Length == parameterTypes.Length);
adamperlin marked this conversation as resolved.
Show resolved Hide resolved
Debug.Assert(parameterDefaultValues.IsDefault || parameterDefaultValues.Length == parameterTypes.Length);
Debug.Assert(returnType.Type is { }); // Expecting System.Void rather than null return type.

bool returnsVoid = returnType.Type.IsVoidType();
Expand All @@ -8962,6 +8964,7 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)

// Use System.Action<...> or System.Func<...> if possible.
if (returnRefKind == RefKind.None &&
parameterDefaultValues.IsDefault &&
(parameterRefKinds.IsDefault || parameterRefKinds.All(refKind => refKind == RefKind.None)) &&
(parameterScopes.IsDefault || parameterScopes.All(scope => scope == DeclarationScope.Unscoped)))
{
Expand Down Expand Up @@ -8996,7 +8999,9 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
location,
parameterTypes[i],
parameterRefKinds.IsDefault ? RefKind.None : parameterRefKinds[i],
parameterScopes.IsDefault ? DeclarationScope.Unscoped : parameterScopes[i]));
parameterScopes.IsDefault ? DeclarationScope.Unscoped : parameterScopes[i],
parameterDefaultValues.IsDefault ? null : parameterDefaultValues[i])
);
}
fieldsBuilder.Add(new AnonymousTypeField(name: "", location, returnType, returnRefKind, DeclarationScope.Unscoped));

Expand Down
11 changes: 10 additions & 1 deletion src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,14 @@ private static TypeWithAnnotations DelegateReturnTypeWithAnnotations(MethodSymbo
}
}

// Account for the situation where we have trailing required parameters AFTER an optional one, i.e., (int opt1 = 3, T1 req1, ..., TN reqN) => {...}
// so we will be missing null default values for req1, ..., reqN when exiting the previous loop and the length of the default value builder will
// be too short.
if (parameterDefaultValueBuilder is not null)
{
parameterDefaultValueBuilder.AddMany(null, lambdaSymbol.Parameters.Length - parameterDefaultValueBuilder.Count);
}

var parameterDefaultValues = parameterDefaultValueBuilder?.ToImmutableAndFree() ?? default;

if (!HasExplicitReturnType(out var returnRefKind, out var returnType))
Expand Down Expand Up @@ -702,7 +710,8 @@ private static TypeWithAnnotations DelegateReturnTypeWithAnnotations(MethodSymbo
returnType,
parameterRefKinds,
parameterScopes,
parameterTypes);
parameterTypes,
parameterDefaultValues);
}

private BoundLambda ReallyBind(NamedTypeSymbol delegateType, bool inExpressionTree)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ private ImmutableArray<ParameterSymbol> MakeParameters()
p.RefKind,
p.Name,
p.DeclaredScope,
p.ExplicitDefaultConstantValue,
// the synthesized parameter doesn't need to have the same ref custom modifiers as the base
refCustomModifiers: default,
inheritAttributes ? p as SourceComplexParameterSymbolBase : null));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ internal bool Equals(AnonymousTypeDescriptor other, TypeCompareKind comparison)
return Fields.SequenceEqual(
other.Fields,
comparison,
static (x, y, comparison) => x.TypeWithAnnotations.Equals(y.TypeWithAnnotations, comparison) && x.RefKind == y.RefKind && x.Scope == y.Scope);
static (x, y, comparison) => x.TypeWithAnnotations.Equals(y.TypeWithAnnotations, comparison) && x.RefKind == y.RefKind && x.Scope == y.Scope && x.DefaultValue == y.DefaultValue);
}

/// <summary>
Expand All @@ -103,7 +103,7 @@ internal AnonymousTypeDescriptor WithNewFieldsTypes(ImmutableArray<TypeWithAnnot
Debug.Assert(!newFieldTypes.IsDefault);
Debug.Assert(newFieldTypes.Length == this.Fields.Length);

var newFields = Fields.ZipAsArray(newFieldTypes, static (field, type) => new AnonymousTypeField(field.Name, field.Location, type, field.RefKind, field.Scope));
var newFields = Fields.ZipAsArray(newFieldTypes, static (field, type) => new AnonymousTypeField(field.Name, field.Location, type, field.RefKind, field.Scope, field.DefaultValue));
return new AnonymousTypeDescriptor(newFields, this.Location);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,19 @@ internal readonly struct AnonymousTypeField

public readonly DeclarationScope Scope;

public readonly ConstantValue? DefaultValue;

/// <summary>Anonymous type field type</summary>
public TypeSymbol Type => TypeWithAnnotations.Type;

public AnonymousTypeField(string name, Location location, TypeWithAnnotations typeWithAnnotations, RefKind refKind, DeclarationScope scope)
public AnonymousTypeField(string name, Location location, TypeWithAnnotations typeWithAnnotations, RefKind refKind, DeclarationScope scope, ConstantValue? defaultValue = null)
adamperlin marked this conversation as resolved.
Show resolved Hide resolved
{
this.Name = name;
this.Location = location;
this.TypeWithAnnotations = typeWithAnnotations;
this.RefKind = refKind;
this.Scope = scope;
this.DefaultValue = defaultValue;
}

[Conditional("DEBUG")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ static bool hasDefaultScope(AnonymousTypeField field) =>
static bool isValidTypeArgument(AnonymousTypeField field)
{
return hasDefaultScope(field) &&
field.DefaultValue is null &&
field.Type is { } type &&
!type.IsPointerOrFunctionPointer() &&
!type.IsRestrictedType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ private ImmutableArray<Symbol> CreateMembers()
var constructor = new SynthesizedDelegateConstructor(this, Manager.System_Object, Manager.System_IntPtr);
var fields = TypeDescriptor.Fields;
int parameterCount = fields.Length - 1;
var parameters = ArrayBuilder<(TypeWithAnnotations, RefKind, DeclarationScope)>.GetInstance(parameterCount);
var parameters = ArrayBuilder<(TypeWithAnnotations, RefKind, DeclarationScope, ConstantValue?)>.GetInstance(parameterCount);
for (int i = 0; i < parameterCount; i++)
{
var field = fields[i];
parameters.Add((field.TypeWithAnnotations, field.RefKind, field.Scope));
parameters.Add((field.TypeWithAnnotations, field.RefKind, field.Scope, field.DefaultValue));
}
var returnField = fields.Last();
var invokeMethod = new SynthesizedDelegateInvokeMethod(this, parameters, returnField.TypeWithAnnotations, returnField.RefKind);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ static SynthesizedDelegateInvokeMethod createInvokeMethod(AnonymousDelegateTempl
var typeParams = containingType.TypeParameters;

int parameterCount = typeParams.Length - (voidReturnTypeOpt is null ? 1 : 0);
var parameters = ArrayBuilder<(TypeWithAnnotations, RefKind, DeclarationScope)>.GetInstance(parameterCount);
var parameters = ArrayBuilder<(TypeWithAnnotations, RefKind, DeclarationScope, ConstantValue?)>.GetInstance(parameterCount);
for (int i = 0; i < parameterCount; i++)
{
parameters.Add((TypeWithAnnotations.Create(typeParams[i]), refKinds.IsNull ? RefKind.None : refKinds[i], DeclarationScope.Unscoped));
parameters.Add((TypeWithAnnotations.Create(typeParams[i]), refKinds.IsNull ? RefKind.None : refKinds[i], DeclarationScope.Unscoped, null));
}

// if we are given Void type the method returns Void, otherwise its return type is the last type parameter of the delegate:
Expand Down Expand Up @@ -127,11 +127,11 @@ static SynthesizedDelegateInvokeMethod createInvokeMethod(
TypeMap typeMap)
{
var parameterCount = fields.Length - 1;
var parameters = ArrayBuilder<(TypeWithAnnotations, RefKind, DeclarationScope)>.GetInstance(parameterCount);
var parameters = ArrayBuilder<(TypeWithAnnotations, RefKind, DeclarationScope, ConstantValue?)>.GetInstance(parameterCount);
for (int i = 0; i < parameterCount; i++)
{
var field = fields[i];
parameters.Add((typeMap.SubstituteType(field.Type), field.RefKind, field.Scope));
parameters.Add((typeMap.SubstituteType(field.Type), field.RefKind, field.Scope, field.DefaultValue));
}

var returnParameter = fields[^1];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ public LambdaParameterSymbol(

public override bool IsDiscard { get; }

internal override bool IsMetadataOptional
{
get { return false; }
}

public override bool IsParams
{
get { return false; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ internal sealed class SynthesizedDelegateInvokeMethod : SynthesizedInstanceMetho
{
private readonly NamedTypeSymbol _containingType;

internal SynthesizedDelegateInvokeMethod(NamedTypeSymbol containingType, ArrayBuilder<(TypeWithAnnotations Type, RefKind RefKind, DeclarationScope Scope)> parameterDescriptions, TypeWithAnnotations returnType, RefKind refKind)
internal SynthesizedDelegateInvokeMethod(NamedTypeSymbol containingType, ArrayBuilder<(TypeWithAnnotations Type, RefKind RefKind, DeclarationScope Scope, ConstantValue? DefaultValue)> parameterDescriptions, TypeWithAnnotations returnType, RefKind refKind)
{
_containingType = containingType;

Parameters = parameterDescriptions.SelectAsArrayWithIndex((p, i, m) => SynthesizedParameterSymbol.Create(m, p.Type, i, p.RefKind, scope: p.Scope), this);
Parameters = parameterDescriptions.SelectAsArrayWithIndex((p, i, m) => SynthesizedParameterSymbol.Create(m, p.Type, i, p.RefKind, scope: p.Scope, defaultValue: p.DefaultValue), this);
ReturnTypeWithAnnotations = returnType;
RefKind = refKind;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ public SynthesizedOperatorParameterSymbol(
TypeSymbol type,
int ordinal,
string name
) : base(container, TypeWithAnnotations.Create(type), ordinal, RefKind.None, DeclarationScope.Unscoped, name)
) : base(container, TypeWithAnnotations.Create(type), ordinal, RefKind.None, DeclarationScope.Unscoped, defaultValue: null, name)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ internal abstract class SynthesizedParameterSymbolBase : ParameterSymbol
private readonly string _name;
private readonly RefKind _refKind;
private readonly DeclarationScope _scope;
private readonly ConstantValue? _defaultValue;
adamperlin marked this conversation as resolved.
Show resolved Hide resolved

public SynthesizedParameterSymbolBase(
MethodSymbol? container,
TypeWithAnnotations type,
int ordinal,
RefKind refKind,
DeclarationScope scope,
ConstantValue? defaultValue,
string name)
{
Debug.Assert(type.HasType);
Expand All @@ -39,6 +41,7 @@ public SynthesizedParameterSymbolBase(
_ordinal = ordinal;
_refKind = refKind;
_scope = scope;
_defaultValue = defaultValue;
_name = name;
}

Expand Down Expand Up @@ -71,7 +74,7 @@ public override bool IsParams

internal override bool IsMetadataOptional
{
get { return false; }
get { return ExplicitDefaultConstantValue != null; }
}

public override bool IsImplicitlyDeclared
Expand All @@ -81,7 +84,7 @@ public override bool IsImplicitlyDeclared

internal override ConstantValue? ExplicitDefaultConstantValue
{
get { return null; }
get { return _defaultValue; }
}

internal override bool IsIDispatchConstant
Expand All @@ -96,17 +99,17 @@ internal override bool IsIUnknownConstant

internal override bool IsCallerLineNumber
{
get { throw ExceptionUtilities.Unreachable; }
get { return false; }
}

internal override bool IsCallerFilePath
{
get { throw ExceptionUtilities.Unreachable; }
get { return false; }
}

internal override bool IsCallerMemberName
{
get { throw ExceptionUtilities.Unreachable; }
get { return false; }
}
adamperlin marked this conversation as resolved.
Show resolved Hide resolved

internal override int CallerArgumentExpressionParameterIndex
Expand Down Expand Up @@ -202,7 +205,7 @@ private SynthesizedParameterSymbol(
RefKind refKind,
DeclarationScope scope,
string name)
: base(container, type, ordinal, refKind, scope, name)
: base(container, type, ordinal, refKind, scope, defaultValue: null, name)
{
}

Expand All @@ -213,10 +216,11 @@ public static ParameterSymbol Create(
RefKind refKind,
string name = "",
DeclarationScope scope = DeclarationScope.Unscoped,
ConstantValue? defaultValue = null,
ImmutableArray<CustomModifier> refCustomModifiers = default,
SourceComplexParameterSymbolBase? baseParameterForAttributes = null)
{
if (refCustomModifiers.IsDefaultOrEmpty && baseParameterForAttributes is null)
if (refCustomModifiers.IsDefaultOrEmpty && baseParameterForAttributes is null && defaultValue is null)
{
return new SynthesizedParameterSymbol(container, type, ordinal, refKind, scope, name);
}
Expand All @@ -227,6 +231,7 @@ public static ParameterSymbol Create(
ordinal,
refKind,
scope,
defaultValue,
name,
refCustomModifiers.NullToEmpty(),
baseParameterForAttributes);
Expand All @@ -245,7 +250,6 @@ internal static ImmutableArray<ParameterSymbol> DeriveParameters(MethodSymbol so

foreach (var oldParam in sourceMethod.Parameters)
{
Debug.Assert(!(oldParam is SynthesizedComplexParameterSymbol));
//same properties as the old one, just change the owner
builder.Add(Create(
destinationMethod,
Expand All @@ -254,6 +258,7 @@ internal static ImmutableArray<ParameterSymbol> DeriveParameters(MethodSymbol so
oldParam.RefKind,
oldParam.Name,
oldParam.DeclaredScope,
oldParam.ExplicitDefaultConstantValue,
oldParam.RefCustomModifiers,
baseParameterForAttributes: null));
}
Expand Down Expand Up @@ -285,13 +290,14 @@ public SynthesizedComplexParameterSymbol(
int ordinal,
RefKind refKind,
DeclarationScope scope,
ConstantValue? defaultValue,
string name,
ImmutableArray<CustomModifier> refCustomModifiers,
SourceComplexParameterSymbolBase? baseParameterForAttributes)
: base(container, type, ordinal, refKind, scope, name)
: base(container, type, ordinal, refKind, scope, defaultValue, name)
{
Debug.Assert(!refCustomModifiers.IsDefault);
Debug.Assert(!refCustomModifiers.IsEmpty || baseParameterForAttributes is object);
Debug.Assert(!refCustomModifiers.IsEmpty || baseParameterForAttributes is object || defaultValue is not null);

_refCustomModifiers = refCustomModifiers;
_baseParameterForAttributes = baseParameterForAttributes;
Expand All @@ -311,9 +317,9 @@ public override ImmutableArray<CSharpAttributeData> GetAttributes()

internal override MarshalPseudoCustomAttributeData? MarshallingInformation => _baseParameterForAttributes?.MarshallingInformation;

internal override bool IsMetadataOptional => _baseParameterForAttributes?.IsMetadataOptional == true;
internal override bool IsMetadataOptional => _baseParameterForAttributes is not null ? _baseParameterForAttributes.IsMetadataOptional == true : base.IsMetadataOptional;
adamperlin marked this conversation as resolved.
Show resolved Hide resolved

internal override ConstantValue? ExplicitDefaultConstantValue => _baseParameterForAttributes?.ExplicitDefaultConstantValue;
internal override ConstantValue? ExplicitDefaultConstantValue => _baseParameterForAttributes?.ExplicitDefaultConstantValue ?? base.ExplicitDefaultConstantValue;

internal override FlowAnalysisAnnotations FlowAnalysisAnnotations
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.Test.Utilities;
using Newtonsoft.Json.Linq;
adamperlin marked this conversation as resolved.
Show resolved Hide resolved
using Roslyn.Test.Utilities;
using Xunit;
using static Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen.Instruction;
Expand Down Expand Up @@ -2116,7 +2117,7 @@ void verifyMembersAndInterfaces(ModuleSymbol module)
"C.<M>d__0..ctor(System.Int32 <>1__state)",
"void C.<M>d__0.MoveNext()",
"void C.<M>d__0.SetStateMachine(System.Runtime.CompilerServices.IAsyncStateMachine stateMachine)",
"System.Collections.Generic.IAsyncEnumerator<System.Int32> C.<M>d__0.System.Collections.Generic.IAsyncEnumerable<System.Int32>.GetAsyncEnumerator(System.Threading.CancellationToken token)",
"System.Collections.Generic.IAsyncEnumerator<System.Int32> C.<M>d__0.System.Collections.Generic.IAsyncEnumerable<System.Int32>.GetAsyncEnumerator([System.Threading.CancellationToken token = default(System.Threading.CancellationToken)])",
"System.Threading.Tasks.ValueTask<System.Boolean> C.<M>d__0.System.Collections.Generic.IAsyncEnumerator<System.Int32>.MoveNextAsync()",
"System.Int32 C.<M>d__0.System.Collections.Generic.IAsyncEnumerator<System.Int32>.Current.get",
"System.Boolean C.<M>d__0.System.Threading.Tasks.Sources.IValueTaskSource<System.Boolean>.GetResult(System.Int16 token)",
Expand Down
Loading