#18455 closed enhancement (fixed)
Remove many unneeded includes of cdefs.pxi
| Reported by: | jdemeyer | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | sage-6.8 |
| Component: | cython | Keywords: | |
| Cc: | jpflori | Merged in: | |
| Authors: | Jeroen Demeyer | Reviewers: | Jean-Pierre Flori, Marc Mezzarobba |
| Report Upstream: | N/A | Work issues: | |
| Branch: | 370106d (Commits) | Commit: | |
| Dependencies: | Stopgaps: |
Description (last modified by )
Many Sage files contain
include 'sage/ext/cdefs.pxi'
without a good reason.
This file adds a dependency on the GMP declarations which is often unwanted.
In some cases, we just move the cdefs.pxi include from the .pxd file to the .pyx file.
Change History (12)
comment:1 Changed 6 years ago by
- Cc jpflori added
comment:2 Changed 6 years ago by
- Description modified (diff)
comment:3 Changed 6 years ago by
- Branch set to u/jdemeyer/ticket/18455
comment:4 Changed 6 years ago by
- Commit set to 2ed346b88143b4f0417432cd5546e9aed00b55e2
- Status changed from new to needs_review
comment:5 Changed 6 years ago by
- Commit changed from 2ed346b88143b4f0417432cd5546e9aed00b55e2 to 57e92784a5df59af03c6f9a778a9fc295b41e038
Branch pushed to git repo; I updated commit sha1. New commits:
| 57e9278 | More removals of cdefs.pxi
|
comment:6 follow-up: ↓ 8 Changed 6 years ago by
Couldn't we completely get rid of cdefs.pxi?
It does not seem that useful.
Or at least get rid of the gmp part within it.
By the way, your commits added an inclusion of cdefs.pxi in integer.pyx, was it intended?
comment:7 Changed 6 years ago by
- Commit changed from 57e92784a5df59af03c6f9a778a9fc295b41e038 to 370106df8a115b314ef4c50ac1ae494458c4d1b1
Branch pushed to git repo; I updated commit sha1. New commits:
| 370106d | Even more removals of cdefs
|
comment:8 in reply to: ↑ 6 Changed 6 years ago by
Replying to jpflori:
Couldn't we completely get rid of
cdefs.pxi?
Well, there is always backwards compatibility for custom Cython code. In the Sage library, we could get of it. But I'd rather not do that in this ticket, it's already big enough.
By the way, your commits added an inclusion of
cdefs.pxiininteger.pyx, was it intended?
Yes.
comment:9 Changed 6 years ago by
Now that #18450 is officially ready for review, I do not plan to make further changes to this ticket (unless requested by a reviewer of course).
comment:10 Changed 6 years ago by
- Reviewers set to Jean-Pierre Flori, Marc Mezzarobba
- Status changed from needs_review to positive_review
comment:11 Changed 6 years ago by
- Branch changed from u/jdemeyer/ticket/18455 to 370106df8a115b314ef4c50ac1ae494458c4d1b1
- Resolution set to fixed
- Status changed from positive_review to closed
comment:12 Changed 6 years ago by
- Commit 370106df8a115b314ef4c50ac1ae494458c4d1b1 deleted
Follow-up: #18519
New commits:
Remove unneeded inclusions of cdefs.pxi