Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#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, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

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 7 years ago by jdemeyer

  • Cc jpflori added

comment:2 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 7 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/18455

comment:4 Changed 7 years ago by jdemeyer

  • Commit set to 2ed346b88143b4f0417432cd5546e9aed00b55e2
  • Status changed from new to needs_review

New commits:

​2ed346bRemove unneeded inclusions of cdefs.pxi

comment:5 Changed 7 years ago by git

  • Commit changed from 2ed346b88143b4f0417432cd5546e9aed00b55e2 to 57e92784a5df59af03c6f9a778a9fc295b41e038

Branch pushed to git repo; I updated commit sha1. New commits:

​57e9278More removals of cdefs.pxi

comment:6 follow-up: Changed 7 years ago by jpflori

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 7 years ago by git

  • Commit changed from 57e92784a5df59af03c6f9a778a9fc295b41e038 to 370106df8a115b314ef4c50ac1ae494458c4d1b1

Branch pushed to git repo; I updated commit sha1. New commits:

​370106dEven more removals of cdefs

comment:8 in reply to: ↑ 6 Changed 7 years ago by jdemeyer

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.pxi in integer.pyx, was it intended?

Yes.

comment:9 Changed 7 years ago by jdemeyer

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 7 years ago by mmezzarobba

  • Reviewers set to Jean-Pierre Flori, Marc Mezzarobba
  • Status changed from needs_review to positive_review

comment:11 Changed 7 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/18455 to 370106df8a115b314ef4c50ac1ae494458c4d1b1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:12 Changed 7 years ago by jdemeyer

  • Commit 370106df8a115b314ef4c50ac1ae494458c4d1b1 deleted

Follow-up: #18519

Note: See TracTickets for help on using tickets.