FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-83714: Fix compiler warning in stat_nanosecond_timestamp() by vstinner · Pull Request #141043 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-83714: Fix compiler warning in stat_nanosecond_timestamp() - #141043

Merged
vstinner merged 1 commit into
python:mainfrom
vstinner:statx_sec_to_ns
Nov 5, 2025
ConversationCommits (1)ChecksFiles changed
Merged

gh-83714: Fix compiler warning in stat_nanosecond_timestamp()#141043
vstinner merged 1 commit into
python:mainfrom
vstinner:statx_sec_to_ns

Conversation

vstinner commented Nov 5, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

Disable the fast path on systems with 32-bit long.

pythongh-83714: Fix compiler warning in stat_nanosecond_timestamp()
c86068e 
Disable the fast path on systems with 32-bit long.
bedevere-app Bot mentioned this pull request Nov 5, 2025
Closed

vstinner commented Nov 5, 2025

Copy link
Copy Markdown
Member Author

Fix the warnings:

./Modules/posixmodule.c:2638:31: warning: comparison is always true due to limited range of data type [-Wtype-limits]
./Modules/posixmodule.c:2638:45: warning: comparison is always true due to limited range of data type [-Wtype-limits]

cc @cmaloney @jbosboom

vstinner commented Nov 5, 2025

Copy link
Copy Markdown
Member Author

!buildbot x86 Debian

bedevere-bot commented Nov 5, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @vstinner for commit c86068e 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F141043%2Fmerge

The command will test the builders whose names match following regular expression: x86 Debian

The builders matched are:

  • x86 Debian Installed with X PR
  • x86 Debian Non-Debug with X PR

vstinner commented Nov 5, 2025

Copy link
Copy Markdown
Member Author

https://buildbot.python.org/#/builders/1285/builds/530 confirms that the compiler warnings on Modules/posixmodule.c are gone.

vstinner commented Nov 5, 2025

Copy link
Copy Markdown
Member Author

On 32-bit, the fast path is not interesting, it only covers seconds in the range [-3; 2]:

>>> -2**31 // 1_000_000_000
-3
>>> (2**31-1) // 1_000_000_000
2

vstinner merged commit 30ab627 into python:main Nov 5, 2025
50 checks passed
vstinner deleted the statx_sec_to_ns branch November 5, 2025 17:31
serhiy-storchaka reviewed Nov 5, 2025
View reviewed changes
Comment thread Modules/posixmodule.c
static PyObject *
stat_nanosecond_timestamp(_posixstate *state, time_t sec, unsigned long nsec)
{
#if SIZEOF_LONG >= 8

serhiy-storchaka Nov 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I do not think this is correct. The comparison is always true, not always false if SIZEOF_LONG < 8.

vstinner Nov 5, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would you mind to elaborate? I'm not sure that I understand your remark. What do you suggest?

serhiy-storchaka Nov 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Previously, if SIZEOF_LONG < 8, the fast path PyLong_FromLongLong(sec * SEC_TO_NS + nsec) was always used. Now it is never used.

vstinner Nov 5, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I wrote a different fix: #141069

vstinner Nov 5, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think that I now understood my mistake. Sorry about that :-( I read LONG_MIN (32-bit) instead of LLONG_MIN (64-bit).

StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
pythongh-83714: Fix a compiler warning in stat_nanosecond_timestamp() (…
c112cce 
…python#141043)

Disable the fast path on systems with 32-bit long.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

serhiy-storchaka serhiy-storchaka left review comments

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL