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

Folding builtins #1949

Merged
merged 21 commits into from
May 8, 2020
Merged

Conversation

iamdefinitelyahuman
Copy link
Contributor

@iamdefinitelyahuman iamdefinitelyahuman commented May 6, 2020

What I did

Add constant folding logic for builtin functions.

How I did it

  • folding logic is found in the evaluate method of each class in vyper/functions/functions.py
  • the high level execution is handled via replace_builtin_functions in vyper/ast/folding.py
  • test cases are in tests/functions/folding/

The only folding implementation I'm not testing is method_id. This function should always fold, so verifying the result of folding vs not-folding seems redundant. Once we add folding into the compiler process we can update the existing test cases around this function.

Next Steps

Things to do next that are outside the scope of this PR.

  • add documentation
    • updates to "Types" and "Builtin functions" within RTD
    • add docstrings to classes in vyper/functions/functions.py
    • add README.md to vyper/functions
  • plug folding logic into main compiler process
  • refactor out now-redundant code

Cute Animal Picture

image

@iamdefinitelyahuman iamdefinitelyahuman force-pushed the folding-builtins branch 3 times, most recently from 00b54f2 to 2efda86 Compare May 6, 2020 21:29
@codecov-io
Copy link

codecov-io commented May 6, 2020

Codecov Report

Merging #1949 into master will decrease coverage by 2.24%.
The diff coverage is 39.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1949      +/-   ##
==========================================
- Coverage   87.04%   84.79%   -2.25%     
==========================================
  Files          55       56       +1     
  Lines        6444     6642     +198     
  Branches     1629     1689      +60     
==========================================
+ Hits         5609     5632      +23     
- Misses        531      692     +161     
- Partials      304      318      +14     
Impacted Files Coverage Δ
vyper/ast/__init__.py 100.00% <ø> (ø)
vyper/ast/validation.py 21.73% <21.73%> (ø)
vyper/functions/functions.py 77.58% <38.53%> (-19.62%) ⬇️
vyper/ast/folding.py 79.01% <66.66%> (-5.12%) ⬇️
vyper/exceptions.py 90.41% <100.00%> (ø)
vyper/ast/nodes.py 85.50% <0.00%> (-5.12%) ⬇️

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 7304472...82ae1af. Read the comment docs.

@iamdefinitelyahuman iamdefinitelyahuman force-pushed the folding-builtins branch 2 times, most recently from e9b3aca to f6cf5bb Compare May 6, 2020 22:08
@fubuloubu
Copy link
Member

fubuloubu commented May 7, 2020

Lol, looks like you found a Hypothesis bug

https://github.com/vyperlang/vyper/pull/1949/checks?check_run_id=651123760

@iamdefinitelyahuman iamdefinitelyahuman force-pushed the folding-builtins branch 5 times, most recently from 7eb7112 to 6ba150a Compare May 7, 2020 13:10
vyper/functions/functions.py Show resolved Hide resolved
vyper/functions/functions.py Outdated Show resolved Hide resolved
vyper/functions/functions.py Outdated Show resolved Hide resolved
vyper/functions/functions.py Outdated Show resolved Hide resolved
vyper/functions/functions.py Show resolved Hide resolved
tests/functions/folding/test_bitwise.py Outdated Show resolved Hide resolved
tests/functions/folding/test_keccak_sha.py Outdated Show resolved Hide resolved
@iamdefinitelyahuman
Copy link
Contributor Author

@fubuloubu unless you have other comments I think this is ready for merging after #1956 merges.

@iamdefinitelyahuman iamdefinitelyahuman marked this pull request as ready for review May 8, 2020 11:13
@fubuloubu fubuloubu merged commit 82f3071 into vyperlang:master May 8, 2020
@iamdefinitelyahuman iamdefinitelyahuman deleted the folding-builtins branch May 9, 2020 16:42
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.

None yet

3 participants