Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Unify collection expression cycle handling #75464

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

jjonescz
Copy link
Member

Resolves #75024.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 10, 2024
@jjonescz jjonescz marked this pull request as ready for review October 10, 2024 11:37
@jjonescz jjonescz requested a review from a team as a code owner October 10, 2024 11:37
@jjonescz jjonescz requested a review from AlekseyTs October 10, 2024 11:37
@@ -13878,16 +13878,7 @@ static void Test(params MyCollection a)
Diagnostic(ErrorCode.ERR_ParamsCollectionInfiniteChainOfConstructorCalls, "[1]").WithArguments("MyCollection", "MyCollection.MyCollection(params MyCollection)").WithLocation(21, 14),
// (21,15): error CS9223: Creation of params collection 'MyCollection' results in an infinite chain of invocation of constructor 'MyCollection.MyCollection(params MyCollection)'.
// Test([1]);
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment referring to the correct line?

Debug.Assert((collectionCreation is BoundNewT && !isExpanded && constructor is null) ||
(collectionCreation is BoundObjectCreationExpression creation && creation.Expanded == isExpanded && creation.Constructor == constructor));

if (collectionCreation.HasErrors)
Copy link
Member

@cston cston Oct 14, 2024

Choose a reason for hiding this comment

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

collectionCreation.HasErrors

Just curious: Why is this check needed rather than continuing and binding the Add() calls?

Copy link
Member Author

@jjonescz jjonescz Oct 15, 2024

Choose a reason for hiding this comment

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

It's not needed, but it prevents cascading diagnostics. For example, in a call like Test(2, 3);, the cycle error would be reported on Test(2, 3), 2, and 3.

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)

@jjonescz jjonescz merged commit 1f78742 into dotnet:main Oct 17, 2024
24 checks passed
@jjonescz jjonescz deleted the 75024-CE-Cycles branch October 17, 2024 09:57
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Oct 17, 2024
@akhera99 akhera99 modified the milestones: Next, 17.13 P1 Oct 28, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling of infinite collection initialization chains could be unified
4 participants