-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix internal error in visitor91x #232
Conversation
@@ x,3 x,4 @@ | ||
@@ x,6 x,7 @@ |
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.
Ooooh after some confusion I think I figured out why this line in the diff output got changed. It's because there's now context after the changed line in await_everywhere_except_gen_target
(i.e. two empty lines and # Issue #226
), so now there's 3 more lines in the diff output.
+ await trio.lowlevel.checkpoint() | ||
|
||
|
||
# Issue #226 | ||
@@ x,3 x,4 @@ | ||
pass | ||
except Exception: | ||
pass | ||
+ await trio.lowlevel.checkpoint() |
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.
once again this looks kinda weird in the github diff view, due to viewing a syntax-highlighted diff file in diff view. But the actual diff is entirely as expected.
@@ -416,6 +416,7 @@ def _parse_eval_file( | |||
msg = f"Line {lineno}: Failed to format\n {message!r}\nwith\n{args}" | |||
raise ParseError(msg) from e | |||
|
|||
assert enabled_codes, "no codes enabled. Fix file name or add `# ARGS --enable=...`" |
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.
minor unrelated QoL when writing new tests to get a nice error message.
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.
Sweet, thanks @jakkdl!
fixes #226
Underlying issue was different try/except's sharing the same reference to self.try_state, due to not passing
copy=True
tosave_state()
.I went through the other calls to
save_state
in the same visitor and added an explicitcopy=False
to the only other one that didn't already specify it. It's possiblesave_state()
shouldn't have a default at all, or default toTrue
since the performance loss from some extra copying would probably be negligible.