classification
Title: remove unnecessary operation in long_compare()
Type: Stage: resolved
Components: Versions: Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Ma Lin, hongweipeng, inada.naoki, sir-sigurd, vstinner
Priority: normal Keywords: patch, patch, patch

Created on 2019-01-09 09:26 by Ma Lin, last changed 2019-09-21 09:56 by hongweipeng.

Pull Requests
URL Status Linked Edit
PR 11473 closed Ma Lin, 2019-01-09 09:27
PR 11473 closed Ma Lin, 2019-01-09 09:27
PR 11473 closed Ma Lin, 2019-01-09 09:27
PR 16146 merged hongweipeng, 2019-09-14 17:36
Messages (5)
msg333293 - (view) Author: Ma Lin (Ma Lin) * Date: 2019-01-09 09:26
static int
long_compare(PyLongObject *a, PyLongObject *b)
{
    ....
}

This function in /Objects/longobject.c is used to compare two PyLongObject's value.
We only need the sign, converting to -1 or +1 is not necessary.
msg352732 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2019-09-18 15:10
New changeset 42acb7b8d29d078bc97b0cfd7c4911b2266b26b9 by Inada Naoki (HongWeipeng) in branch 'master':
bpo-35696: Simplify long_compare() (GH-16146)
https://github.com/python/cpython/commit/42acb7b8d29d078bc97b0cfd7c4911b2266b26b9
msg352875 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-09-20 17:16
I reopen the issue. The win32 job of Azure Pipelines now logs a compiler warning:

Objects\longobject.c(412,5): warning C4244:  'function': conversion from 'unsigned long' to 'sdigit', possible loss of data
Objects\longobject.c(420,5): warning C4244:  'function': conversion from 'unsigned __int64' to 'sdigit', possible loss of data

See also discussion on PR 16146:
https://github.com/python/cpython/pull/16146#issuecomment-533351728
msg352883 - (view) Author: Sergey Fedoseev (sir-sigurd) * Date: 2019-09-20 19:28
These warnings are caused by https://github.com/python/cpython/commit/c6734ee7c55add5fdc2c821729ed5f67e237a096.

I'd fix them, but I'm not sure if we are going to restore CHECK_SMALL_INT() ¯\_(ツ)_/¯
msg352903 - (view) Author: Ma Lin (Ma Lin) * Date: 2019-09-20 22:26
> I'd fix them, but I'm not sure if we are going to restore CHECK_SMALL_INT() ¯\_(ツ)_/¯

I suggest we slow down, carefully sort out the recent commits for longobject.c:
https://bugs.python.org/issue37812#msg352837

Make the code has consistent style, better readability...
History
Date User Action Args
2019-09-21 09:56:56hongweipengsetnosy: + hongweipeng
2019-09-20 22:26:47Ma Linsetmessages: + msg352903
2019-09-20 19:28:53sir-sigurdsetnosy: + sir-sigurd
messages: + msg352883
2019-09-20 17:16:06vstinnersetstatus: closed -> open

nosy: + vstinner
messages: + msg352875

keywords: patch, patch, patch
resolution: fixed ->
2019-09-18 15:14:29inada.naokisetkeywords: patch, patch, patch
status: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.9, - Python 3.8
2019-09-18 15:10:28inada.naokisetnosy: + inada.naoki
messages: + msg352732
2019-09-14 17:36:08hongweipengsetpull_requests: + pull_request15756
2019-01-09 09:27:45Ma Linsetkeywords: + patch
stage: patch review
pull_requests: + pull_request10976
2019-01-09 09:27:41Ma Linsetkeywords: + patch
stage: (no value)
pull_requests: + pull_request10975
2019-01-09 09:27:33Ma Linsetkeywords: + patch
stage: (no value)
pull_requests: + pull_request10974
2019-01-09 09:26:39Ma Lincreate