Skip to content

bpo-30465: Fix C downcast warning on Windows in ast.c #6593

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

Merged
merged 1 commit into from
Apr 30, 2018
Merged

bpo-30465: Fix C downcast warning on Windows in ast.c #6593

merged 1 commit into from
Apr 30, 2018

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Apr 25, 2018

ast.c: fstring_fix_node_location() downcasts a pointer difference to
a C int. Replace int with Py_ssize_t to fix the compiler warning.

https://bugs.python.org/issue30465

@ambv
Copy link
Contributor

ambv commented Apr 25, 2018

I don't understand why this is necessary. Where is the downcast happening?

@vstinner
Copy link
Member Author

I'm sorry, I forgot to copy the compiler warning. I looked at a recent AMD64 build on Windows buildbots:

d:\buildarea\3.x.bolen-windows10\build\python\ast.c(4316): warning C4244: '+=': conversion from '__int64' to 'int', possible loss of data [D:\buildarea\3.x.bolen-windows10\build\PCbuild\pythoncore.vcxproj]

ast.c:4316:

    int cols = parent->n_col_offset;
(...)
            cols += substr - start;  // <--- HERE

@ambv
Copy link
Contributor

ambv commented Apr 27, 2018

The warning is because substr and start are addresses so they're 64 bit. But their subtraction is never going to exceed 32 bits (a single line of code exceeding 4 billion characters? Nope).

Rather than change col_offset from an int to Py_ssize_t, maybe let's just cast the subtraction on line 4316 to (int)?

If you'd rather keep Py_ssize_t, also change lineno to this type.

ast.c: fstring_fix_node_location() downcasts a pointer difference to
a C int. Replace int with Py_ssize_t to fix the compiler warning.
@vstinner
Copy link
Member Author

The warning is because substr and start are addresses so they're 64 bit. But their subtraction is never going to exceed 32 bits (a single line of code exceeding 4 billion characters? Nope).

I looked again at the code, and I think that using Py_ssize_t is wrong because node uses int:

typedef struct _node {
    ...
    int                 n_lineno;
    int                 n_col_offset;
    ...
} node;

I modified my PR to explicitly downcast to int.

@ambv ambv merged commit fb7e799 into python:master Apr 30, 2018
@bedevere-bot
Copy link

@ambv: Please replace # with GH- in the commit message next time. Thanks!

@ambv
Copy link
Contributor

ambv commented Apr 30, 2018

Exactly! Thanks! ✨ 🍰 ✨

@vstinner vstinner deleted the fix_ast_win_warn branch May 29, 2018 22:30
vstinner added a commit that referenced this pull request Apr 23, 2019
* bpo-9566: Fix compiler warnings in gcmodule.c (GH-11010)

Change PyDTrace_GC_DONE() argument type from int to Py_ssize_t.

(cherry picked from commit edad38e)

* bpo-30465: Fix C downcast warning on Windows in ast.c (#6593)

ast.c: fstring_fix_node_location() downcasts a pointer difference to
a C int. Replace int with Py_ssize_t to fix the compiler warning.

(cherry picked from commit fb7e799)

* bpo-9566: Fix compiler warnings in peephole.c (GH-10652)

(cherry picked from commit 028f0ef)

* bpo-27645, sqlite: Fix integer overflow on sleep (#6594)

Use the _PyTime_t type and round away from zero (ROUND_UP,
_PyTime_ROUND_TIMEOUT) the sleep duration, when converting a Python
object to seconds and then to milliseconds. Raise an OverflowError in
case of overflow.

Previously the (int)double conversion rounded towards zero
(ROUND_DOWN).

(cherry picked from commit ca40501)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants