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

Overrides =copy for PackedSets #21417

Merged
merged 1 commit into from Feb 26, 2023
Merged

Overrides =copy for PackedSets #21417

merged 1 commit into from Feb 26, 2023

Conversation

ringabout
Copy link
Member

ref #21393

fixes the problem, I'm going to look into the root cause later.

It is a breaking change:

Currently the assignment operator = for PackedSet[A]
performs some rather meaningless shallow copy. Since Nim currently does
not allow the assignment operator to be overloaded, use the assign proc <#assign,PackedSet[A],PackedSet[A]>_ to get a deep copy.

The PR intends to fix the shallow issue. It overrides the =copy type binding operation and does a deep copy. The root cause might be that the strictfuncs cannot identify the
mutation of underlying refs.

@ringabout ringabout closed this Feb 21, 2023
@ringabout ringabout deleted the pr_packedsets branch February 21, 2023 14:53
@Araq
Copy link
Member

Araq commented Feb 21, 2023

Breaking or not, it's a good change!

@ringabout ringabout restored the pr_packedsets branch February 22, 2023 03:17
@ringabout ringabout reopened this Feb 22, 2023
@Araq Araq merged commit 6fea221 into devel Feb 26, 2023
37 checks passed
@Araq Araq deleted the pr_packedsets branch February 26, 2023 23:57
@github-actions
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 6fea221

Hint: mm: orc; opt: speed; options: -d:release
166273 lines; 8.321s; 610.68MiB peakmem

survivorm pushed a commit to survivorm/Nim that referenced this pull request Feb 28, 2023
capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
bung87 pushed a commit to bung87/Nim that referenced this pull request Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants