Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamperlin
adamperlin committed Aug 15, 2022
1 parent 8254fca commit 9d113cf
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 27 deletions.
6 changes: 4 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8853,7 +8853,9 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
var parameterScopes = parameters.Any(p => p.EffectiveScope != DeclarationScope.Unscoped) ?
parameters.SelectAsArray(p => p.EffectiveScope) :
default;
return GetMethodGroupOrLambdaDelegateType(node.Syntax, method.RefKind, method.ReturnTypeWithAnnotations, method.ParameterRefKinds, parameterScopes, method.ParameterTypesWithAnnotations);

// PROTOTYPE: bind default values and pass them through here to allow support for default parameters in method groups
return GetMethodGroupOrLambdaDelegateType(node.Syntax, method.RefKind, method.ReturnTypeWithAnnotations, method.ParameterRefKinds, parameterScopes, method.ParameterTypesWithAnnotations, parameterDefaultValues: default);
}

/// <summary>
Expand Down Expand Up @@ -8940,7 +8942,7 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
ImmutableArray<RefKind> parameterRefKinds,
ImmutableArray<DeclarationScope> parameterScopes,
ImmutableArray<TypeWithAnnotations> parameterTypes,
ImmutableArray<ConstantValue?> parameterDefaultValues = default)
ImmutableArray<ConstantValue?> parameterDefaultValues)
{
Debug.Assert(ContainingMemberOrLambda is { });
Debug.Assert(parameterRefKinds.IsDefault || parameterRefKinds.Length == parameterTypes.Length);
Expand Down
22 changes: 6 additions & 16 deletions src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,28 +651,18 @@ private static TypeWithAnnotations DelegateReturnTypeWithAnnotations(MethodSymbo
{
var param = lambdaSymbol.Parameters[i];
var constVal = param.ExplicitDefaultConstantValue;
if (constVal != null)
if (constVal != null && parameterDefaultValueBuilder == null)
{
if (parameterDefaultValueBuilder == null)
{
parameterDefaultValueBuilder = ArrayBuilder<ConstantValue?>.GetInstance(ParameterCount);

// front fill the array with nulls because all the parameters before this one must have been non-default.
parameterDefaultValueBuilder.AddMany(null, i);
}
parameterDefaultValueBuilder = ArrayBuilder<ConstantValue?>.GetInstance(ParameterCount);
parameterDefaultValueBuilder.AddMany(null, i);
}

if (parameterDefaultValueBuilder != null)
{
parameterDefaultValueBuilder.Add(constVal);

This comment has been minimized.

Sign in to view

Copy link
@Youssef1313

Youssef1313 Aug 15, 2022 •
edited
Loading

Member

Looks like this is not guarded by constVal null check now. Is that intentional? Ignore this. I commented too fast, it's making sense now.
}
}

// 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
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Lowering/SynthesizedMethodBaseSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ protected SynthesizedMethodBaseSymbol(NamedTypeSymbol containingType,
}

protected void AssignTypeMapAndTypeParameters(TypeMap typeMap, ImmutableArray<TypeParameterSymbol> typeParameters)

{
Debug.Assert(typeMap != null);
Debug.Assert(this.TypeMap == null);
Expand Down
10 changes: 6 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedParameterSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,16 @@ internal override bool IsMetadataOptional
get { return ExplicitDefaultConstantValue != null; }
}

internal override ConstantValue? ExplicitDefaultConstantValue => null;

public override bool IsImplicitlyDeclared
{
get { return true; }
}

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

internal override bool IsIDispatchConstant
{
get { throw ExceptionUtilities.Unreachable; }
Expand Down Expand Up @@ -313,7 +316,7 @@ public override ImmutableArray<CSharpAttributeData> GetAttributes()

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

internal override bool IsMetadataOptional => _baseParameterForAttributes is not null ? _baseParameterForAttributes.IsMetadataOptional == true : base.IsMetadataOptional;
internal override bool IsMetadataOptional => _baseParameterForAttributes?.IsMetadataOptional ?? base.IsMetadataOptional;

internal override bool IsCallerLineNumber
{
Expand Down Expand Up @@ -349,6 +352,5 @@ internal override ImmutableHashSet<string> NotNullIfParameterNotNull
return base.NotNullIfParameterNotNull;
}
}

}
}
11 changes: 6 additions & 5 deletions src/Compilers/CSharp/Test/Semantic/Semantics/DelegateTypeTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12211,16 +12211,17 @@ class Program
{
public static void Main()
{
var lam = ([CallerMemberName] string member = "member", [CallerFilePath] string filePath = "file",
[CallerLineNumber] int lineNumber = 0) => Console.WriteLine($"{filePath}::{member}:{lineNumber}");
lam();
var lam = (int arg, [CallerMemberName] string member = "member", [CallerFilePath] string filePath = "file",
[CallerLineNumber] int lineNumber = 0,
[CallerArgumentExpression("arg")] string argExpression = "callerArgExpression") => Console.WriteLine($"{filePath}::{member}({argExpression}):{lineNumber}");
lam(3);
}
}
""";
// PROTOTYPE: Do we want to allow [Caller{MemberName, LineNumber, FilePath}] attributes for lambdas since
// PROTOTYPE: Do we want to allow [Caller{MemberName, LineNumber, FilePath, ArgumentExpression}] attributes for lambdas since
// we now have default parameters? The current behavior is to ignore these attributes so that the provided
// default would always be used in these cases.
CompileAndVerify(source, expectedOutput: "file::member:0");
CompileAndVerify(source, targetFramework: TargetFramework.Net60, expectedOutput: "file::member(callerArgExpression):0");
}
}
}

0 comments on commit 9d113cf

Please sign in to comment.