Skip to content

CSharp.DeclarationTable – avoid realizing lazy roots during add/remove operations.#63517

Merged
AlekseyTs merged 1 commit intodotnet:mainfrom
AlekseyTs:WorkItem1436225
Aug 24, 2022
Merged

CSharp.DeclarationTable – avoid realizing lazy roots during add/remove operations.#63517
AlekseyTs merged 1 commit intodotnet:mainfrom
AlekseyTs:WorkItem1436225

Conversation

@AlekseyTs
Copy link
Copy Markdown
Contributor

Related to AB#1436225.

@AlekseyTs AlekseyTs requested a review from a team as a code owner August 22, 2022 17:26
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

This is an alternative for #59392.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@sharwell FYI

Copy link
Copy Markdown
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

At first glance, I like this approach better than #59392, given the simplicity. I do want to wait for @sharwell's input before approving though.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@333fred

I do want to wait for @sharwell's input before approving though.

I think this is a good change on its own. VB's DeclarationTable behaves this way already.

Copy link
Copy Markdown
Member

@333fred 333fred 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 1), as this aligns VB and C#'s implementations. Further optimizations can be applied to both languages as a follow up if needed.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler For the second review.

@RikkiGibson RikkiGibson self-assigned this Aug 23, 2022
@AlekseyTs AlekseyTs merged commit a137471 into dotnet:main Aug 24, 2022
@ghost ghost added this to the Next milestone Aug 24, 2022
@dibarbet dibarbet modified the milestones: Next, 17.4 P2 Sep 1, 2022
@sharwell sharwell added this to the 17.4 P2 milestone Nov 11, 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.

6 participants