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

bpo-39689: Do not use native packing for format "?" with standard size by skrah · Pull Request #18969 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-39689: Do not use native packing for format "?" with standard size - #18969

Merged
miss-islington merged 1 commit into
python:masterfrom
skrah:skip_native_bool_optimization
Mar 24, 2020
ConversationCommits (1)ChecksFiles changed
Merged

bpo-39689: Do not use native packing for format "?" with standard size#18969
miss-islington merged 1 commit into
python:masterfrom
skrah:skip_native_bool_optimization

Conversation

skrah commented Mar 12, 2020 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Contributor

skrah commented Mar 12, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

This implements the current documentation somewhat pedantically:

The '?' conversion code corresponds to the _Bool type defined by C99. If this type is not available, it is simulated using a char. In standard mode, it is always represented by one byte.

We always have C99 now, so native mode always uses _Bool. When compiling with
-fsanitize=bool, this should (and does) fail:

>>> from struct import *
>>> unpack("?", b"\x02")
/home/stefan/cpython/Modules/_struct.c:487:12: runtime error: load of value 2, which is not a valid value for type '_Bool'
(False,)

This, however, should pass (and does after this patch):

>>> unpack("<?", b"\x02")
(True,)

encukou requested changes Mar 17, 2020
View reviewed changes

encukou left a comment

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

The docs currently say:

For the '?' format character, the return value is either True or False. When packing, the truth value of the argument object is used. Either 0 or 1 in the native or standard bool representation will be packed, and any non-zero value will be True when unpacking.

So, if we do want change that and expose UB to Python, the docs need to change. IMO, this will need a news entry as well.

The docs also currently talk about the characters @=<>! only affecting byte order, size, and alignment. Will that need to be expanded?

bedevere-bot commented Mar 17, 2020

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

miss-islington merged commit 472fc84 into python:master Mar 24, 2020

miss-islington commented Mar 25, 2020

Copy link
Copy Markdown
Contributor

Thanks @skrah for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington commented Mar 25, 2020

Copy link
Copy Markdown
Contributor

Thanks @skrah for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

bedevere-bot commented Mar 25, 2020

Copy link
Copy Markdown

GH-19154 is a backport of this pull request to the 3.7 branch.

bedevere-bot commented Mar 25, 2020

Copy link
Copy Markdown

GH-19155 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 25, 2020
bpo-39689: Do not use native packing for format "?" with standard size (
ffdc144 
pythonGH-18969)

(cherry picked from commit 472fc84)

Co-authored-by: Stefan Krah <skrah@bytereef.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 25, 2020
bpo-39689: Do not use native packing for format "?" with standard size (
9978822 
pythonGH-18969)

(cherry picked from commit 472fc84)

Co-authored-by: Stefan Krah <skrah@bytereef.org>
miss-islington added a commit that referenced this pull request Mar 31, 2020
bpo-39689: Do not use native packing for format "?" with standard size (
0f9e889 
GH-18969)

(cherry picked from commit 472fc84)

Co-authored-by: Stefan Krah <skrah@bytereef.org>
miss-islington added a commit that referenced this pull request Mar 31, 2020
bpo-39689: Do not use native packing for format "?" with standard size (
572ef74 
GH-18969)

(cherry picked from commit 472fc84)

Co-authored-by: Stefan Krah <skrah@bytereef.org>
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

encukou encukou approved these changes
mdickinson Awaiting requested review from mdickinson
meadori Awaiting requested review from meadori

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL