-
-
Notifications
You must be signed in to change notification settings - Fork 32k
GH-130415: Narrow str
to ""
based on boolean tests
#130476
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
Changes from 1 commit
80c1c6c
23695d6
fcf4116
e23b84a
ab263fe
d5bfcdc
601392d
58884ab
905d3ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
f
var to empty
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -439,7 +439,7 @@ dummy_func(void) { | |
// removed and the narrowed value to be invalid: | ||
if (next_opcode == _GUARD_IS_FALSE_POP) { | ||
sym_set_const(value, Py_GetConstant(Py_CONSTANT_EMPTY_STR)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is strictly incorrect. We don't know that There are two possible fixes for this.
I prefer the second option, although it may be more work, as it is more flexible and can be extended more easily. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, @Fidget-Spinner and I suggested something like the latter on the issue (new symbols like However, I don't think we should let perfect be the enemy of good here. We have nice, working optimizations in these PRs; just because we might sink info onto side exits in the future probably shouldn't prevent us from making changes like this now for 3.14, which are perfectly correct for the current optimizer (which doesn't sink anything). I'm inclined to land these changes and other similar ones for But I'll defer to you here. If having |
||
res = sym_new_type(ctx, &PyUnicode_Type); | ||
res = sym_new_type(ctx, &PyBool_Type); | ||
} | ||
} | ||
} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 can easily see the optimizer turning
"aaa"[:0]
into""
.empty
doesn't need to be a constant, we just need it to be mostly "", for profiling.Use something like
empty = "a"[:(n % 1000) == 0]
Uh oh!
There was an error while loading. Please reload this page.
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.
Since we check the actual path taken as part of the test, we need the value to always be
""
, not just mostly""
. So maybe:The optimizer can't prove
false
isFalse
, so it's good enough for our purposes.