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.

Sign up for GitHub

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

Implement lambda params with modifiers without type name #69273

Closed
wants to merge 22 commits into from
Closed

Implement lambda params with modifiers without type name #69273

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
709f34b
Impl by-ref lambda params without type name
Rekkonnect Jul 28, 2023
f93d99a
Remove unnecessary property
Rekkonnect Jul 29, 2023
b892b36
Add support for scoped ref and params
Rekkonnect Jul 30, 2023
00e96eb
Fix parameter syntax assertions and return
Rekkonnect Jul 30, 2023
9e34cf7
Reduce CS1661 diagnostics
Rekkonnect Jul 30, 2023
f606d54
Add tests for discard parameters
Rekkonnect Jul 30, 2023
5de151d
Support diagnostics for params
Rekkonnect Jul 31, 2023
ec86619
Fix params test
Rekkonnect Jul 31, 2023
b19c11e
Improve tests and assertions
Rekkonnect Jul 31, 2023
8f37c4c
Adjust lambda parameter mismatch
Rekkonnect Jul 31, 2023
310b7b8
Fix tests with ERR_CantConvAnonMethParams
Rekkonnect Aug 1, 2023
d0595bf
Fix lambda parameter parsing tests
Rekkonnect Aug 1, 2023
a3b48a6
Merge branch 'main' into features/lambda-by-ref-params
Rekkonnect Aug 1, 2023
9fc1a1e
Fix ref readonly test diagnostics
Rekkonnect Aug 1, 2023
245bce7
Revert changes to CS1661
Rekkonnect Aug 1, 2023
26a73d8
Merge branch 'main' into features/lambda-by-ref-params
Rekkonnect Aug 5, 2023
468da2b
Update tests based on #69334
Rekkonnect Aug 5, 2023
bc3f5ff
Avoid nullable parameter syntaxes for lambda
Rekkonnect Aug 5, 2023
1cb3f27
Merge branch 'main' into features/lambda-by-ref-params
Rekkonnect Apr 6, 2024
0b386d8
Fix broken test
Rekkonnect Apr 6, 2024
34a34a2
Merge branch 'main' into features/lambda-by-ref-params
Rekkonnect Oct 4, 2024
a201d80
Fix code after merge + apply suggestion
Rekkonnect Oct 4, 2024
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
42 changes: 20 additions & 22 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lambda.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@ private UnboundLambda AnalyzeAnonymousFunction(

var typeSyntax = p.Type;
TypeWithAnnotations type = default;
var refKind = RefKind.None;
var scope = ScopedKind.None;
var refKind = ParameterHelpers.GetModifiers(p.Modifiers, out _, out var paramsKeyword, out _, out var scope);

if (typeSyntax == null)
{
Expand All @@ -177,21 +176,21 @@ private UnboundLambda AnalyzeAnonymousFunction(
else
{
type = BindType(typeSyntax, diagnostics);
ParameterHelpers.CheckParameterModifiers(p, diagnostics, parsingFunctionPointerParams: false,
parsingLambdaParams: !isAnonymousMethod,
parsingAnonymousMethodParams: isAnonymousMethod);
refKind = ParameterHelpers.GetModifiers(p.Modifiers, out _, out var paramsKeyword, out _, out scope);
}

var isLastParameter = parameterCount == parameterSyntaxList.Value.Count;
if (isLastParameter && paramsKeyword.Kind() != SyntaxKind.None)
{
hasParamsArray = true;
ParameterHelpers.CheckParameterModifiers(p, diagnostics, parsingFunctionPointerParams: false,
parsingLambdaParams: !isAnonymousMethod,
parsingAnonymousMethodParams: isAnonymousMethod);

ReportUseSiteDiagnosticForSynthesizedAttribute(Compilation,
WellKnownMember.System_ParamArrayAttribute__ctor,
diagnostics,
paramsKeyword.GetLocation());
}
var isLastParameter = parameterCount == parameterSyntaxList.Value.Count;
if (isLastParameter && paramsKeyword.Kind() != SyntaxKind.None)
{
hasParamsArray = true;

ReportUseSiteDiagnosticForSynthesizedAttribute(Compilation,
WellKnownMember.System_ParamArrayAttribute__ctor,
diagnostics,
paramsKeyword.GetLocation());
}

namesBuilder.Add(p.Identifier.ValueText);
Expand Down Expand Up @@ -352,14 +351,13 @@ private UnboundLambda BindAnonymousFunction(AnonymousFunctionExpressionSyntax sy

var lambda = AnalyzeAnonymousFunction(syntax, diagnostics);
var data = lambda.Data;
if (data.HasExplicitlyTypedParameterList)

int firstDefault = -1;
for (int i = 0; i < lambda.ParameterCount; i++)
{
int firstDefault = -1;
for (int i = 0; i < lambda.ParameterCount; i++)
var paramSyntax = lambda.ParameterSyntax(i);
if (paramSyntax is not null)
Copy link
Member

Choose a reason for hiding this comment

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

Why can paramSyntax be null? If the lambda has non-zero ParameterCount I would expect it to have corrsponding parameter syntaxes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be null in some tests for parameters with modifiers without parameter parentheses (e.g. ref x => x), which is illegal per the spec. Without the underlying changes, now permitting the null parameter syntax, the assertion would fail.

Since the modifiers are not allowed in single-parameter unparenthesized lambda expressions, we need to at least expect the case to arise.

Copy link
Member

Choose a reason for hiding this comment

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

i don't really get it either. i would not expect any sort of lambda to have a nulll parameter syntax.

Copy link
Member

Choose a reason for hiding this comment

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

Since the modifiers are not allowed in single-parameter unparenthesized lambda expressions, we need to at least expect the case to arise.

this sentence doesn't make sense. Just because the modifiers are not legal doesn't mean you get a null parameter syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made changes to preserve this, but kept the removal of the check about having an explicitly typed parameter list. To circumvent breaks, lambda binding data for simple lambda expressions (parameters without parentheses) now contain a singleton separated syntax list of parameter syntaxes.

{
// paramSyntax should not be null here; we should always be operating on an anonymous function which will have parameter information
var paramSyntax = lambda.ParameterSyntax(i);
Debug.Assert(paramSyntax is { });
if (paramSyntax.Default != null && firstDefault == -1)
{
firstDefault = i;
Expand All @@ -370,7 +368,7 @@ private UnboundLambda BindAnonymousFunction(AnonymousFunctionExpressionSyntax sy

// UNDONE: Where do we report improper use of pointer types?
ParameterHelpers.ReportParameterErrors(owner: null, paramSyntax, ordinal: i, lastParameterIndex: lambda.ParameterCount - 1, isParams: isParams, lambda.ParameterTypeWithAnnotations(i),
lambda.RefKind(i), lambda.DeclaredScope(i), containingSymbol: null, thisKeyword: default, paramsKeyword: paramsKeyword, firstDefault, diagnostics);
lambda.RefKind(i), lambda.DeclaredScope(i), containingSymbol: null, thisKeyword: default, paramsKeyword: paramsKeyword, firstDefault, diagnostics);
}
}

Expand Down
63 changes: 45 additions & 18 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2136,31 +2136,18 @@ internal void GenerateAnonymousFunctionConversionError(BindingDiagnosticBag diag

if (reason == LambdaConversionResult.MismatchedParameterType)
{
// Cannot convert {0} to type '{1}' because the parameter types do not match the delegate parameter types
Error(diagnostics, ErrorCode.ERR_CantConvAnonMethParams, syntax, id, targetType);
Debug.Assert(anonymousFunction.ParameterCount == delegateParameters.Length);

bool hasTypeMismatch = false;
bool hasRefKindMismatch = false;

for (int i = 0; i < anonymousFunction.ParameterCount; ++i)
{
var lambdaParameterType = anonymousFunction.ParameterType(i);
if (lambdaParameterType.IsErrorType())
{
continue;
}

var lambdaParameterLocation = anonymousFunction.ParameterLocation(i);
var lambdaRefKind = anonymousFunction.RefKind(i);
var delegateParameterType = delegateParameters[i].Type;
var delegateRefKind = delegateParameters[i].RefKind;

if (!lambdaParameterType.Equals(delegateParameterType, TypeCompareKind.AllIgnoreOptions))
{
SymbolDistinguisher distinguisher = new SymbolDistinguisher(this.Compilation, lambdaParameterType, delegateParameterType);

// Parameter {0} is declared as type '{1}{2}' but should be '{3}{4}'
Error(diagnostics, ErrorCode.ERR_BadParamType, lambdaParameterLocation,
i + 1, lambdaRefKind.ToParameterPrefix(), distinguisher.First, delegateRefKind.ToParameterPrefix(), distinguisher.Second);
}
else if (lambdaRefKind != delegateRefKind)
if (lambdaRefKind != delegateRefKind)
{
if (delegateRefKind == RefKind.None)
{
Expand All @@ -2172,6 +2159,46 @@ internal void GenerateAnonymousFunctionConversionError(BindingDiagnosticBag diag
// Parameter {0} must be declared with the '{1}' keyword
Error(diagnostics, ErrorCode.ERR_BadParamRef, lambdaParameterLocation, i + 1, delegateRefKind.ToParameterDisplayString());
}

hasRefKindMismatch = true;
}
}

if (!hasRefKindMismatch)
{
for (int i = 0; i < anonymousFunction.ParameterCount; ++i)
{
TypeSymbol? lambdaParameterType = null;
if (anonymousFunction.HasExplicitlyTypedParameterList)
{
lambdaParameterType = anonymousFunction.ParameterType(i);
if (lambdaParameterType.IsErrorType())
{
continue;
}
}

var lambdaParameterLocation = anonymousFunction.ParameterLocation(i);
var lambdaRefKind = anonymousFunction.RefKind(i);
var delegateParameterType = delegateParameters[i].Type;
var delegateRefKind = delegateParameters[i].RefKind;

if (lambdaParameterType is not null && !lambdaParameterType.Equals(delegateParameterType, TypeCompareKind.AllIgnoreOptions))
{
SymbolDistinguisher distinguisher = new SymbolDistinguisher(this.Compilation, lambdaParameterType, delegateParameterType);

// Report the error beforehand to preserve previous behavior
if (!hasTypeMismatch)
{
hasTypeMismatch = true;
// Cannot convert {0} to type '{1}' because the parameter types do not match the delegate parameter types
Error(diagnostics, ErrorCode.ERR_CantConvAnonMethParams, syntax, id, targetType);
}

// Parameter {0} is declared as type '{1}{2}' but should be '{3}{4}'
Error(diagnostics, ErrorCode.ERR_BadParamType, lambdaParameterLocation,
i + 1, lambdaRefKind.ToParameterPrefix(), distinguisher.First, delegateRefKind.ToParameterPrefix(), distinguisher.Second);
}
}
}
return;
Expand Down
32 changes: 14 additions & 18 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1453,31 +1453,27 @@ private static LambdaConversionResult IsAnonymousFunctionCompatibleWithDelegate(
return LambdaConversionResult.BadParameterCount;
}

// SPEC: If F has an explicitly typed parameter list, each parameter in D has the same type
// SPEC: and modifiers as the corresponding parameter in F.
// SPEC: If F has an implicitly typed parameter list, D has no ref or out parameters.
// SPEC: If F has an explicitly typed or parenthesized implicitly type parameter list,
// SPEC: each parameter in D has the same type and modifiers as the corresponding parameter in F.
// SPEC: If F has an unparenthesized implicitly typed parameter list, D has no ref or out parameters.

if (anonymousFunction.HasExplicitlyTypedParameterList)
for (int p = 0; p < delegateParameters.Length; ++p)
{
for (int p = 0; p < delegateParameters.Length; ++p)
bool refsCompatible = OverloadResolution.AreRefsCompatibleForMethodConversion(delegateParameters[p].RefKind, anonymousFunction.RefKind(p), compilation);
bool typesCompatible = true;
if (anonymousFunction.HasExplicitlyTypedParameterList)
{
if (!OverloadResolution.AreRefsCompatibleForMethodConversion(delegateParameters[p].RefKind, anonymousFunction.RefKind(p), compilation) ||
!delegateParameters[p].Type.Equals(anonymousFunction.ParameterType(p), TypeCompareKind.AllIgnoreOptions))
{
return LambdaConversionResult.MismatchedParameterType;
}
typesCompatible = delegateParameters[p].Type.Equals(anonymousFunction.ParameterType(p), TypeCompareKind.AllIgnoreOptions);
}
}
else
{
for (int p = 0; p < delegateParameters.Length; ++p)

if (!typesCompatible || !refsCompatible)
{
if (delegateParameters[p].RefKind != RefKind.None)
{
return LambdaConversionResult.RefInImplicitlyTypedLambda;
}
return LambdaConversionResult.MismatchedParameterType;
}
}

if (!anonymousFunction.HasExplicitlyTypedParameterList)
{
// In C# it is not possible to make a delegate type
// such that one of its parameter types is a static type. But static types are
// in metadata just sealed abstract types; there is nothing stopping someone in
Expand Down
18 changes: 9 additions & 9 deletions src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1497,28 +1497,28 @@ public override bool ParameterIsDiscard(int index)

public override RefKind RefKind(int index)
{
Debug.Assert(0 <= index && index < _parameterTypesWithAnnotations.Length);
Debug.Assert(0 <= index);
return _parameterRefKinds.IsDefault ? Microsoft.CodeAnalysis.RefKind.None : _parameterRefKinds[index];
}

public override ScopedKind DeclaredScope(int index)
{
Debug.Assert(0 <= index && index < _parameterTypesWithAnnotations.Length);
Debug.Assert(0 <= index);
return _parameterDeclaredScopes.IsDefault ? ScopedKind.None : _parameterDeclaredScopes[index];
}

public override ParameterSyntax ParameterSyntax(int index)
public override ParameterSyntax? ParameterSyntax(int index)
{

Debug.Assert(_parameterSyntaxList is not null && 0 <= index && index < _parameterSyntaxList.Value.Count);
return _parameterSyntaxList.Value[index];
Debug.Assert(0 <= index);
Debug.Assert(_parameterSyntaxList is null || index < _parameterSyntaxList.Value.Count);
return _parameterSyntaxList?[index];
}

public override TypeWithAnnotations ParameterTypeWithAnnotations(int index)
{
Debug.Assert(this.HasExplicitlyTypedParameterList);
Debug.Assert(0 <= index && index < _parameterTypesWithAnnotations.Length);
return _parameterTypesWithAnnotations[index];
Debug.Assert(0 <= index);
Debug.Assert(_parameterTypesWithAnnotations.IsDefault || index < _parameterTypesWithAnnotations.Length);
return _parameterTypesWithAnnotations.IsDefault ? default : _parameterTypesWithAnnotations[index];
}

protected override UnboundLambdaState WithCachingCore(bool includeCache)
Expand Down
7 changes: 3 additions & 4 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12862,16 +12862,15 @@ private ParameterSyntax ParseLambdaParameter()
{
var attributes = ParseAttributeDeclarations(inExpressionContext: false);

// Params are actually illegal in a lambda, but we'll allow it for error recovery purposes and
// give the "params unexpected" error at semantic analysis time.
SyntaxListBuilder modifiers = _pool.Allocate();
if (IsParameterModifierExcludingScoped(this.CurrentToken) || this.CurrentToken.ContextualKind == SyntaxKind.ScopedKeyword)
{
ParseParameterModifiers(modifiers, isFunctionPointerParameter: false);
}

// If we have "scoped/ref/out/in/params" always try to parse out a type.
var paramType = modifiers.Count != 0 || ShouldParseLambdaParameterType()
bool shouldParseLambdaParameterType = ShouldParseLambdaParameterType();

var paramType = shouldParseLambdaParameterType
? ParseType(ParseTypeMode.Parameter)
: null;

Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/Source/LambdaSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,9 @@ private ImmutableArray<ParameterSymbol> MakeParameters(
else if (p < numDelegateParameters)
{
type = parameterTypes[p];
refKind = RefKind.None;
scope = ScopedKind.None;
refKind = unboundLambda.RefKind(p);
scope = unboundLambda.DeclaredScope(p);
paramSyntax = unboundLambda.ParameterSyntax(p);
}
else
{
Expand Down
53 changes: 28 additions & 25 deletions src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,31 +664,34 @@ public static void ReportParameterErrors(
// error CS1670: params is not valid in this context
diagnostics.Add(ErrorCode.ERR_IllegalParams, paramsKeyword.GetLocation());
}
else if (isParams && !typeWithAnnotations.IsSZArray())
else if (!typeWithAnnotations.IsDefault)
{
// error CS0225: The params parameter must be a single dimensional array
diagnostics.Add(ErrorCode.ERR_ParamsMustBeArray, paramsKeyword.GetLocation());
}
else if (typeWithAnnotations.IsStatic)
{
Debug.Assert(containingSymbol is null || (containingSymbol is FunctionPointerMethodSymbol or { ContainingType: not null }));
// error CS0721: '{0}': static types cannot be used as parameters
diagnostics.Add(
ErrorFacts.GetStaticClassParameterCode(containingSymbol?.ContainingType?.IsInterfaceType() ?? false),
syntax.Type?.Location ?? syntax.GetLocation(),
typeWithAnnotations.Type);
}
else if (firstDefault != -1 && parameterIndex > firstDefault && !isDefault && !isParams)
{
// error CS1737: Optional parameters must appear after all required parameters
Location loc = ((ParameterSyntax)syntax).Identifier.GetNextToken(includeZeroWidth: true).GetLocation(); //could be missing
diagnostics.Add(ErrorCode.ERR_DefaultValueBeforeRequiredValue, loc);
}
else if (refKind != RefKind.None &&
typeWithAnnotations.IsRestrictedType(ignoreSpanLikeTypes: true))
{
// CS1601: Cannot make reference to variable of type 'System.TypedReference'
diagnostics.Add(ErrorCode.ERR_MethodArgCantBeRefAny, syntax.Location, typeWithAnnotations.Type);
if (isParams && !typeWithAnnotations.IsSZArray())
{
// error CS0225: The params parameter must be a single dimensional array
diagnostics.Add(ErrorCode.ERR_ParamsMustBeArray, paramsKeyword.GetLocation());
}
else if (typeWithAnnotations.IsStatic)
{
Debug.Assert(containingSymbol is null || (containingSymbol is FunctionPointerMethodSymbol or { ContainingType: not null }));
// error CS0721: '{0}': static types cannot be used as parameters
diagnostics.Add(
ErrorFacts.GetStaticClassParameterCode(containingSymbol?.ContainingType?.IsInterfaceType() ?? false),
syntax.Type?.Location ?? syntax.GetLocation(),
typeWithAnnotations.Type);
}
else if (firstDefault != -1 && parameterIndex > firstDefault && !isDefault && !isParams)
{
// error CS1737: Optional parameters must appear after all required parameters
Location loc = ((ParameterSyntax)syntax).Identifier.GetNextToken(includeZeroWidth: true).GetLocation(); //could be missing
diagnostics.Add(ErrorCode.ERR_DefaultValueBeforeRequiredValue, loc);
}
else if (refKind != RefKind.None &&
typeWithAnnotations.IsRestrictedType(ignoreSpanLikeTypes: true))
{
// CS1601: Cannot make reference to variable of type 'System.TypedReference'
diagnostics.Add(ErrorCode.ERR_MethodArgCantBeRefAny, syntax.Location, typeWithAnnotations.Type);
}
}

if (isParams && ordinal != lastParameterIndex)
Expand All @@ -697,7 +700,7 @@ public static void ReportParameterErrors(
diagnostics.Add(ErrorCode.ERR_ParamsLast, syntax.GetLocation());
}

if (declaredScope == ScopedKind.ScopedValue && !typeWithAnnotations.IsRefLikeType())
if (declaredScope == ScopedKind.ScopedValue && !typeWithAnnotations.IsDefault && !typeWithAnnotations.IsRefLikeType())
{
diagnostics.Add(ErrorCode.ERR_ScopedRefAndRefStructOnly, syntax.Location);
}
Expand Down
Loading