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

Additional tests for lambda attributes, explicit return type, and inferred delegate type #59634

Merged
merged 5 commits into from
Feb 20, 2022

Conversation

cston
Copy link
Member

@cston cston commented Feb 18, 2022

Add various tests identified in test plan #52192 ...

Attributes

Parsing

  • Object/collection initializers

Semantics

  • GetTypeInfo (see LambdaTests.LambdaAttributes_AttributeSemanticModel())
  • SyntaxNormalizer

Explicit return type

Syntax

  • async MyMethod() => null; on the top level parses as a local function statement with a return type of async

Semantics

Inferred (natural) delegate type

Semantics

@cston cston marked this pull request as ready for review February 18, 2022 19:06
@cston cston requested a review from a team as a code owner February 18, 2022 19:06
@jcouv jcouv self-assigned this Feb 18, 2022
@jcouv jcouv added Test Test failures in roslyn-CI New Feature - Lambda Improvements labels Feb 18, 2022
@@ -399,6 +399,33 @@ public void TestNormalizeStatement1()
TestNormalizeStatement("Func<string, int> f = blah;", "Func<string, int> f = blah;");
}

[Theory]
[InlineData("[return:A]void Local([B]object o){}", "[return: A]\r\nvoid Local([B] object o)\r\n{\r\n}")]
Copy link
Member

@jcouv jcouv Feb 18, 2022

Choose a reason for hiding this comment

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

nit: Consider making the input worse (literally the opposite for every position where possible on the attributes):

Suggested change
[InlineData("[return:A]void Local([B]object o){}", "[return: A]\r\nvoid Local([B] object o)\r\n{\r\n}")]
[InlineData("[ return:A ]void Local( [ B ]object o){}", "[return: A]\r\nvoid Local([B] object o)\r\n{\r\n}")]
``` #Closed

UsingTree(source, TestOptions.Regular9);
verify();

UsingTree(source, TestOptions.RegularNext);
Copy link
Member

@jcouv jcouv Feb 18, 2022

Choose a reason for hiding this comment

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

Seems like this should be Regular10 (we only need to use RegularNext when we mean Regular11 which doesn't exist yet) #Resolved

UsingExpression(source, TestOptions.Regular9);
verify();

UsingExpression(source, TestOptions.RegularNext);
Copy link
Member

@jcouv jcouv Feb 18, 2022

Choose a reason for hiding this comment

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

Ditto #Closed


var symbolInfo = model.GetSymbolInfo(returnTypeSyntax);
Assert.Equal(expectedType, symbolInfo.Symbol);
}
Copy link
Member

@jcouv jcouv Feb 18, 2022

Choose a reason for hiding this comment

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

Did you find the stress test that has deeply nested lambdas? We could add a variant where the return type removes the "stress". #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Added OverloadResolutionPerfTests.NestedLambdas_WithParameterAndReturnTypes().

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 2)

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 5)

@cston cston merged commit c85df2b into dotnet:main Feb 20, 2022
@cston cston deleted the lambda-tests branch February 20, 2022 04:42
@ghost ghost added this to the Next milestone Feb 20, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P2 Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants