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

'with' on anonymous type needs to lower values #54086

Merged
merged 3 commits into from
Jun 15, 2021
Merged

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Jun 14, 2021

Fixing an embarrassing bug (forgot to lower the sub-expressions in this case)...

Fixes #53849
Relates to record structs feature: #51199 (test plan)

@jcouv jcouv added this to the C# 10 milestone Jun 14, 2021
@jcouv jcouv self-assigned this Jun 14, 2021
@jcouv jcouv marked this pull request as ready for review June 14, 2021 22:08
@jcouv jcouv requested a review from a team as a code owner June 14, 2021 22:08
@RikkiGibson RikkiGibson self-assigned this Jun 14, 2021
@@ -183,7 +183,8 @@ public override BoundNode VisitWithExpression(BoundWithExpression withExpr)
Debug.Assert(left.MemberSymbol is not null);

// We evaluate the values provided in source first
BoundLocal valueTemp = _factory.StoreToTemp(assignment.Right, out BoundAssignmentOperator boundAssignmentToTemp);
var rewrittenRight = (BoundExpression)Visit(assignment.Right)!;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using LocalRewriter.VisitExpression which should make the cast and the ! unnecessary

System.Console.WriteLine(adjusted);
";
var comp = CreateCompilation(src, parseOptions: TestOptions.RegularPreview);
comp.VerifyEmitDiagnostics();
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 15, 2021

Choose a reason for hiding this comment

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

VerifyEmitDiagnostics

Minor: Calling VerifyDiagnostics on result of CompileAndVerify might be more efficient (avoids doing the work twice). #Resolved

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 2)

@jcouv jcouv enabled auto-merge (squash) June 15, 2021 20:04
@jcouv jcouv merged commit e2d605e into dotnet:main Jun 15, 2021
@ghost ghost modified the milestones: C# 10, Next Jun 15, 2021
@RikkiGibson RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021
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.

Internal Compiler Error with new feature "Extend with expression to anonymous type"
3 participants