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

Fix usage of Py_ssize_t type in Python/compile.c #63816

Closed
vstinner opened this issue Nov 15, 2013 · 8 comments
Closed

Fix usage of Py_ssize_t type in Python/compile.c #63816

vstinner opened this issue Nov 15, 2013 · 8 comments

Comments

@vstinner
Copy link
Member

BPO 19617
Nosy @vstinner, @benjaminp, @serhiy-storchaka
Files
  • compile_ssize_t.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2013-11-19.21:24:37.918>
    created_at = <Date 2013-11-15.23:08:07.747>
    labels = []
    title = 'Fix usage of Py_ssize_t type in Python/compile.c'
    updated_at = <Date 2013-11-19.22:56:55.865>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2013-11-19.22:56:55.865>
    actor = 'python-dev'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-11-19.21:24:37.918>
    closer = 'vstinner'
    components = []
    creation = <Date 2013-11-15.23:08:07.747>
    creator = 'vstinner'
    dependencies = []
    files = ['32644']
    hgrepos = []
    issue_num = 19617
    keywords = ['patch']
    message_count = 8.0
    messages = ['202976', '202982', '203263', '203292', '203425', '203427', '203433', '203441']
    nosy_count = 4.0
    nosy_names = ['vstinner', 'benjamin.peterson', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue19617'
    versions = ['Python 3.4']

    @vstinner
    Copy link
    Member Author

    On Windows 64-bit, Visual Studio generates a lot of warnings because Py_ssize_t values are downcasted to int.

    Attached patch fixes all warnings and move the final downcast into compiler_addop_i(). This function uses an assertion to check that integer parameter fits into an C int type. I don't know if it's safe to "return 0" on overflow error.

    The patch fixes also some indentation issues seen was I wrote the patch.

    Nobody complained before, I don't know if the bugs can be seen in practice, so I prefer to not touch Python 2.7 nor 3.2.

    @benjaminp
    Copy link
    Contributor

    You could use the Py_SAFE_DOWNCAST macro everywhere.

    @vstinner
    Copy link
    Member Author

    You could use the Py_SAFE_DOWNCAST macro everywhere.

    I prefer to store sizes in a size type (Py_ssize_t), and only downcast where it is really needed, in compiler_addop_i(). So in the future, if someone wants to support values larger than INT_MAX, only one function need to be changed.

    @benjaminp
    Copy link
    Contributor

    I meant where you added casts, you could use it.

    2013/11/18 STINNER Victor <report@bugs.python.org>:

    STINNER Victor added the comment:

    > You could use the Py_SAFE_DOWNCAST macro everywhere.

    I prefer to store sizes in a size type (Py_ssize_t), and only downcast where it is really needed, in compiler_addop_i(). So in the future, if someone wants to support values larger than INT_MAX, only one function need to be changed.

    ----------


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue19617\>


    @vstinner
    Copy link
    Member Author

    Oops, I forgot to mention this issue number in the commit:
    ---
    changeset: 87277:68fd86a83ece
    tag: tip
    user: Victor Stinner <victor.stinner@gmail.com>
    date: Tue Nov 19 22:23:20 2013 +0100
    files: Python/compile.c
    description:
    Issue bpo-9566: compile.c uses Py_ssize_t instead of int to store sizes to fix
    compiler warnings on Windows 64-bit. Use Py_SAFE_DOWNCAST() where the final
    downcast is needed.

    The bytecode doesn't support integer parameters larger than 32-bit yet.
    ---

    I added some more Py_SAFE_DOWNCAST() in the final commit.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 19, 2013

    New changeset 8d3e85dfa46f by Victor Stinner in branch 'default':
    Issue bpo-9566, bpo-19617: Fix compilation on Windows
    http://hg.python.org/cpython/rev/8d3e85dfa46f

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 19, 2013

    New changeset ee4da7291211 by Victor Stinner in branch 'default':
    Issue bpo-9566, bpo-19617: New try to fix compilation on Windows
    http://hg.python.org/cpython/rev/ee4da7291211

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 19, 2013

    New changeset 116bd550e309 by Victor Stinner in branch 'default':
    Issue bpo-9566, bpo-19617: Fix more compiler warnings in compile.c on Windows 64-bit
    http://hg.python.org/cpython/rev/116bd550e309

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    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

    No branches or pull requests

    2 participants