-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Update CFG checker to use new diagnostics #589
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/diagnostics #589 +/- ##
====================================================
+ Coverage 91.73% 91.77% +0.04%
====================================================
Files 61 61
Lines 6509 6579 +70
====================================================
+ Hits 5971 6038 +67
- Misses 538 541 +3 ☔ View full report in Codecov by Sentry. |
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.
These are some very cool errors!
Some queries about falsy loops and compound predicate expressions
| ^ `z` might be undefined ... | ||
| | ||
6 | if x and (z := y + 1): | ||
| - ... if this expression is `False` |
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.
to my eye this looks like if x False
rather than if x and (z := y + 1) is False
...
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 think the message is correct:
If x
were true then z
would be defined because of the walrus assignment even if the whole thing tends out to be false.
If x
is false, then z
is not defined because and
short circuits
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.
This is the test:
@guppy
def foo(x: bool, y: int) -> int:
if x and (z := y + 1):
return z
else:
return z
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.
that makes sense! thanks
| ^ `y` might be undefined ... | ||
| | ||
6 | for _ in xs: | ||
| -- ... if this expression is `False` |
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.
this is a bit confusing, do we need to say "Falsey"...?
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 like the idea of differentiating between bools and values that are truthy. The problem is that we can no longer make this distinction here since we already inserted an implicit call to __bool__
.
An additional problem is that for loops are desugared to
iter = xs.__iter__()
while True:
b, iter = iter.__hasnext__()
if b:
x, it = iter.__next__()
else:
break
iter.__end__()
and the message actually points to the while True
bit which actually is a bool.
Created issues #591 and #592 to figure out how to solve this
| ^ `z` might be undefined ... | ||
| | ||
6 | if x or (z := y + 1): | ||
| - ... if this expression is `True` |
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.
same as previous query
No description provided.