Skip to content
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: improve panics in IR generation #3708

Merged
merged 3 commits into from
Dec 25, 2023

Conversation

charles-cooper
Copy link
Member

this QOL commit improves on 9165926 by passing through the __traceback__ field when the exception is modified (instead of using __cause__ - cf. PEP-3134 regarding the difference) and improves error messages when an IRnode is not returned properly. using __traceback__ generally results in a better experience because the immediate cause of the exception is displayed when running vyper -v instead of needing to scroll up through the exception chain (if the exception chain is reproduced correctly at all in the first place).

What I did

How I did it

How to verify it

Commit message

Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

this QOL commit improves on 9165926 by passing through the
`__traceback__` field when the exception is modified (instead of using
`__cause__` - cf. PEP-3134 regarding the difference) and improves error
messages when an IRnode is not returned properly. using `__traceback__`
generally results in a better experience because the immediate cause of
the exception is displayed when running `vyper -v` instead of needing to
scroll up through the exception chain (if the exception chain is
reproduced correctly at all in the first place).
@charles-cooper charles-cooper marked this pull request as ready for review December 24, 2023 16:28
@codecov-commenter
Copy link

codecov-commenter commented Dec 24, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (5319cfb) 83.97% compared to head (4e78510) 84.02%.

Files Patch % Lines
vyper/codegen/expr.py 40.00% 6 Missing ⚠️
vyper/exceptions.py 75.00% 1 Missing and 1 partial ⚠️
vyper/codegen/stmt.py 83.33% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3708      +/-   ##
==========================================
+ Coverage   83.97%   84.02%   +0.04%     
==========================================
  Files          92       92              
  Lines       13044    13045       +1     
  Branches     2928     2925       -3     
==========================================
+ Hits        10954    10961       +7     
+ Misses       1661     1658       -3     
+ Partials      429      426       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pcaversaccio
Copy link
Collaborator

pcaversaccio commented Dec 25, 2023

I'm not sure this is the right venue to discuss this (I still drop it here tho), but I thought it would be valuable if we would start adding labels and milestones for each PR going forward:

image

Like the milestone is usually the next Vyper release number & the label can be e.g. IR, Codegen etc. (can also be combined ofc), whatever makes most sense. So the effort is really low doing this.

Using such an approach would help a lot to filter for the PRs someone is interested in reading. At least I use this approach for e.g. 🐍 snekmate (see an example here), and people appreciate it. Let me know what you think about it.

@charles-cooper charles-cooper merged commit 2df916c into vyperlang:master Dec 25, 2023
84 checks passed
@charles-cooper charles-cooper deleted the improve-panics branch December 25, 2023 14:21
charles-cooper added a commit to charles-cooper/vyper that referenced this pull request Dec 27, 2023
* feat: improve panics in IR generation

this QOL commit improves on 9165926 by passing through the
`__traceback__` field when the exception is modified (instead of using
`__cause__` - cf. PEP-3134 regarding the difference) and improves error
messages when an IRnode is not returned properly. using `__traceback__`
generally results in a better experience because the immediate cause of
the exception is displayed when running `vyper -v` instead of needing to
scroll up through the exception chain (if the exception chain is
reproduced correctly at all in the first place).

---------

Co-authored-by: Harry Kalogirou <harkal@nlogn.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants