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: implement append and pop for dynarray #2615

Merged
merged 75 commits into from
Feb 15, 2022

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Jan 22, 2022

What I did

implement #2611

How I did it

add "append" and "pop" to typechecker, thread through codegen
make some minor changes to variables in LLL to prevent name shadowing

How to verify it

see tests

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 Jan 22, 2022

Codecov Report

Merging #2615 (ca3af6e) into master (548d35d) will increase coverage by 0.02%.
The diff coverage is 89.10%.

❗ Current head ca3af6e differs from pull request most recent head e5f13b9. Consider uploading reports for the commit e5f13b9 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2615      +/-   ##
==========================================
+ Coverage   86.25%   86.27%   +0.02%     
==========================================
  Files          91       91              
  Lines        9579     9657      +78     
  Branches     2423     2439      +16     
==========================================
+ Hits         8262     8332      +70     
- Misses        823      828       +5     
- Partials      494      497       +3     
Impacted Files Coverage Δ
vyper/lll/optimizer.py 86.28% <0.00%> (-1.51%) ⬇️
vyper/semantics/types/bases.py 83.95% <ø> (ø)
vyper/semantics/validation/annotation.py 90.60% <71.42%> (-1.07%) ⬇️
vyper/semantics/types/function.py 84.92% <80.00%> (-0.49%) ⬇️
vyper/semantics/types/indexable/sequence.py 88.09% <83.33%> (-0.34%) ⬇️
vyper/codegen/core.py 84.21% <97.82%> (+0.98%) ⬆️
vyper/codegen/expr.py 79.28% <100.00%> (+0.19%) ⬆️
vyper/codegen/stmt.py 89.02% <100.00%> (+0.51%) ⬆️
vyper/semantics/validation/local.py 91.34% <100.00%> (ø)
vyper/semantics/validation/utils.py 91.03% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 548d35d...e5f13b9. Read the comment docs.

charles-cooper and others added 28 commits February 5, 2022 15:58
WIP: frontend for dynamic array functions
we can run into issues if loop variables get repeated in the same scope,
for safety just don't allow it.
clarify some code by moving some optimizations down into optimizer

also, block expressions like `empty(uint256[3])[ix]`
for maintainability - so it's easier to track where `empty` literals get
handled.
this is important because it is used in array bounds checks
in some tests when optimizations are off, apparently the gas usage can
increase base fee past 0
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 14, 2022

This pull request introduces 3 alerts when merging fcfd1ea into 5efb11a - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Variable defined multiple times

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 14, 2022

This pull request introduces 3 alerts when merging fb6a127 into 1368adb - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Variable defined multiple times

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 15, 2022

This pull request introduces 3 alerts when merging e08f0d3 into 1368adb - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Variable defined multiple times

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 15, 2022

This pull request introduces 3 alerts when merging f1378c6 into 548d35d - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Variable defined multiple times

@charles-cooper charles-cooper marked this pull request as ready for review February 15, 2022 01:14
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 15, 2022

This pull request introduces 2 alerts when merging ca3af6e into 548d35d - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes
  • 1 for Missing call to `__init__` during object initialization

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 15, 2022

This pull request introduces 1 alert when merging e5f13b9 into 43f491f - view on LGTM.com

new alerts:

  • 1 for Missing call to `__init__` during object initialization

self.my_array.append(x) # fail
return self.my_array
""",
lambda x: None,
Copy link
Member

Choose a reason for hiding this comment

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

This raises right?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah - return of the sentinel None maps to assert_tx_failed in the test

@charles-cooper charles-cooper merged commit 27cec1b into vyperlang:master Feb 15, 2022
@charles-cooper charles-cooper deleted the dyn_array_functions branch February 15, 2022 16:08
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