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

Add Python 3.6 support #25

Merged
merged 2 commits into from
Sep 22, 2017
Merged

Add Python 3.6 support #25

merged 2 commits into from
Sep 22, 2017

Conversation

mgedmin
Copy link
Member

@mgedmin mgedmin commented Sep 21, 2017

Closes #24.

@mgedmin
Copy link
Member Author

mgedmin commented Sep 21, 2017

I'm seeing a new build warning on Python 3.6 (only):

src/zodbpickle/_pickle_33.c: In function ‘Pdata_grow’:
src/zodbpickle/_pickle_33.c:221:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (new_allocated > (PY_SSIZE_T_MAX / sizeof(PyObject *)))
                       ^

@mgedmin mgedmin mentioned this pull request Sep 21, 2017
@jamadden
Copy link
Member

I'm seeing a new build warning on Python 3.6 (only)

Interesting. I think that's highly platform dependent, although I don't really understand why it started showing up. new_allocated is defined as Py_ssize_t. The definition of PY_SSIZE_T_MAX is the same in 2.7:

cpython [2.7|…1]$ ack 'define PY_SSIZE_T_MAX'
Include/pyport.h
202:#define PY_SSIZE_T_MAX ((Py_ssize_t)(((size_t)-1)>>1))

as it is in 3.6:

cpython [3.6|…1]$ ack 'define PY_SSIZE_T_MAX'
Include/pyport.h
109:#define PY_SSIZE_T_MAX ((Py_ssize_t)(((size_t)-1)>>1))

AFAICS, Py_ssize_t itself is the same in 2.7 as it is in 3.6, at least on any platforms that have ssize_t defined. If that isn't defined, then it changes: on both versions, the fallback is to make it the same as Py_intptr_t, but the value of that has changed. In 3.6, it's unconditionally intptr_t, but 2.7 supports platforms that don't have that defined, so it could wind up as just int. In any case, those should all be signed.

Now, sizeof is always defined to return an unsigned integer (technically an unsigned integer constant). I forget the rules for C type conversions between unsigned and signed integers but I think promotion is generally to unsigned?

Does this warning go away if you cast the right hand expression to Py_ssize_t?

@jamadden
Copy link
Member

FWIW, the 3.6 pickle module does a similar cast. Although it has also been changed to just use size_t instead of Py_ssize_t back in 3.5 because of this very same issue.

So it looks like what may have changed was the addition of more warning flags to the compiler by distutils.

@mgedmin mgedmin merged commit 6f69570 into master Sep 22, 2017
@icemac icemac deleted the py36-support branch November 30, 2017 12:43
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