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

[py3k] Integer floor division (//): small int check omitted #46669

Closed
terryjreedy opened this issue Mar 19, 2008 · 13 comments
Closed

[py3k] Integer floor division (//): small int check omitted #46669

terryjreedy opened this issue Mar 19, 2008 · 13 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@terryjreedy
Copy link
Member

BPO 2417
Nosy @kupuguy, @terryjreedy, @facundobatista, @abalkin, @pitrou
Files
  • issue2417.diff
  • issue2417a.diff: Patch against revision 61624
  • issue2417b.diff: Patch against revision 65210
  • 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 2008-07-24.18:57:36.608>
    created_at = <Date 2008-03-19.04:45:56.985>
    labels = ['interpreter-core', 'type-bug']
    title = '[py3k] Integer floor division (//): small int check omitted'
    updated_at = <Date 2008-07-24.18:57:36.606>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2008-07-24.18:57:36.606>
    actor = 'facundobatista'
    assignee = 'none'
    closed = True
    closed_date = <Date 2008-07-24.18:57:36.608>
    closer = 'facundobatista'
    components = ['Interpreter Core']
    creation = <Date 2008-03-19.04:45:56.985>
    creator = 'terry.reedy'
    dependencies = []
    files = ['9776', '9785', '10962']
    hgrepos = []
    issue_num = 2417
    keywords = ['patch']
    message_count = 13.0
    messages = ['64037', '64060', '64063', '64064', '64065', '64066', '64090', '64119', '64122', '64150', '70185', '70188', '70220']
    nosy_count = 6.0
    nosy_names = ['duncanb', 'terry.reedy', 'facundobatista', 'belopolsky', 'Rhamphoryncus', 'pitrou']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue2417'
    versions = ['Python 3.0']

    @terryjreedy
    Copy link
    Member Author

    Python 3.0a3 (r30a3:61161, Mar 1 2008, 22:51:17) [MSC v.1500 32 bit
    (Intel)] on win32

    >>> a,b=1,1//1
    >>> a is b
    False
    
    IDLE 3.0a3 
    >>> a,b=1,1//1
    >>> a is b
    True

    ditto for 2.5.2 interpreter

    On c.l.p, Duncan Booth wrote
    I've had a look to see why this happens: long division (and in Python 3
    all integers are longs) allocates a new long to hold the result of the
    division so it will never use one of the preallocated 'small int' values.

    That maybe explains the change from 2.5 but not the difference from
    IDLE.  More important, the small int checks are present with the other
    operations:
    >>> 1*1 is 1
    True
    >>> 1+1 is 2
    True
    >>> 1-1 is 0
    True
    >>> 1**1 is 1
    True

    so the omission with // is plausibly a bug.

    @terryjreedy terryjreedy added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Mar 19, 2008
    @terryjreedy terryjreedy changed the title Integer floor division (//): small int check omitted [py3k] Integer floor division (//): small int check omitted Mar 19, 2008
    @abalkin
    Copy link
    Member

    abalkin commented Mar 19, 2008

    I agree this is a bug. Here is a related problem:

    >>> 1 is divmod(1,1)[0]
    False

    @abalkin
    Copy link
    Member

    abalkin commented Mar 19, 2008

    >>> int('1') is 1
    False
    >>> 1 is int('0b1', 2)
    False
    >>> 1 is 0b10000 >> 4
    False

    there are probably more ...

    @abalkin
    Copy link
    Member

    abalkin commented Mar 19, 2008

    >>> 1 is 1|1
    False
    >>> 1 is 1&1
    False
    >>> 0 is 1^1
    False
    >>> 2 is 1<<1
    False

    @facundobatista
    Copy link
    Member

    I really don't understand *why* it's a bug.

    Is anywhere in the Language Reference that says that 1 should be the
    same object that another 1?

    Or do you mean that they *should* be the same object for speed and/or
    memory reasons? In this case... is this a bug? Or a request for
    improvement or something?

    @Rhamphoryncus
    Copy link
    Mannequin

    Rhamphoryncus mannequin commented Mar 19, 2008

    The original bug is not whether or not python reuses int objects, but
    rather that an existing optimization disappears under certain
    circumstances. Something is breaking our optimization.

    The later cases where the optimization is simply gone in 3.0a3 are more
    difficult. We don't *have* to keep them.. but we probably want them.
    The small int reuse happens for a reason.

    @terryjreedy
    Copy link
    Member Author

    I agree that this is not a bug in the strict sense. I could have
    selected Type: resource usage (for memory increase). But the change of
    behavior is also visible. I suspect the change is not intentional both
    because of the pattern and because of recent discussion on PyDev that
    small int caching should be kept.

    If the precise change is intentional, this should be documented (here)
    to answer further questions on c.l.p.

    @abalkin
    Copy link
    Member

    abalkin commented Mar 19, 2008

    Attached patch fixes the cases discovered so far. It is possible that
    in some of these cases creation of the long object that is later
    discarded can be avoided by detecting small int return early, but such
    optimizations should probably be left for later.

    Any advise on how to write a unit test? Should the unit test detect
    python compiled with NSMALLNEGINTS + NSMALLPOSINTS == 0 and disable
    identity checks?

    @Rhamphoryncus
    Copy link
    Mannequin

    Rhamphoryncus mannequin commented Mar 19, 2008

    Unless someone has a legitimate use case for disabling small_int that
    doesn't involve debugging (which I really doubt), I'd just assume it's
    always in use.

    @abalkin
    Copy link
    Member

    abalkin commented Mar 20, 2008

    Attached (issue2417a.diff) patch adds unit tests and fixes a few corner
    cases when a non-preallocated 0 was returned to python.

    @facundobatista
    Copy link
    Member

    Alexander, tried the issue2417a.diff patch against 65210, and does not
    apply cleanly, could you please submit an updated one?

    Thanks!

    @abalkin
    Copy link
    Member

    abalkin commented Jul 24, 2008

    It looks like e-mail submission did not work. Uploading updated patch as issue2417b.diff .

    @facundobatista
    Copy link
    Member

    Commited in r65220.

    Thank you everybody!!

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants