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

add compile-time evaluation of slice() #3667

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

iFrostizz
Copy link
Contributor

@iFrostizz iFrostizz commented Nov 10, 2023

What I did

Adding compile-time evaluation support for slice.

How I did it

Fill in the blanks in the evaluate function of the Slice class builtin

How to verify it

Wrote some unit tests that compare the compile time and runtime evaluation of the function for equivalent inputs and made sure that the output were the same.

Commit message

Add compile-time support for the slice built-in

Description for the changelog

Cute Animal Picture

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

@codecov-commenter
Copy link

codecov-commenter commented Nov 10, 2023

Codecov Report

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

Comparison is base (4dd47e3) 89.61% compared to head (5b959fe) 83.68%.
Report is 15 commits behind head on master.

Files Patch % Lines
vyper/builtins/functions.py 83.87% 2 Missing and 3 partials ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3667      +/-   ##
==========================================
- Coverage   89.61%   83.68%   -5.93%     
==========================================
  Files          80       92      +12     
  Lines       11343    13077    +1734     
  Branches     2553     2940     +387     
==========================================
+ Hits        10165    10944     +779     
- Misses        791     1695     +904     
- Partials      387      438      +51     

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

if isinstance(literal_value, vy_ast.Bytes):
sublit = literal_value.value[start_val : (start_val + length_val)]
return vy_ast.Bytes.from_node(node, value=sublit)
elif isinstance(literal_value, vy_ast.Str):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for elif because the previous branch already returned

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so put "if" everywhere right ? it was lookin safer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edited!

if len(bytes_value) != 32:
raise ArgumentException("Length can only be of 32", bytestring)
elif isinstance(bytestring, vy_ast.Str):
bytes_value = bytestring.value
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charles-cooper how do you feel about this line ? Here the bytes_value type is str while otherwise it's bytes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks confusing

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.

4 participants