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

Do not use special dll linkage for "cdef public" functions #1687

Merged
merged 1 commit into from Jul 15, 2017

Conversation

jdemeyer
Copy link
Contributor

This is meant to fix the module_api Cython test on Cygwin and help with various bugs in SageMath, such as https://trac.sagemath.org/ticket/21459

@embray Please test on Cygwin.

@embray
Copy link
Contributor

embray commented Apr 28, 2017

Thanks, I'll have a look soon as possible.

@embray
Copy link
Contributor

embray commented May 3, 2017

Can confirm this fixes the module_api test on Cygwin.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Jul 4, 2017

With Cython 0.26, this is the only remaining Cython patch in SageMath.

@robertwb
Copy link
Contributor

robertwb commented Jul 5, 2017

I'd like someone with Windows experience to verify this is the intended behavior.

@embray
Copy link
Contributor

embray commented Jul 6, 2017

I did.

@robertwb
Copy link
Contributor

robertwb commented Jul 9, 2017

What are the odds that someone is relying on the current behavior and will be broken? Seems it merits at least a note in CHANGES.rst.

@embray
Copy link
Contributor

embray commented Jul 10, 2017

I don't think it will break anything because the current behavior is broken anyways, but I agree it merits a changelog entry.

@robertwb
Copy link
Contributor

OK, document on CHANGES.rst and I'll merge.

@robertwb robertwb added this to the 0.26 milestone Jul 10, 2017
@insertinterestingnamehere
Copy link
Contributor

This strikes me as a really positive move. This will make public declarations work for sharing Cython declarations with C files built into the same extension module. Currently that only works on Python 3 since the DL_IMPORT/DL_EXPORT macros are no-ops there. The tests imply that that's the intended use-case for public declarations. In theory this could potentially break code that uses public declarations to share things with C-level code outside the module itself, but that's a dubious way of doing things, and it would only work on Python 2 right now. That behavior is really more of a bug than a feature. Removing the specifiers makes the behavior consistent and allows use-cases that follow best practices to work across all Python versions.

@robertwb robertwb merged commit 632f18f into cython:master Jul 15, 2017
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

4 participants