-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
GH-130415: Use boolean guards to narrow types to values in the JIT #130659
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
Merged
brandtbucher
merged 13 commits into
python:main
from
brandtbucher:jit-narrow-to-bool-bool
Mar 2, 2025
Merged
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
d0f236a
Add failing regression tests
brandtbucher 7929a57
Add "truth" symbols
brandtbucher 6c4588e
Fix up test
brandtbucher 1d17afe
is
brandtbucher dcb294c
Add creation and narrowing of truths
brandtbucher 41e77a8
Use the new truth symbols
brandtbucher d0b1724
Fix tests
brandtbucher c8214c9
blurb add
brandtbucher ffa559b
"truth" -> "truthiness"
brandtbucher cbd4569
Fix some compiler warnings
brandtbucher 9f1b271
Invert the "not" arg for clarity
brandtbucher 07697ed
Merge branch 'main' into jit-narrow-to-bool-bool
brandtbucher 4edba18
Add tests for truthiness symbol
brandtbucher File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Use the new truth symbols
- Loading branch information
commit 41e77a88757ddd8439cdd87fb446837a3a3c0259
There are no files selected for viewing
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
From casual reading, this looks like an error as the
true
argument implies that this has the valueTrue
.Maybe rename
sym_new_truth
assym_from_truthiness
and havefalse
as the argument forNOT
andtrue
as the argument forTO_BOOL
?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.
Or symbolic consts, like
TO_BOOL
andINVERT
?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.
I'd like to avoid a double-negative in the arg name (since inverting it from its current meaning would mean something like "not not"). What if I make it an int and use
0
and1
instead? I feel like a big part of the friction in reading this is the use oftrue
andfalse
as arguments.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.
0 /1 or false/true doesn't matter. Good symbolic names is what is needed.
Maybe even use two different functions with different names?