-
-
Notifications
You must be signed in to change notification settings - Fork 32k
bpo-46513: Remove AC_C_CHAR_UNSIGNED / __CHAR_UNSIGNED__ #30851
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
Conversation
🤖 New build scheduled with the buildbot fleet by @tiran for commit ebd654724c6e3d0b528f1313b7654002eef40b52 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
ebd6547
to
881a1cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Modules/audioop.c
Outdated
/* This module currently does not work on systems where only unsigned | ||
characters are available. Take it out of Setup. Sorry. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last part of the comment should be removed, it is confusing without the preprocessor guards. I'd consider just dropping the comment, a compiler where "signed char" is not signed should be considered broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment was introduced by b6775db in 1994, just five years after the first ever C standard was published. Internet distribution was slow and software companies were rigid back then so no surprise compatibility with per vendor compiler quirks was a real deal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this outdated comment. audioop explicitly uses signed char
and unsigned char
. Maybe to avoid any risk of confusion, the C code should use int8_t and uint8_t.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is pining for the fjords.
Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10. |
Sorry, @tiran, I could not cleanly backport this to |
Sorry @tiran, I had trouble checking out the |
GH-30914 is a backport of this pull request to the 3.10 branch. |
…onGH-30851) (cherry picked from commit 6e5a193) Co-authored-by: Christian Heimes <[email protected]>
…nGH-30851) (cherry picked from commit 6e5a193) Co-authored-by: Christian Heimes <[email protected]>
GH-30915 is a backport of this pull request to the 3.9 branch. |
…0851) (GH-30914) Co-authored-by: Christian Heimes <[email protected]>
) (GH-30915) Co-authored-by: Christian Heimes <[email protected]>
…nGH-30851) (pythonGH-30915) Co-authored-by: Christian Heimes <[email protected]>
…nGH-30851) (pythonGH-30915) Co-authored-by: Christian Heimes <[email protected]>
…nGH-30851) (pythonGH-30915) Co-authored-by: Christian Heimes <[email protected]>
https://bugs.python.org/issue46513
Automerge-Triggered-By: GH:tiran