Skip to content

Fix issues where internal pattern matching state was being copied#69336

Merged
CyrusNajmabadi merged 5 commits intodotnet:mainfrom
CyrusNajmabadi:patternMatcherCopies
Aug 10, 2023
Merged

Fix issues where internal pattern matching state was being copied#69336
CyrusNajmabadi merged 5 commits intodotnet:mainfrom
CyrusNajmabadi:patternMatcherCopies

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

No description provided.

@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 2, 2023
/// <summary>
/// Not readonly as this value caches data within it, and so it needs to be able to mutate.
/// </summary>
public WordSimilarityChecker SimilarityChecker;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the alternative is to accept that thsi caching optimization was never working, remove it entirely, and make all this stuff readonly.

@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review August 4, 2023 16:09
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner August 4, 2023 16:09
@CyrusNajmabadi CyrusNajmabadi requested a review from Cosifne August 4, 2023 16:10
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

@genlu @Cosifne @akhera99 ptal :)

1 similar comment
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

@genlu @Cosifne @akhera99 ptal :)

@CyrusNajmabadi CyrusNajmabadi merged commit e2f862f into dotnet:main Aug 10, 2023
@CyrusNajmabadi CyrusNajmabadi deleted the patternMatcherCopies branch August 10, 2023 17:25
@ghost ghost added this to the Next milestone Aug 10, 2023
@dibarbet dibarbet modified the milestones: Next, 17.8 P2 Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE 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.

3 participants