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

Deduplicate FLINT finalizers #2008

Merged
merged 6 commits into from
Jan 30, 2025
Merged

Conversation

lgoettgens
Copy link
Collaborator

[...] several high-level Julia structs wrap the same underlying FLINT structs (e.g. fmpz_poly_struct is wrapped by ZZPolyRingElem, ZZAbsPowerSeriesRingElem and ZZRelPowerSeriesRingElem) and so they could use the same finalizer (well technically they already could do that now).
excerpt from #1889 (comment) by @fingolfin

implements exactly this by first moving all of them to a common block, renaming them w.r.t. the called flint function, sorting alphabetically, and then merging neighboring ones where appropriate.

@lgoettgens lgoettgens requested a review from fingolfin January 29, 2025 13:31
@lgoettgens lgoettgens changed the title Collect all flintsomething_clear_fn Deduplicate flint finalizers Jan 29, 2025
@lgoettgens lgoettgens changed the title Deduplicate flint finalizers Deduplicate FLINT finalizers Jan 29, 2025
Copy link

codecov bot commented Jan 29, 2025

Codecov Report

Attention: Patch coverage is 96.46465% with 7 lines in your changes missing coverage. Please review.

Project coverage is 88.29%. Comparing base (0644dcc) to head (4179f87).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/flint/FlintTypes.jl 96.44% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2008      +/-   ##
==========================================
- Coverage   88.31%   88.29%   -0.03%     
==========================================
  Files          98       98              
  Lines       36162    36101      -61     
==========================================
- Hits        31938    31876      -62     
- Misses       4224     4225       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fingolfin fingolfin added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Jan 29, 2025
@lgoettgens lgoettgens merged commit 82193e6 into Nemocas:master Jan 30, 2025
24 checks passed
@lgoettgens lgoettgens deleted the lg/finalizer branch January 30, 2025 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants